Skip to content

Commit 1d2ea22

Browse files
lukastaegertlukastaegert
authored
Implement shouldTransformCachedModule hook (#4341)
* Create docs * Implement shouldTransformCachedModule * Flush atomic file writes * Fix formatting * Improve test reliability
1 parent da3bb43 commit 1d2ea22

File tree

9 files changed

+116
-12
lines changed

9 files changed

+116
-12
lines changed

‎docs/05-plugin-development.md‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ Notifies a plugin when watcher process closes and all open resources should be c
102102

103103
#### `load`
104104

105-
**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.
105+
**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).
106106

107107
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).
108108

@@ -116,7 +116,7 @@ You can use [`this.getModuleInfo`](guide/en/#thisgetmoduleinfo) to find out the
116116

117117
#### `moduleParsed`
118118

119-
**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).
119+
**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).
120120

121121
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.
122122

@@ -222,9 +222,19 @@ Note that while `resolveId` will be called for each import of a module and can t
222222

223223
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).
224224

225+
#### `shouldTransformCachedModule`
226+
227+
**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).
228+
229+
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`.
230+
231+
This hook can also be used to find out which modules were cached and access their cached meta information.
232+
233+
If a plugin does not return `true`, Rollup will trigger this hook for other plugins, otherwise all remaining plugins will be skipped.
234+
225235
#### `transform`
226236

227-
**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.
237+
**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.
228238

229239
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).
230240

‎docs/build-hooks.mmd‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ flowchart TB
2525
resolveid("resolveId"):::hook-first
2626
click resolveid "/guide/en/#resolveid" _parent
2727

28+
shouldtransformcachedmodule("shouldTransformCachedModule"):::hook-first
29+
click shouldtransformcachedmodule "/guide/en/#shouldtransformcachedmodule" _parent
30+
2831
transform("transform"):::hook-sequential
2932
click transform "/guide/en/#transform" _parent
3033

@@ -41,10 +44,17 @@ flowchart TB
4144

4245
resolveid
4346
--> |non-external|load
44-
--> transform
47+
--> |not cached|transform
4548
--> moduleparsed
4649
.-> |no imports|buildend
4750

51+
load
52+
--> |cached|shouldtransformcachedmodule
53+
--> |false|moduleparsed
54+
55+
shouldtransformcachedmodule
56+
--> |true|transform
57+
4858
moduleparsed
4959
--> |"each import()"|resolvedynamicimport
5060
--> |non-external|load

‎src/ModuleLoader.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,17 @@ export class ModuleLoader {
274274
if (
275275
cachedModule &&
276276
!cachedModule.customTransformCache &&
277-
cachedModule.originalCode === sourceDescription.code
277+
cachedModule.originalCode === sourceDescription.code &&
278+
!(await this.pluginDriver.hookFirst('shouldTransformCachedModule', [
279+
{
280+
ast: cachedModule.ast,
281+
code: cachedModule.code,
282+
id: cachedModule.id,
283+
meta: cachedModule.meta,
284+
moduleSideEffects: cachedModule.moduleSideEffects,
285+
syntheticNamedExports: cachedModule.syntheticNamedExports
286+
}
287+
]))
278288
) {
279289
if (cachedModule.transformFiles) {
280290
for (const emittedFile of cachedModule.transformFiles)

‎src/rollup/types.d.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ export interface SourceDescription extends Partial<PartialNull<ModuleOptions>> {
101101
map?: SourceMapInput;
102102
}
103103

104-
export interface TransformModuleJSON extends Partial<PartialNull<ModuleOptions>> {
104+
export interface TransformModuleJSON {
105105
ast?: AcornNode;
106106
code: string;
107107
// note if plugins use new this.cache to opt-out auto transform cache
@@ -113,7 +113,7 @@ export interface TransformModuleJSON extends Partial<PartialNull<ModuleOptions>>
113113
transformDependencies: string[];
114114
}
115115

116-
export interface ModuleJSON extends TransformModuleJSON {
116+
export interface ModuleJSON extends TransformModuleJSON, ModuleOptions {
117117
ast: AcornNode;
118118
dependencies: string[];
119119
id: string;
@@ -242,6 +242,18 @@ export type ResolveIdHook = (
242242
options: { custom?: CustomPluginOptions; isEntry: boolean }
243243
) => Promise<ResolveIdResult> | ResolveIdResult;
244244

245+
export type ShouldTransformCachedModuleHook = (
246+
this: PluginContext,
247+
options: {
248+
ast: AcornNode;
249+
code: string;
250+
id: string;
251+
meta: CustomPluginOptions;
252+
moduleSideEffects: boolean | 'no-treeshake';
253+
syntheticNamedExports: boolean | string;
254+
}
255+
) => Promise<boolean> | boolean;
256+
245257
export type IsExternal = (
246258
source: string,
247259
importer: string | undefined,
@@ -367,6 +379,7 @@ export interface PluginHooks extends OutputPluginHooks {
367379
) => Promise<InputOptions | null | undefined> | InputOptions | null | undefined;
368380
resolveDynamicImport: ResolveDynamicImportHook;
369381
resolveId: ResolveIdHook;
382+
shouldTransformCachedModule: ShouldTransformCachedModuleHook;
370383
transform: TransformHook;
371384
watchChange: WatchChangeHook;
372385
}
@@ -419,6 +432,7 @@ export type AsyncPluginHooks =
419432
| 'renderStart'
420433
| 'resolveDynamicImport'
421434
| 'resolveId'
435+
| 'shouldTransformCachedModule'
422436
| 'transform'
423437
| 'writeBundle'
424438
| 'closeBundle';
@@ -434,7 +448,8 @@ export type FirstPluginHooks =
434448
| 'resolveDynamicImport'
435449
| 'resolveFileUrl'
436450
| 'resolveId'
437-
| 'resolveImportMeta';
451+
| 'resolveImportMeta'
452+
| 'shouldTransformCachedModule';
438453

439454
export type SequentialPluginHooks =
440455
| 'augmentChunkHash'

‎src/utils/PluginDriver.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ const inputHookNames: {
5555
options: 1,
5656
resolveDynamicImport: 1,
5757
resolveId: 1,
58+
shouldTransformCachedModule: 1,
5859
transform: 1,
5960
watchChange: 1
6061
};

‎src/utils/transform.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,6 @@ export default async function transform(
165165
ast,
166166
code,
167167
customTransformCache,
168-
meta: module.info.meta,
169168
originalCode,
170169
originalSourcemap,
171170
sourcemapChain,

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
const fs = require('fs');
22
const path = require('path');
3-
const { atomicWriteFileSync } = require('../../../../utils');
3+
const { writeAndSync } = require('../../../../utils');
44

55
let configFile;
66

@@ -26,7 +26,7 @@ module.exports = {
2626
format: 'es'
2727
}
2828
}),
29-
3000
29+
2000
3030
);
3131
});`
3232
);
@@ -36,7 +36,7 @@ module.exports = {
3636
},
3737
abortOnStderr(data) {
3838
if (data === 'initial\n') {
39-
atomicWriteFileSync(
39+
writeAndSync(
4040
configFile,
4141
`
4242
console.error('updated');

‎test/incremental/index.js‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,4 +336,54 @@ describe('incremental', () => {
336336
assert.strictEqual(transformCalls, 2);
337337
assert.strictEqual(moduleParsedCalls, 4); // should not be cached
338338
});
339+
340+
it('runs shouldTransformCachedModule when using a cached module', async () => {
341+
let shouldTransformCachedModuleCalls = 0;
342+
343+
const transformPlugin = {
344+
async shouldTransformCachedModule({ ast, id, meta, ...other }) {
345+
shouldTransformCachedModuleCalls++;
346+
assert.strictEqual(ast.type, 'Program');
347+
assert.deepStrictEqual(other, {
348+
code: modules[id],
349+
moduleSideEffects: true,
350+
syntheticNamedExports: false
351+
});
352+
switch (id) {
353+
case 'foo':
354+
assert.deepStrictEqual(meta, { transform: { calls: 1, id } });
355+
// we return promises to ensure they are awaited
356+
return Promise.resolve(false);
357+
case 'entry':
358+
assert.deepStrictEqual(meta, { transform: { calls: 0, id } });
359+
return Promise.resolve(true);
360+
default:
361+
throw new Error(`Unexpected id ${id}.`);
362+
}
363+
},
364+
transform: (code, id) => {
365+
return { meta: { transform: { calls: transformCalls, id } } };
366+
}
367+
};
368+
const cache = await rollup.rollup({
369+
input: 'entry',
370+
plugins: [transformPlugin, plugin]
371+
});
372+
assert.strictEqual(shouldTransformCachedModuleCalls, 0);
373+
assert.strictEqual(transformCalls, 2);
374+
375+
const {
376+
cache: { modules: cachedModules }
377+
} = await rollup.rollup({
378+
input: 'entry',
379+
plugins: [transformPlugin, plugin],
380+
cache
381+
});
382+
assert.strictEqual(shouldTransformCachedModuleCalls, 2);
383+
assert.strictEqual(transformCalls, 3);
384+
assert.strictEqual(cachedModules[0].id, 'foo');
385+
assert.deepStrictEqual(cachedModules[0].meta, { transform: { calls: 1, id: 'foo' } });
386+
assert.strictEqual(cachedModules[1].id, 'entry');
387+
assert.deepStrictEqual(cachedModules[1].meta, { transform: { calls: 2, id: 'entry' } });
388+
});
339389
});

‎test/utils.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ exports.assertDirectoriesAreEqual = assertDirectoriesAreEqual;
1616
exports.assertFilesAreEqual = assertFilesAreEqual;
1717
exports.assertIncludes = assertIncludes;
1818
exports.atomicWriteFileSync = atomicWriteFileSync;
19+
exports.writeAndSync = writeAndSync;
1920
exports.getFileNamesAndRemoveOutput = getFileNamesAndRemoveOutput;
2021

2122
function normaliseError(error) {
@@ -232,3 +233,11 @@ function atomicWriteFileSync(filePath, contents) {
232233
fs.writeFileSync(stagingPath, contents);
233234
fs.renameSync(stagingPath, filePath);
234235
}
236+
237+
// It appears that on MacOS, it sometimes takes long for the file system to update
238+
function writeAndSync(filePath, contents) {
239+
const file = fs.openSync(filePath, 'w');
240+
fs.writeSync(file, contents);
241+
fs.fsyncSync(file);
242+
fs.closeSync(file);
243+
}

0 commit comments

Comments
 (0)