From bd150648a853e4177ca528f56bc39df1e3e7ee99 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sat, 23 Apr 2022 14:27:49 +0200 Subject: [PATCH 1/2] Throw proper error when indirectly reexporting a recursive binding --- src/Module.ts | 40 +++++++++++++++---- .../samples/circular-reexport/_config.js | 12 ++++++ .../samples/circular-reexport/main.js | 3 ++ 3 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 test/function/samples/circular-reexport/_config.js create mode 100644 test/function/samples/circular-reexport/main.js diff --git a/src/Module.ts b/src/Module.ts index e65975a4325..1e7c4ada2ba 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -134,7 +134,7 @@ function getVariableForExportNameRecursive( target: Module | ExternalModule, name: string, importerForSideEffects: Module | undefined, - isExportAllSearch: boolean, + isExportAllSearch: boolean | undefined, searchedNamesAndModules = new Map>() ): [variable: Variable | null, indirectExternal?: boolean] { const searchedModules = searchedNamesAndModules.get(name); @@ -561,7 +561,10 @@ export default class Module { return [this.exportShimVariable]; } const name = exportDeclaration.localName; - const variable = this.traceVariable(name, importerForSideEffects)!; + const variable = this.traceVariable(name, { + importerForSideEffects, + searchedNamesAndModules + })!; if (importerForSideEffects) { getOrCreate( importerForSideEffects.sideEffectDependenciesByVariable, @@ -793,7 +796,18 @@ export default class Module { }; } - traceVariable(name: string, importerForSideEffects?: Module): Variable | null { + traceVariable( + name: string, + { + importerForSideEffects, + isExportAllSearch, + searchedNamesAndModules + }: { + importerForSideEffects?: Module; + isExportAllSearch?: boolean; + searchedNamesAndModules?: Map>; + } = EMPTY_OBJECT + ): Variable | null { const localVariable = this.scope.variables.get(name); if (localVariable) { return localVariable; @@ -807,9 +821,13 @@ export default class Module { return otherModule.namespace; } - const [declaration] = otherModule.getVariableForExportName(importDeclaration.name, { - importerForSideEffects: importerForSideEffects || this - }); + const [declaration] = getVariableForExportNameRecursive( + otherModule, + importDeclaration.name, + importerForSideEffects || this, + isExportAllSearch, + searchedNamesAndModules + ); if (!declaration) { return this.error( @@ -1057,7 +1075,9 @@ export default class Module { name, importerForSideEffects, true, - searchedNamesAndModules + // We are creating a copy to handle the case where the same binding is + // imported through different namespace reexports gracefully + copyNameToModulesMap(searchedNamesAndModules) ); if (module instanceof ExternalModule || indirectExternal) { @@ -1199,3 +1219,9 @@ function setAlternativeExporterIfCyclic( } } } + +const copyNameToModulesMap = ( + searchedNamesAndModules?: Map> +): Map> | undefined => + searchedNamesAndModules && + new Map(Array.from(searchedNamesAndModules, ([name, modules]) => [name, new Set(modules)])); diff --git a/test/function/samples/circular-reexport/_config.js b/test/function/samples/circular-reexport/_config.js new file mode 100644 index 00000000000..6379705fca7 --- /dev/null +++ b/test/function/samples/circular-reexport/_config.js @@ -0,0 +1,12 @@ +const path = require('path'); +const ID_MAIN = path.join(__dirname, 'main.js'); + +module.exports = { + description: 'throws proper error for circular reexports', + error: { + code: 'CIRCULAR_REEXPORT', + id: ID_MAIN, + message: '"foo" cannot be exported from main.js as it is a reexport that references itself.', + watchFiles: [ID_MAIN] + } +}; diff --git a/test/function/samples/circular-reexport/main.js b/test/function/samples/circular-reexport/main.js new file mode 100644 index 00000000000..6572efc076a --- /dev/null +++ b/test/function/samples/circular-reexport/main.js @@ -0,0 +1,3 @@ +import { foo } from './main.js'; + +export { foo }; From 8de69439716b446a24dabf1ad0947a4a9bd6cd29 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 28 Apr 2022 06:39:18 +0200 Subject: [PATCH 2/2] Fix unrelated typo --- docs/05-plugin-development.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/05-plugin-development.md b/docs/05-plugin-development.md index 89d30b1ea79..9d6fa5447a3 100644 --- a/docs/05-plugin-development.md +++ b/docs/05-plugin-development.md @@ -905,7 +905,7 @@ Use Rollup's internal acorn instance to parse code to an AST. #### `this.resolve` -**Type:** `(source: string, importer?: string, options?: {skipSelf?: boolean, isEntry?: boolean, isEntry?: boolean, custom?: {[plugin: string]: any}}) => Promise<{id: string, external: boolean | "absolute", moduleSideEffects: boolean | 'no-treeshake', syntheticNamedExports: boolean | string, meta: {[plugin: string]: any}} | null>` +**Type:** `(source: string, importer?: string, options?: {skipSelf?: boolean, isEntry?: boolean, custom?: {[plugin: string]: any}}) => Promise<{id: string, external: boolean | "absolute", moduleSideEffects: boolean | 'no-treeshake', syntheticNamedExports: boolean | string, meta: {[plugin: string]: any}} | null>` Resolve imports to module ids (i.e. file names) using the same plugins that Rollup uses, and determine if an import should be external. If `null` is returned, the import could not be resolved by Rollup or any plugin but was not explicitly marked as external by the user. If an absolute external id is returned that should remain absolute in the output either via the [`makeAbsoluteExternalsRelative`](guide/en/#makeabsoluteexternalsrelative) option or by explicit plugin choice in the [`resolveId`](guide/en/#resolveid) hook, `external` will be `"absolute"` instead of `true`.