Skip to content

Commit ef25721

Browse files
lukastaegertlukastaegert
authored
Improve and fix cross-chunk-reexport warning (#4829)
* Improve and fix cross-chunk-reexport warning * Show warning also for namespace imports
1 parent 7c50c18 commit ef25721

14 files changed

Lines changed: 175 additions & 14 deletions

File tree

‎src/Chunk.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -710,13 +710,15 @@ export default class Chunk {
710710
alternativeReexportModule = importingModule.alternativeReexportModules.get(variable);
711711
if (alternativeReexportModule) {
712712
const exportingChunk = this.chunkByModule.get(alternativeReexportModule);
713-
if (exportingChunk && exportingChunk !== exportChunk) {
713+
if (exportingChunk !== exportChunk) {
714714
this.inputOptions.onwarn(
715715
errorCyclicCrossChunkReexport(
716-
variableModule.getExportNamesByVariable().get(variable)![0],
716+
// Namespaces do not have an export name
717+
variableModule.getExportNamesByVariable().get(variable)?.[0] || '*',
717718
variableModule.id,
718719
alternativeReexportModule.id,
719-
importingModule.id
720+
importingModule.id,
721+
this.outputOptions.preserveModules
720722
)
721723
);
722724
}
@@ -732,8 +734,10 @@ export default class Chunk {
732734
for (const exportedVariable of map.keys()) {
733735
const isSynthetic = exportedVariable instanceof SyntheticNamedExportVariable;
734736
const importedVariable = isSynthetic ? exportedVariable.getBaseVariable() : exportedVariable;
737+
this.checkCircularDependencyImport(importedVariable, module);
738+
// When preserving modules, we do not create namespace objects but directly
739+
// use the actual namespaces, which would be broken by this logic.
735740
if (!(importedVariable instanceof NamespaceVariable && this.outputOptions.preserveModules)) {
736-
this.checkCircularDependencyImport(importedVariable, module);
737741
const exportingModule = importedVariable.module;
738742
if (exportingModule instanceof Module) {
739743
const chunk = this.chunkByModule.get(exportingModule);
@@ -1374,12 +1378,13 @@ export default class Chunk {
13741378
const chunk = this.chunkByModule.get(variable.module as Module);
13751379
if (chunk !== this) {
13761380
this.imports.add(variable);
1377-
if (
1378-
!(variable instanceof NamespaceVariable && this.outputOptions.preserveModules) &&
1379-
variable.module instanceof Module
1380-
) {
1381-
chunk!.exports.add(variable);
1381+
if (variable.module instanceof Module) {
13821382
this.checkCircularDependencyImport(variable, module);
1383+
// When preserving modules, we do not create namespace objects but directly
1384+
// use the actual namespaces, which would be broken by this logic.
1385+
if (!(variable instanceof NamespaceVariable && this.outputOptions.preserveModules)) {
1386+
chunk!.exports.add(variable);
1387+
}
13831388
}
13841389
}
13851390
}

‎src/utils/error.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,8 @@ export function errorCyclicCrossChunkReexport(
259259
exportName: string,
260260
exporter: string,
261261
reexporter: string,
262-
importer: string
262+
importer: string,
263+
preserveModules: boolean
263264
): RollupLog {
264265
return {
265266
code: CYCLIC_CROSS_CHUNK_REEXPORT,
@@ -271,7 +272,9 @@ export function errorCyclicCrossChunkReexport(
271272
reexporter
272273
)}" 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(
273274
importer
274-
)}" to point directly to the exporting module or do not use "preserveModules" to ensure these modules end up in the same chunk.`,
275+
)}" to point directly to the exporting module or ${
276+
preserveModules ? 'do not use "output.preserveModules"' : 'reconfigure "output.manualChunks"'
277+
} to ensure these modules end up in the same chunk.`,
275278
reexporter
276279
};
277280
}

‎test/function/samples/circular-missed-reexports/_config.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,14 @@ module.exports = {
3838
^
3939
2: export { exists };`,
4040
url: 'https://rollupjs.org/troubleshooting/#error-name-is-not-exported-by-module'
41+
},
42+
{
43+
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
44+
exporter: ID_DEP1,
45+
id: ID_MAIN,
46+
message:
47+
'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.',
48+
reexporter: ID_DEP2
4149
}
4250
]
4351
};

‎test/function/samples/circular-namespace-reexport-manual-chunks/_config.js‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
const path = require('node:path');
2+
3+
const ID_INDEX = path.join(__dirname, 'index.js');
4+
const ID_TYPES = path.join(__dirname, 'types.js');
5+
const ID_FORMATTERS = path.join(__dirname, 'formatters.js');
6+
const ID_MAIN = path.join(__dirname, 'main.js');
7+
8+
module.exports = {
9+
description:
10+
'correctly handles namespace reexports with circular dependencies when using manual chunks',
11+
options: {
12+
output: {
13+
manualChunks(id) {
14+
return path.basename(id);
15+
}
16+
}
17+
},
18+
warnings: [
19+
{
20+
code: 'CIRCULAR_DEPENDENCY',
21+
ids: [ID_INDEX, ID_TYPES, ID_INDEX],
22+
message: 'Circular dependency: index.js -> types.js -> index.js'
23+
},
24+
{
25+
code: 'CIRCULAR_DEPENDENCY',
26+
ids: [ID_INDEX, ID_FORMATTERS, ID_INDEX],
27+
message: 'Circular dependency: index.js -> formatters.js -> index.js'
28+
},
29+
{
30+
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
31+
exporter: ID_TYPES,
32+
id: ID_FORMATTERS,
33+
message:
34+
'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.',
35+
reexporter: ID_INDEX
36+
},
37+
{
38+
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
39+
exporter: ID_FORMATTERS,
40+
id: ID_MAIN,
41+
message:
42+
'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.',
43+
reexporter: ID_INDEX
44+
},
45+
{
46+
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
47+
exporter: ID_FORMATTERS,
48+
id: ID_MAIN,
49+
message:
50+
'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.',
51+
reexporter: ID_INDEX
52+
},
53+
{
54+
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
55+
exporter: ID_FORMATTERS,
56+
id: ID_TYPES,
57+
message:
58+
'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.',
59+
reexporter: ID_INDEX
60+
},
61+
{
62+
code: 'EMPTY_BUNDLE',
63+
message: 'Generated an empty chunk: "index.js".',
64+
names: ['index.js']
65+
}
66+
]
67+
};

‎test/function/samples/circular-namespace-reexport-manual-chunks/formatters.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { LANGUAGES } from './index.js';
2+
3+
export const format = () => LANGUAGES;

‎test/function/samples/circular-namespace-reexport-manual-chunks/index.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export { LANGUAGES } from './types.js';
2+
export * as formatters from './formatters.js';

‎test/function/samples/circular-namespace-reexport-manual-chunks/main.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import { formatters } from './index.js'
2+
export default formatters;

‎test/function/samples/circular-namespace-reexport-manual-chunks/types.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import { formatters } from './index.js'
2+
export const LANGUAGES = () => formatters;

‎test/function/samples/circular-namespace-reexport-preserve-modules/_config.js‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
const path = require('node:path');
2+
3+
const ID_INDEX = path.join(__dirname, 'index.js');
4+
const ID_TYPES = path.join(__dirname, 'types.js');
5+
const ID_FORMATTERS = path.join(__dirname, 'formatters.js');
6+
const ID_MAIN = path.join(__dirname, 'main.js');
7+
8+
module.exports = {
9+
description:
10+
'correctly handles namespace reexports with circular dependencies when preserving modules',
11+
options: {
12+
output: {
13+
preserveModules: true
14+
}
15+
},
16+
warnings: [
17+
{
18+
code: 'CIRCULAR_DEPENDENCY',
19+
ids: [ID_INDEX, ID_TYPES, ID_INDEX],
20+
message: 'Circular dependency: index.js -> types.js -> index.js'
21+
},
22+
{
23+
code: 'CIRCULAR_DEPENDENCY',
24+
ids: [ID_INDEX, ID_FORMATTERS, ID_INDEX],
25+
message: 'Circular dependency: index.js -> formatters.js -> index.js'
26+
},
27+
{
28+
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
29+
exporter: ID_FORMATTERS,
30+
id: ID_MAIN,
31+
message:
32+
'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.',
33+
reexporter: ID_INDEX
34+
},
35+
{
36+
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
37+
exporter: ID_FORMATTERS,
38+
id: ID_MAIN,
39+
message:
40+
'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.',
41+
reexporter: ID_INDEX
42+
},
43+
{
44+
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
45+
exporter: ID_FORMATTERS,
46+
id: ID_TYPES,
47+
message:
48+
'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.',
49+
reexporter: ID_INDEX
50+
},
51+
{
52+
code: 'CYCLIC_CROSS_CHUNK_REEXPORT',
53+
exporter: ID_TYPES,
54+
id: ID_FORMATTERS,
55+
message:
56+
'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.',
57+
reexporter: ID_INDEX
58+
}
59+
]
60+
};

‎test/function/samples/circular-namespace-reexport-preserve-modules/formatters.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { LANGUAGES } from './index.js';
2+
3+
export const format = () => LANGUAGES;

0 commit comments

Comments
 (0)