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
rollup · Feb 22, 2022 · 7cc87f1 · 7cc87f1
1 parent 3106ec9
commit 7cc87f1
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 8 deletions.
diff --git 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`<br> **Kind:** `async, first`<br> **Previous Hook:** [`load`](guide/en/#load) where the cached file was loaded to compare its code with the cached version.<br> **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`<br> **Kind:** `async, first`<br> **Previous Hook:** [`load`](guide/en/#load) where the cached file was loaded to compare its code with the cached version.<br> **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
@@ -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
@@ -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
@@ -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> | boolean;

diff --git a/test/function/samples/plugin-error-should-transform/_config.js 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
@@ -0,0 +1 @@
+export default 42;
diff --git 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' } });
 	});
 });