Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve and fix cross-chunk-reexport warning #4829

Merged
merged 2 commits into from Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 14 additions & 9 deletions src/Chunk.ts
Expand Up @@ -710,13 +710,15 @@ export default class Chunk {
alternativeReexportModule = importingModule.alternativeReexportModules.get(variable);
if (alternativeReexportModule) {
const exportingChunk = this.chunkByModule.get(alternativeReexportModule);
if (exportingChunk && exportingChunk !== exportChunk) {
if (exportingChunk !== exportChunk) {
this.inputOptions.onwarn(
errorCyclicCrossChunkReexport(
variableModule.getExportNamesByVariable().get(variable)![0],
// Namespaces do not have an export name
variableModule.getExportNamesByVariable().get(variable)?.[0] || '*',
variableModule.id,
alternativeReexportModule.id,
importingModule.id
importingModule.id,
this.outputOptions.preserveModules
)
);
}
Expand All @@ -732,8 +734,10 @@ export default class Chunk {
for (const exportedVariable of map.keys()) {
const isSynthetic = exportedVariable instanceof SyntheticNamedExportVariable;
const importedVariable = isSynthetic ? exportedVariable.getBaseVariable() : exportedVariable;
this.checkCircularDependencyImport(importedVariable, module);
// When preserving modules, we do not create namespace objects but directly
// use the actual namespaces, which would be broken by this logic.
if (!(importedVariable instanceof NamespaceVariable && this.outputOptions.preserveModules)) {
this.checkCircularDependencyImport(importedVariable, module);
const exportingModule = importedVariable.module;
if (exportingModule instanceof Module) {
const chunk = this.chunkByModule.get(exportingModule);
Expand Down Expand Up @@ -1374,12 +1378,13 @@ export default class Chunk {
const chunk = this.chunkByModule.get(variable.module as Module);
if (chunk !== this) {
this.imports.add(variable);
if (
!(variable instanceof NamespaceVariable && this.outputOptions.preserveModules) &&
variable.module instanceof Module
) {
chunk!.exports.add(variable);
if (variable.module instanceof Module) {
this.checkCircularDependencyImport(variable, module);
// When preserving modules, we do not create namespace objects but directly
// use the actual namespaces, which would be broken by this logic.
if (!(variable instanceof NamespaceVariable && this.outputOptions.preserveModules)) {
chunk!.exports.add(variable);
}
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/utils/error.ts
Expand Up @@ -259,7 +259,8 @@ export function errorCyclicCrossChunkReexport(
exportName: string,
exporter: string,
reexporter: string,
importer: string
importer: string,
preserveModules: boolean
): RollupLog {
return {
code: CYCLIC_CROSS_CHUNK_REEXPORT,
Expand All @@ -271,7 +272,9 @@ export function errorCyclicCrossChunkReexport(
reexporter
)}" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "${relativeId(
importer
)}" to point directly to the exporting module or do not use "preserveModules" to ensure these modules end up in the same chunk.`,
)}" to point directly to the exporting module or ${
preserveModules ? 'do not use "output.preserveModules"' : 'reconfigure "output.manualChunks"'
} to ensure these modules end up in the same chunk.`,
reexporter
};
}
Expand Down
8 changes: 8 additions & 0 deletions test/function/samples/circular-missed-reexports/_config.js
Expand Up @@ -38,6 +38,14 @@ module.exports = {
^
2: export { exists };`,
url: 'https://rollupjs.org/troubleshooting/#error-name-is-not-exported-by-module'
},
{
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
exporter: ID_DEP1,
id: ID_MAIN,
message:
'Export "exists4" of module "dep1.js" was reexported through module "dep2.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "main.js" to point directly to the exporting module or reconfigure "output.manualChunks" to ensure these modules end up in the same chunk.',
reexporter: ID_DEP2
}
]
};
@@ -0,0 +1,67 @@
const path = require('node:path');

const ID_INDEX = path.join(__dirname, 'index.js');
const ID_TYPES = path.join(__dirname, 'types.js');
const ID_FORMATTERS = path.join(__dirname, 'formatters.js');
const ID_MAIN = path.join(__dirname, 'main.js');

module.exports = {
description:
'correctly handles namespace reexports with circular dependencies when using manual chunks',
options: {
output: {
manualChunks(id) {
return path.basename(id);
}
}
},
warnings: [
{
code: 'CIRCULAR_DEPENDENCY',
ids: [ID_INDEX, ID_TYPES, ID_INDEX],
message: 'Circular dependency: index.js -> types.js -> index.js'
},
{
code: 'CIRCULAR_DEPENDENCY',
ids: [ID_INDEX, ID_FORMATTERS, ID_INDEX],
message: 'Circular dependency: index.js -> formatters.js -> index.js'
},
{
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
exporter: ID_TYPES,
id: ID_FORMATTERS,
message:
'Export "LANGUAGES" of module "types.js" was reexported through module "index.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "formatters.js" to point directly to the exporting module or reconfigure "output.manualChunks" to ensure these modules end up in the same chunk.',
reexporter: ID_INDEX
},
{
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
exporter: ID_FORMATTERS,
id: ID_MAIN,
message:
'Export "*" of module "formatters.js" was reexported through module "index.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "main.js" to point directly to the exporting module or reconfigure "output.manualChunks" to ensure these modules end up in the same chunk.',
reexporter: ID_INDEX
},
{
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
exporter: ID_FORMATTERS,
id: ID_MAIN,
message:
'Export "*" of module "formatters.js" was reexported through module "index.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "main.js" to point directly to the exporting module or reconfigure "output.manualChunks" to ensure these modules end up in the same chunk.',
reexporter: ID_INDEX
},
{
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
exporter: ID_FORMATTERS,
id: ID_TYPES,
message:
'Export "*" of module "formatters.js" was reexported through module "index.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "types.js" to point directly to the exporting module or reconfigure "output.manualChunks" to ensure these modules end up in the same chunk.',
reexporter: ID_INDEX
},
{
code: 'EMPTY_BUNDLE',
message: 'Generated an empty chunk: "index.js".',
names: ['index.js']
}
]
};
@@ -0,0 +1,3 @@
import { LANGUAGES } from './index.js';

export const format = () => LANGUAGES;
@@ -0,0 +1,2 @@
export { LANGUAGES } from './types.js';
export * as formatters from './formatters.js';
@@ -0,0 +1,2 @@
import { formatters } from './index.js'
export default formatters;
@@ -0,0 +1,2 @@
import { formatters } from './index.js'
export const LANGUAGES = () => formatters;
@@ -0,0 +1,60 @@
const path = require('node:path');

const ID_INDEX = path.join(__dirname, 'index.js');
const ID_TYPES = path.join(__dirname, 'types.js');
const ID_FORMATTERS = path.join(__dirname, 'formatters.js');
const ID_MAIN = path.join(__dirname, 'main.js');

module.exports = {
description:
'correctly handles namespace reexports with circular dependencies when preserving modules',
options: {
output: {
preserveModules: true
}
},
warnings: [
{
code: 'CIRCULAR_DEPENDENCY',
ids: [ID_INDEX, ID_TYPES, ID_INDEX],
message: 'Circular dependency: index.js -> types.js -> index.js'
},
{
code: 'CIRCULAR_DEPENDENCY',
ids: [ID_INDEX, ID_FORMATTERS, ID_INDEX],
message: 'Circular dependency: index.js -> formatters.js -> index.js'
},
{
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
exporter: ID_FORMATTERS,
id: ID_MAIN,
message:
'Export "*" of module "formatters.js" was reexported through module "index.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "main.js" to point directly to the exporting module or do not use "output.preserveModules" to ensure these modules end up in the same chunk.',
reexporter: ID_INDEX
},
{
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
exporter: ID_FORMATTERS,
id: ID_MAIN,
message:
'Export "*" of module "formatters.js" was reexported through module "index.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "main.js" to point directly to the exporting module or do not use "output.preserveModules" to ensure these modules end up in the same chunk.',
reexporter: ID_INDEX
},
{
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
exporter: ID_FORMATTERS,
id: ID_TYPES,
message:
'Export "*" of module "formatters.js" was reexported through module "index.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "types.js" to point directly to the exporting module or do not use "output.preserveModules" to ensure these modules end up in the same chunk.',
reexporter: ID_INDEX
},
{
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
exporter: ID_TYPES,
id: ID_FORMATTERS,
message:
'Export "LANGUAGES" of module "types.js" was reexported through module "index.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "formatters.js" to point directly to the exporting module or do not use "output.preserveModules" to ensure these modules end up in the same chunk.',
reexporter: ID_INDEX
}
]
};
@@ -0,0 +1,3 @@
import { LANGUAGES } from './index.js';

export const format = () => LANGUAGES;
@@ -0,0 +1,2 @@
export { LANGUAGES } from './types.js';
export * as formatters from './formatters.js';
@@ -0,0 +1,2 @@
import { formatters } from './index.js'
export default formatters;
@@ -0,0 +1,2 @@
import { formatters } from './index.js'
export const LANGUAGES = () => formatters;
6 changes: 3 additions & 3 deletions test/function/samples/circular-preserve-modules/_config.js
Expand Up @@ -4,7 +4,7 @@ const ID_FIRST = path.join(__dirname, 'first.js');
const ID_SECOND = path.join(__dirname, 'second.js');

module.exports = {
description: 'correctly handles circular dependencies whe preserving modules',
description: 'correctly handles circular dependencies when preserving modules',
options: {
output: { preserveModules: true }
},
Expand All @@ -24,15 +24,15 @@ module.exports = {
exporter: ID_SECOND,
id: ID_FIRST,
message:
'Export "second" of module "second.js" was reexported through module "main.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "first.js" to point directly to the exporting module or do not use "preserveModules" to ensure these modules end up in the same chunk.',
'Export "second" of module "second.js" was reexported through module "main.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "first.js" to point directly to the exporting module or do not use "output.preserveModules" to ensure these modules end up in the same chunk.',
reexporter: ID_MAIN
},
{
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
exporter: ID_FIRST,
id: ID_SECOND,
message:
'Export "first" of module "first.js" was reexported through module "main.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "second.js" to point directly to the exporting module or do not use "preserveModules" to ensure these modules end up in the same chunk.',
'Export "first" of module "first.js" was reexported through module "main.js" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.\nEither change the import in "second.js" to point directly to the exporting module or do not use "output.preserveModules" to ensure these modules end up in the same chunk.',
reexporter: ID_MAIN
}
]
Expand Down