Improve and fix cross-chunk-reexport warning (#4829)

* Improve and fix cross-chunk-reexport warning

* Show warning also for namespace imports

src/Chunk.ts

@@ -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
 							)
 						);
 					}
@@ -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);
@@ -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);
+					}
 				}
 			}
 		}

src/utils/error.ts

@@ -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,
@@ -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
 	};
 }

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

@@ -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
 		}
 	]
 };

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

@@ -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']
+		}
+	]
+};

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

@@ -0,0 +1,3 @@
+import { LANGUAGES } from './index.js';
+
+export const format = () => LANGUAGES;

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

@@ -0,0 +1,2 @@
+export { LANGUAGES } from  './types.js';
+export * as formatters from './formatters.js';

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

@@ -0,0 +1,2 @@
+import { formatters } from './index.js'
+export default formatters;

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

@@ -0,0 +1,2 @@
+import { formatters } from './index.js'
+export const LANGUAGES = () => formatters;

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

@@ -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
+		}
+	]
+};

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

@@ -0,0 +1,3 @@
+import { LANGUAGES } from './index.js';
+
+export const format = () => LANGUAGES;

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

@@ -0,0 +1,2 @@
+export { LANGUAGES } from  './types.js';
+export * as formatters from './formatters.js';

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

@@ -0,0 +1,2 @@
+import { formatters } from './index.js'
+export default formatters;

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

@@ -0,0 +1,2 @@
+import { formatters } from './index.js'
+export const LANGUAGES = () => formatters;

test/function/samples/circular-preserve-modules/_config.js

@@ -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 }
 	},
@@ -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
 		}
 	]