From 7cc87f1b447716f4823b10186d01ce02e75d7828 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 22 Feb 2022 07:12:11 +0100 Subject: [PATCH] Add resolved sources to shouldTransformCachedModule (#4414) * Add dependencies to shouldTransform hook * Test errors in shouldTransform are handled correctly * Provide resolvedSources instead of importedIds * Add documentation --- docs/05-plugin-development.md | 2 +- src/Module.ts | 5 ++ src/ModuleLoader.ts | 1 + src/rollup/types.d.ts | 3 +- .../plugin-error-should-transform/_config.js | 44 ++++++++++++++++ .../plugin-error-should-transform/main.js | 1 + test/incremental/index.js | 51 ++++++++++++++++--- 7 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 test/function/samples/plugin-error-should-transform/_config.js create mode 100644 test/function/samples/plugin-error-should-transform/main.js diff --git a/docs/05-plugin-development.md b/docs/05-plugin-development.md index 3c58242ec05..228b0d81807 100644 --- a/docs/05-plugin-development.md +++ b/docs/05-plugin-development.md @@ -266,7 +266,7 @@ In watch mode or when using the cache explicitly, the resolved imports of a cach #### `shouldTransformCachedModule` -**Type:** `({id: string, code: string, ast: ESTree.Program, meta: {[plugin: string]: any}, moduleSideEffects: boolean | "no-treeshake", syntheticNamedExports: string | boolean}) => boolean`
**Kind:** `async, first`
**Previous Hook:** [`load`](guide/en/#load) where the cached file was loaded to compare its code with the cached version.
**Next Hook:** [`moduleParsed`](guide/en/#moduleparsed) if no plugin returns `true`, otherwise [`transform`](guide/en/#transform). +**Type:** `({id: string, code: string, ast: ESTree.Program, resoledSources: {[source: string]: ResolvedId}, meta: {[plugin: string]: any}, moduleSideEffects: boolean | "no-treeshake", syntheticNamedExports: string | boolean}) => boolean`
**Kind:** `async, first`
**Previous Hook:** [`load`](guide/en/#load) where the cached file was loaded to compare its code with the cached version.
**Next Hook:** [`moduleParsed`](guide/en/#moduleparsed) if no plugin returns `true`, otherwise [`transform`](guide/en/#transform). If the Rollup cache is used (e.g. in watch mode or explicitly via the JavaScript API), Rollup will skip the [`transform`](guide/en/#transform) hook of a module if after the [`load`](guide/en/#transform) hook, the loaded `code` is identical to the code of the cached copy. To prevent this, discard the cached copy and instead transform a module, plugins can implement this hook and return `true`. diff --git a/src/Module.ts b/src/Module.ts index 0ce3ef4e39b..789b5cad9b9 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -279,6 +279,8 @@ export default class Module { .filter(Boolean) as ResolvedId[]; }, get dynamicallyImportedIds() { + // We cannot use this.dynamicDependencies because this is needed before + // dynamicDependencies are populated const dynamicallyImportedIds: string[] = []; for (const { id } of dynamicImports) { if (id) { @@ -316,6 +318,8 @@ export default class Module { return Array.from(sources, source => module.resolvedIds[source]).filter(Boolean); }, get importedIds() { + // We cannot use this.dependencies because this is needed before + // dependencies are populated return Array.from(sources, source => module.resolvedIds[source]?.id).filter(Boolean); }, get importers() { @@ -703,6 +707,7 @@ export default class Module { transformFiles, ...moduleOptions }: TransformModuleJSON & { + resolvedIds?: ResolvedIdMap; transformFiles?: EmittedFile[] | undefined; }): void { this.info.code = code; diff --git a/src/ModuleLoader.ts b/src/ModuleLoader.ts index efa402f6a80..9589da12e51 100644 --- a/src/ModuleLoader.ts +++ b/src/ModuleLoader.ts @@ -283,6 +283,7 @@ export class ModuleLoader { id: cachedModule.id, meta: cachedModule.meta, moduleSideEffects: cachedModule.moduleSideEffects, + resolvedSources: cachedModule.resolvedIds, syntheticNamedExports: cachedModule.syntheticNamedExports } ])) diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index baf2b6c553a..10a8341e873 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -108,7 +108,6 @@ export interface TransformModuleJSON { customTransformCache: boolean; originalCode: string; originalSourcemap: ExistingDecodedSourceMap | null; - resolvedIds?: ResolvedIdMap; sourcemapChain: DecodedSourceMapOrMissing[]; transformDependencies: string[]; } @@ -117,6 +116,7 @@ export interface ModuleJSON extends TransformModuleJSON, ModuleOptions { ast: AcornNode; dependencies: string[]; id: string; + resolvedIds: ResolvedIdMap; transformFiles: EmittedFile[] | undefined; } @@ -254,6 +254,7 @@ export type ShouldTransformCachedModuleHook = ( id: string; meta: CustomPluginOptions; moduleSideEffects: boolean | 'no-treeshake'; + resolvedSources: ResolvedIdMap; syntheticNamedExports: boolean | string; } ) => Promise | boolean; diff --git a/test/function/samples/plugin-error-should-transform/_config.js b/test/function/samples/plugin-error-should-transform/_config.js new file mode 100644 index 00000000000..8f1780395b5 --- /dev/null +++ b/test/function/samples/plugin-error-should-transform/_config.js @@ -0,0 +1,44 @@ +const path = require('path'); +const acorn = require('acorn'); + +const code = 'export default 42;\n'; +const ID_MAIN = path.join(__dirname, 'main.js'); + +module.exports = { + description: 'errors in shouldTransformCachedModule abort the build', + options: { + cache: { + modules: [ + { + id: ID_MAIN, + ast: acorn.parse(code, { + ecmaVersion: 6, + sourceType: 'module' + }), + code, + dependencies: [], + dynamicDependencies: [], + originalCode: code, + resolvedIds: {}, + sourcemapChain: [], + transformDependencies: [] + } + ] + }, + plugins: [ + { + name: 'test', + shouldTransformCachedModule() { + throw new Error('broken'); + } + } + ] + }, + error: { + code: 'PLUGIN_ERROR', + hook: 'shouldTransformCachedModule', + message: 'broken', + plugin: 'test', + watchFiles: [ID_MAIN] + } +}; diff --git a/test/function/samples/plugin-error-should-transform/main.js b/test/function/samples/plugin-error-should-transform/main.js new file mode 100644 index 00000000000..7a4e8a723a4 --- /dev/null +++ b/test/function/samples/plugin-error-should-transform/main.js @@ -0,0 +1 @@ +export default 42; diff --git a/test/incremental/index.js b/test/incremental/index.js index 7de0fb47aeb..542785ce873 100644 --- a/test/incremental/index.js +++ b/test/incremental/index.js @@ -338,10 +338,15 @@ describe('incremental', () => { }); it('runs shouldTransformCachedModule when using a cached module', async () => { + modules = { + entry: `import foo from 'foo'; export default foo;`, + foo: `export default import('bar')`, + bar: `export default 42` + }; let shouldTransformCachedModuleCalls = 0; const transformPlugin = { - async shouldTransformCachedModule({ ast, id, meta, ...other }) { + async shouldTransformCachedModule({ ast, id, meta, resolvedSources, ...other }) { shouldTransformCachedModuleCalls++; assert.strictEqual(ast.type, 'Program'); assert.deepStrictEqual(other, { @@ -352,10 +357,34 @@ describe('incremental', () => { switch (id) { case 'foo': assert.deepStrictEqual(meta, { transform: { calls: 1, id } }); + assert.deepStrictEqual(resolvedSources, { + __proto__: null, + bar: { + external: false, + id: 'bar', + meta: {}, + moduleSideEffects: true, + syntheticNamedExports: false + } + }); // we return promises to ensure they are awaited return Promise.resolve(false); + case 'bar': + assert.deepStrictEqual(meta, { transform: { calls: 2, id } }); + assert.deepStrictEqual(resolvedSources, { __proto__: null }); + return Promise.resolve(false); case 'entry': assert.deepStrictEqual(meta, { transform: { calls: 0, id } }); + assert.deepStrictEqual(resolvedSources, { + __proto__: null, + foo: { + external: false, + id: 'foo', + meta: {}, + moduleSideEffects: true, + syntheticNamedExports: false + } + }); return Promise.resolve(true); default: throw new Error(`Unexpected id ${id}.`); @@ -369,8 +398,12 @@ describe('incremental', () => { input: 'entry', plugins: [transformPlugin, plugin] }); - assert.strictEqual(shouldTransformCachedModuleCalls, 0); - assert.strictEqual(transformCalls, 2); + assert.strictEqual( + shouldTransformCachedModuleCalls, + 0, + 'initial shouldTransformCachedModule calls' + ); + assert.strictEqual(transformCalls, 3, 'initial transform calls'); const { cache: { modules: cachedModules } @@ -379,11 +412,17 @@ describe('incremental', () => { plugins: [transformPlugin, plugin], cache }); - assert.strictEqual(shouldTransformCachedModuleCalls, 2); - assert.strictEqual(transformCalls, 3); + assert.strictEqual( + shouldTransformCachedModuleCalls, + 3, + 'final shouldTransformCachedModule calls' + ); + assert.strictEqual(transformCalls, 4, 'final transform calls'); assert.strictEqual(cachedModules[0].id, 'foo'); assert.deepStrictEqual(cachedModules[0].meta, { transform: { calls: 1, id: 'foo' } }); assert.strictEqual(cachedModules[1].id, 'entry'); - assert.deepStrictEqual(cachedModules[1].meta, { transform: { calls: 2, id: 'entry' } }); + assert.deepStrictEqual(cachedModules[1].meta, { transform: { calls: 3, id: 'entry' } }); + assert.strictEqual(cachedModules[2].id, 'bar'); + assert.deepStrictEqual(cachedModules[2].meta, { transform: { calls: 2, id: 'bar' } }); }); });