Implement shouldTransformCachedModule hook (#4341)

* Create docs

* Implement shouldTransformCachedModule

* Flush atomic file writes

* Fix formatting

* Improve test reliability

docs/05-plugin-development.md

@@ -102,7 +102,7 @@ Notifies a plugin when watcher process closes and all open resources should be c
 
 #### `load`
 
-**Type:** `(id: string) => string | null | {code: string, map?: string | SourceMap, ast? : ESTree.Program, moduleSideEffects?: boolean | "no-treeshake" | null, syntheticNamedExports?: boolean | string | null, meta?: {[plugin: string]: any} | null}`<br> **Kind:** `async, first`<br> **Previous Hook:** [`resolveId`](guide/en/#resolveid) or [`resolveDynamicImport`](guide/en/#resolvedynamicimport) where the loaded id was resolved. Additionally, this hook can be triggered at any time from plugin hooks by calling [`this.load`](guide/en/#thisload) to preload the module corresponding to an id.<br> **Next Hook:** [`transform`](guide/en/#transform) to transform the loaded file.
+**Type:** `(id: string) => string | null | {code: string, map?: string | SourceMap, ast? : ESTree.Program, moduleSideEffects?: boolean | "no-treeshake" | null, syntheticNamedExports?: boolean | string | null, meta?: {[plugin: string]: any} | null}`<br> **Kind:** `async, first`<br> **Previous Hook:** [`resolveId`](guide/en/#resolveid) or [`resolveDynamicImport`](guide/en/#resolvedynamicimport) where the loaded id was resolved. Additionally, this hook can be triggered at any time from plugin hooks by calling [`this.load`](guide/en/#thisload) to preload the module corresponding to an id.<br> **Next Hook:** [`transform`](guide/en/#transform) to transform the loaded file if no cache was used, or there was no cached copy with the same `code`, otherwise [`shouldTransformCachedModule`](guide/en/#shouldtransformcachedmodule).
 
 Defines a custom loader. Returning `null` defers to other `load` functions (and eventually the default behavior of loading from the file system). To prevent additional parsing overhead in case e.g. this hook already used `this.parse` to generate an AST for some reason, this hook can optionally return a `{ code, ast, map }` object. The `ast` must be a standard ESTree AST with `start` and `end` properties for each node. If the transformation does not move code, you can preserve existing sourcemaps by setting `map` to `null`. Otherwise you might need to generate the source map. See [the section on source code transformations](#source-code-transformations).
 
@@ -116,7 +116,7 @@ You can use [`this.getModuleInfo`](guide/en/#thisgetmoduleinfo) to find out the
 
 #### `moduleParsed`
 
-**Type:** `(moduleInfo: ModuleInfo) => void`<br> **Kind:** `async, parallel`<br> **Previous Hook:** [`transform`](guide/en/#transform) where the currently handled file was transformed.<br> NextHook: [`resolveId`](guide/en/#resolveid) and [`resolveDynamicImport`](guide/en/#resolvedynamicimport) to resolve all discovered static and dynamic imports in parallel if present, otherwise [`buildEnd`](guide/en/#buildend).
+**Type:** `(moduleInfo: ModuleInfo) => void`<br> **Kind:** `async, parallel`<br> **Previous Hook:** [`transform`](guide/en/#transform) where the currently handled file was transformed.<br> **Next Hook:** [`resolveId`](guide/en/#resolveid) and [`resolveDynamicImport`](guide/en/#resolvedynamicimport) to resolve all discovered static and dynamic imports in parallel if present, otherwise [`buildEnd`](guide/en/#buildend).
 
 This hook is called each time a module has been fully parsed by Rollup. See [`this.getModuleInfo`](guide/en/#thisgetmoduleinfo) for what information is passed to this hook.
 
@@ -222,9 +222,19 @@ Note that while `resolveId` will be called for each import of a module and can t
 
 When triggering this hook from a plugin via [`this.resolve`](guide/en/#thisresolve), it is possible to pass a custom options object to this hook. While this object will be passed unmodified, plugins should follow the convention of adding a `custom` property with an object where the keys correspond to the names of the plugins that the options are intended for. For details see [custom resolver options](guide/en/#custom-resolver-options).
 
+#### `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).
+
+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`.
+
+This hook can also be used to find out which modules were cached and access their cached meta information.
+
+If a plugin does not return `true`, Rollup will trigger this hook for other plugins, otherwise all remaining plugins will be skipped.
+
 #### `transform`
 
-**Type:** `(code: string, id: string) => string | null | {code?: string, map?: string | SourceMap, ast? : ESTree.Program, moduleSideEffects?: boolean | "no-treeshake" | null, syntheticNamedExports?: boolean | string | null, meta?: {[plugin: string]: any} | null}`<br> **Kind:** `async, sequential`<br> **Previous Hook:** [`load`](guide/en/#load) where the currently handled file was loaded.<br> NextHook: [`moduleParsed`](guide/en/#moduleparsed) once the file has been processed and parsed.
+**Type:** `(code: string, id: string) => string | null | {code?: string, map?: string | SourceMap, ast? : ESTree.Program, moduleSideEffects?: boolean | "no-treeshake" | null, syntheticNamedExports?: boolean | string | null, meta?: {[plugin: string]: any} | null}`<br> **Kind:** `async, sequential`<br> **Previous Hook:** [`load`](guide/en/#load) where the currently handled file was loaded. If caching is used and there was a cached copy of that module, [`shouldTransformCachedModule`](guide/en/#shouldtransformcachedmodule) if a plugin returned `true` for that hook.<br> **Next Hook:** [`moduleParsed`](guide/en/#moduleparsed) once the file has been processed and parsed.
 
 Can be used to transform individual modules. To prevent additional parsing overhead in case e.g. this hook already used `this.parse` to generate an AST for some reason, this hook can optionally return a `{ code, ast, map }` object. The `ast` must be a standard ESTree AST with `start` and `end` properties for each node. If the transformation does not move code, you can preserve existing sourcemaps by setting `map` to `null`. Otherwise you might need to generate the source map. See [the section on source code transformations](#source-code-transformations).
 

docs/build-hooks.mmd

@@ -25,6 +25,9 @@ flowchart TB
 	resolveid("resolveId"):::hook-first
 	click resolveid "/guide/en/#resolveid" _parent
 
+	shouldtransformcachedmodule("shouldTransformCachedModule"):::hook-first
+	click shouldtransformcachedmodule "/guide/en/#shouldtransformcachedmodule" _parent
+
 	transform("transform"):::hook-sequential
 	click transform "/guide/en/#transform" _parent
 
@@ -41,10 +44,17 @@ flowchart TB
 
     resolveid
     --> |non-external|load
-    --> transform
+    --> |not cached|transform
     --> moduleparsed
     .-> |no imports|buildend
 
+    load
+    --> |cached|shouldtransformcachedmodule
+    --> |false|moduleparsed
+
+    shouldtransformcachedmodule
+    --> |true|transform
+
     moduleparsed
     --> |"each import()"|resolvedynamicimport
     --> |non-external|load

src/ModuleLoader.ts

@@ -274,7 +274,17 @@ export class ModuleLoader {
 		if (
 			cachedModule &&
 			!cachedModule.customTransformCache &&
-			cachedModule.originalCode === sourceDescription.code
+			cachedModule.originalCode === sourceDescription.code &&
+			!(await this.pluginDriver.hookFirst('shouldTransformCachedModule', [
+				{
+					ast: cachedModule.ast,
+					code: cachedModule.code,
+					id: cachedModule.id,
+					meta: cachedModule.meta,
+					moduleSideEffects: cachedModule.moduleSideEffects,
+					syntheticNamedExports: cachedModule.syntheticNamedExports
+				}
+			]))
 		) {
 			if (cachedModule.transformFiles) {
 				for (const emittedFile of cachedModule.transformFiles)

src/rollup/types.d.ts

@@ -101,7 +101,7 @@ export interface SourceDescription extends Partial<PartialNull<ModuleOptions>> {
 	map?: SourceMapInput;
 }
 
-export interface TransformModuleJSON extends Partial<PartialNull<ModuleOptions>> {
+export interface TransformModuleJSON {
 	ast?: AcornNode;
 	code: string;
 	// note if plugins use new this.cache to opt-out auto transform cache
@@ -113,7 +113,7 @@ export interface TransformModuleJSON extends Partial<PartialNull<ModuleOptions>>
 	transformDependencies: string[];
 }
 
-export interface ModuleJSON extends TransformModuleJSON {
+export interface ModuleJSON extends TransformModuleJSON, ModuleOptions {
 	ast: AcornNode;
 	dependencies: string[];
 	id: string;
@@ -242,6 +242,18 @@ export type ResolveIdHook = (
 	options: { custom?: CustomPluginOptions; isEntry: boolean }
 ) => Promise<ResolveIdResult> | ResolveIdResult;
 
+export type ShouldTransformCachedModuleHook = (
+	this: PluginContext,
+	options: {
+		ast: AcornNode;
+		code: string;
+		id: string;
+		meta: CustomPluginOptions;
+		moduleSideEffects: boolean | 'no-treeshake';
+		syntheticNamedExports: boolean | string;
+	}
+) => Promise<boolean> | boolean;
+
 export type IsExternal = (
 	source: string,
 	importer: string | undefined,
@@ -367,6 +379,7 @@ export interface PluginHooks extends OutputPluginHooks {
 	) => Promise<InputOptions | null | undefined> | InputOptions | null | undefined;
 	resolveDynamicImport: ResolveDynamicImportHook;
 	resolveId: ResolveIdHook;
+	shouldTransformCachedModule: ShouldTransformCachedModuleHook;
 	transform: TransformHook;
 	watchChange: WatchChangeHook;
 }
@@ -419,6 +432,7 @@ export type AsyncPluginHooks =
 	| 'renderStart'
 	| 'resolveDynamicImport'
 	| 'resolveId'
+	| 'shouldTransformCachedModule'
 	| 'transform'
 	| 'writeBundle'
 	| 'closeBundle';
@@ -434,7 +448,8 @@ export type FirstPluginHooks =
 	| 'resolveDynamicImport'
 	| 'resolveFileUrl'
 	| 'resolveId'
-	| 'resolveImportMeta';
+	| 'resolveImportMeta'
+	| 'shouldTransformCachedModule';
 
 export type SequentialPluginHooks =
 	| 'augmentChunkHash'

src/utils/PluginDriver.ts

@@ -55,6 +55,7 @@ const inputHookNames: {
 	options: 1,
 	resolveDynamicImport: 1,
 	resolveId: 1,
+	shouldTransformCachedModule: 1,
 	transform: 1,
 	watchChange: 1
 };

src/utils/transform.ts

@@ -165,7 +165,6 @@ export default async function transform(
 		ast,
 		code,
 		customTransformCache,
-		meta: module.info.meta,
 		originalCode,
 		originalSourcemap,
 		sourcemapChain,

test/cli/samples/watch/watch-config-early-update/_config.js

@@ -1,6 +1,6 @@
 const fs = require('fs');
 const path = require('path');
-const { atomicWriteFileSync } = require('../../../../utils');
+const { writeAndSync } = require('../../../../utils');
 
 let configFile;
 
@@ -26,7 +26,7 @@ module.exports = {
                 format: 'es'
               }
             }),
-          3000
+          2000
         );
       });`
 		);
@@ -36,7 +36,7 @@ module.exports = {
 	},
 	abortOnStderr(data) {
 		if (data === 'initial\n') {
-			atomicWriteFileSync(
+			writeAndSync(
 				configFile,
 				`
 				console.error('updated');

test/incremental/index.js

@@ -336,4 +336,54 @@ describe('incremental', () => {
 		assert.strictEqual(transformCalls, 2);
 		assert.strictEqual(moduleParsedCalls, 4); // should not be cached
 	});
+
+	it('runs shouldTransformCachedModule when using a cached module', async () => {
+		let shouldTransformCachedModuleCalls = 0;
+
+		const transformPlugin = {
+			async shouldTransformCachedModule({ ast, id, meta, ...other }) {
+				shouldTransformCachedModuleCalls++;
+				assert.strictEqual(ast.type, 'Program');
+				assert.deepStrictEqual(other, {
+					code: modules[id],
+					moduleSideEffects: true,
+					syntheticNamedExports: false
+				});
+				switch (id) {
+					case 'foo':
+						assert.deepStrictEqual(meta, { transform: { calls: 1, id } });
+						// we return promises to ensure they are awaited
+						return Promise.resolve(false);
+					case 'entry':
+						assert.deepStrictEqual(meta, { transform: { calls: 0, id } });
+						return Promise.resolve(true);
+					default:
+						throw new Error(`Unexpected id ${id}.`);
+				}
+			},
+			transform: (code, id) => {
+				return { meta: { transform: { calls: transformCalls, id } } };
+			}
+		};
+		const cache = await rollup.rollup({
+			input: 'entry',
+			plugins: [transformPlugin, plugin]
+		});
+		assert.strictEqual(shouldTransformCachedModuleCalls, 0);
+		assert.strictEqual(transformCalls, 2);
+
+		const {
+			cache: { modules: cachedModules }
+		} = await rollup.rollup({
+			input: 'entry',
+			plugins: [transformPlugin, plugin],
+			cache
+		});
+		assert.strictEqual(shouldTransformCachedModuleCalls, 2);
+		assert.strictEqual(transformCalls, 3);
+		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' } });
+	});
 });

test/utils.js

@@ -16,6 +16,7 @@ exports.assertDirectoriesAreEqual = assertDirectoriesAreEqual;
 exports.assertFilesAreEqual = assertFilesAreEqual;
 exports.assertIncludes = assertIncludes;
 exports.atomicWriteFileSync = atomicWriteFileSync;
+exports.writeAndSync = writeAndSync;
 exports.getFileNamesAndRemoveOutput = getFileNamesAndRemoveOutput;
 
 function normaliseError(error) {
@@ -232,3 +233,11 @@ function atomicWriteFileSync(filePath, contents) {
 	fs.writeFileSync(stagingPath, contents);
 	fs.renameSync(stagingPath, filePath);
 }
+
+// It appears that on MacOS, it sometimes takes long for the file system to update
+function writeAndSync(filePath, contents) {
+	const file = fs.openSync(filePath, 'w');
+	fs.writeSync(file, contents);
+	fs.fsyncSync(file);
+	fs.closeSync(file);
+}