From 38820e5de21c78d8f98f11bfc69097eee1457c75 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 11 Nov 2021 09:13:04 +0100 Subject: [PATCH] Do not wait for import resolution and refine docs --- docs/05-plugin-development.md | 13 +++++-- src/Module.ts | 2 +- src/ModuleLoader.ts | 21 +++-------- .../samples/preload-cyclic-module/_config.js | 37 +++++++++++++++++++ .../samples/preload-cyclic-module/main.js | 6 +++ .../samples/preload-module/_config.js | 4 +- 6 files changed, 62 insertions(+), 21 deletions(-) create mode 100644 test/function/samples/preload-cyclic-module/_config.js create mode 100644 test/function/samples/preload-cyclic-module/main.js diff --git a/docs/05-plugin-development.md b/docs/05-plugin-development.md index d3356dcf4e1..17dbd5f9f1c 100644 --- a/docs/05-plugin-development.md +++ b/docs/05-plugin-development.md @@ -695,16 +695,23 @@ Loads and parses the module corresponding to the given id, attaching additional This allows you to inspect the final content of modules before deciding how to resolve them in the [`resolveId`](guide/en/#resolveid) hook and e.g. resolve to a proxy module instead. If the module becomes part of the graph later, there is no additional overhead from using this context function as the module will not be parsed again. The signature allows you to directly pass the return value of [`this.resolve`](guide/en/#thisresolve) to this function as long as it is neither `null` nor external. +The returned promise will resolve once the module has been fully transformed and parsed but before any imports have been resolved. That means that the resulting `ModuleInfo` will have empty `importedIds` and `dynamicallyImportedIds`. This helps to avoid deadlock situations when awaiting `this.load` in a `resolveId` hook. If you are interested in `importedIds` and `dynamicallyImportedIds`, you need should implement a `moduleParsed` hook. + Note that with regard to the `moduleSideEffects`, `syntheticNamedExports` and `meta` options, the same restrictions apply as for the `resolveId` hook: Their values only have an effect if the module has not been loaded yet. Thus, it is very important to use `this.resolve` first to find out if any plugins want to set special values for these options in their `resolveId` hook, and pass these options on to `this.load` if appropriate. The example below showcases how this can be handled to add a proxy module for modules containing a special code comment: ```js export default function addProxyPlugin() { return { async resolveId(source, importer, options) { + if (importer?.endsWith('?proxy')) { + // Do not proxy ids used in proxies + return null; + } // We make sure to pass on any resolveId options to this.resolve to get the module id const resolution = await this.resolve(source, importer, { skipSelf: true, ...options }); // We can only pre-load existing and non-external ids - if (!resolution?.external) { + if (resolution && !resolution.external) { + // we pass on the entire resolution information const moduleInfo = await this.load(resolution); if (moduleInfo.code.indexOf('/* use proxy */') >= 0) { return `${resolution.id}?proxy`; @@ -716,7 +723,7 @@ export default function addProxyPlugin() { load(id) { if (id.endsWith('?proxy')) { const importee = id.slice(0, -'?proxy'.length); - return `console.log('proxy!); export * from '${importee}';`; + return `console.log('proxy for ${importee}'); export * from ${JSON.stringify(importee)};`; } return null; } @@ -726,7 +733,7 @@ export default function addProxyPlugin() { If the module was already loaded, this will just wait for the parsing to complete and then return its module information. If the module was not yet imported by another module, this will not automatically trigger loading other modules imported by this module. Instead, static and dynamic dependencies will only be loaded once this module has actually been imported at least once. -Be aware that you cannot await calling `this.load` for a module during that module's own `load` or `transform` hook as that would essentially wait for those hooks to complete first and lead to a deadlock. +While it is safe to use `this.load` in a `resolveId` hook, you should be very careful when awaiting it in a `load` or `transform` hook. If there are cyclic dependencies in the module graph, this can easily lead to a deadlock, so any plugin needs to manually take care to avoid waiting for `this.load` inside the `load` or `transform` of the any module that is in a cycle with the loaded module. #### `this.meta` diff --git a/src/Module.ts b/src/Module.ts index 4c48420228e..98fd5e8d4dc 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -280,7 +280,7 @@ export default class Module { return Array.from(module.implicitlyLoadedBefore, getId); }, get importedIds() { - return Array.from(module.sources, source => module.resolvedIds[source].id); + return Array.from(module.sources, source => module.resolvedIds[source]?.id).filter(Boolean); }, get importers() { return module.importers.sort(); diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index 0649d0b0951..3442b5f1061 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -65,14 +65,7 @@ export class ModuleLoader { private readonly implicitEntryModules = new Set(); private readonly indexedEntryModules: { index: number; module: Module }[] = []; private latestLoadModulesPromise: Promise = Promise.resolve(); - private moduleLoadingState = new Map< - Module, - { - loadAndResolveDependenciesPromise: Promise; - // Set to null once/if dependencies will be loaded as well - loadPromise: null | LoadModulePromise; - } - >(); + private moduleLoadPromises = new Map(); private nextEntryModuleIndex = 0; private readQueue = new Queue(); @@ -372,10 +365,9 @@ export class ModuleLoader { }); if (isPreload) { - this.moduleLoadingState.set(module, { loadAndResolveDependenciesPromise, loadPromise }); - await loadAndResolveDependenciesPromise; + this.moduleLoadPromises.set(module, loadPromise); + await loadPromise; } else { - this.moduleLoadingState.set(module, { loadAndResolveDependenciesPromise, loadPromise: null }); await this.fetchModuleDependencies(module, ...(await loadPromise)); // To handle errors when resolving dependencies or in moduleParsed await loadAndResolveDependenciesPromise; @@ -529,9 +521,9 @@ export class ModuleLoader { } private async handleExistingModule(module: Module, isEntry: boolean, isPreload: boolean) { - const loadingState = this.moduleLoadingState.get(module)!; + const loadPromise = this.moduleLoadPromises.get(module); if (isPreload) { - await loadingState.loadAndResolveDependenciesPromise; + await loadPromise; return; } if (isEntry) { @@ -542,9 +534,8 @@ export class ModuleLoader { } module.implicitlyLoadedAfter.clear(); } - const { loadPromise } = loadingState; if (loadPromise) { - loadingState.loadPromise = null; + this.moduleLoadPromises.delete(module); await this.fetchModuleDependencies(module, ...(await loadPromise)); } return; diff --git a/test/function/samples/preload-cyclic-module/_config.js b/test/function/samples/preload-cyclic-module/_config.js new file mode 100644 index 00000000000..6de3be6c77f --- /dev/null +++ b/test/function/samples/preload-cyclic-module/_config.js @@ -0,0 +1,37 @@ +module.exports = { + description: 'handles pre-loading a cyclic module in the resolveId hook', + warnings: [ + { + code: 'CIRCULAR_DEPENDENCY', + cycle: ['main.js', 'main.js?proxy', 'main.js'], + importer: 'main.js', + message: 'Circular dependency: main.js -> main.js?proxy -> main.js' + } + ], + options: { + plugins: [ + { + async resolveId(source, importer, options) { + if (!importer || importer.endsWith('?proxy')) { + return null; + } + const resolution = await this.resolve(source, importer, { skipSelf: true, ...options }); + if (resolution && !resolution.external) { + const moduleInfo = await this.load(resolution); + if (moduleInfo.code.indexOf('/* use proxy */') >= 0) { + return `${resolution.id}?proxy`; + } + } + return resolution; + }, + load(id) { + if (id.endsWith('?proxy')) { + const importee = id.slice(0, -'?proxy'.length); + return `export * from ${JSON.stringify(importee)}; export const extra = 'extra';`; + } + return null; + } + } + ] + } +}; diff --git a/test/function/samples/preload-cyclic-module/main.js b/test/function/samples/preload-cyclic-module/main.js new file mode 100644 index 00000000000..57f7fea2ae5 --- /dev/null +++ b/test/function/samples/preload-cyclic-module/main.js @@ -0,0 +1,6 @@ +/* main *//* use proxy */ +import { foo as bar, extra } from './main.js'; +export const foo = 'foo'; +assert.strictEqual(bar, 'foo'); +assert.strictEqual(extra, 'extra'); + diff --git a/test/function/samples/preload-module/_config.js b/test/function/samples/preload-module/_config.js index 1c2eddf7eea..f9326371ae0 100644 --- a/test/function/samples/preload-module/_config.js +++ b/test/function/samples/preload-module/_config.js @@ -38,7 +38,7 @@ module.exports = { id: ID_MAIN, implicitlyLoadedAfterOneOf: [], implicitlyLoadedBefore: [], - importedIds: [ID_DEP], + importedIds: [], importers: [], isEntry: false, isExternal: false, @@ -50,7 +50,7 @@ module.exports = { transformedModules.filter(id => id === ID_MAIN, 'transformed').length, 1 ); - assert.strictEqual(parsedModules.filter(id => id === ID_MAIN, 'parsed').length, 1); + assert.strictEqual(parsedModules.filter(id => id === ID_MAIN, 'parsed').length, 0); // No dependencies have been loaded yet assert.deepStrictEqual([...this.getModuleIds()], [ID_MAIN]); await this.load({ id: ID_OTHER });