Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement shouldTransformCachedModule hook #4341

Merged
merged 5 commits into from Jan 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 13 additions & 3 deletions docs/05-plugin-development.md
Expand Up @@ -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).

Expand All @@ -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.

Expand Down Expand Up @@ -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).

Expand Down
12 changes: 11 additions & 1 deletion docs/build-hooks.mmd
Expand Up @@ -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

Expand All @@ -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
Expand Down
12 changes: 11 additions & 1 deletion src/ModuleLoader.ts
Expand Up @@ -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)
Expand Down
21 changes: 18 additions & 3 deletions src/rollup/types.d.ts
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -419,6 +432,7 @@ export type AsyncPluginHooks =
| 'renderStart'
| 'resolveDynamicImport'
| 'resolveId'
| 'shouldTransformCachedModule'
| 'transform'
| 'writeBundle'
| 'closeBundle';
Expand All @@ -434,7 +448,8 @@ export type FirstPluginHooks =
| 'resolveDynamicImport'
| 'resolveFileUrl'
| 'resolveId'
| 'resolveImportMeta';
| 'resolveImportMeta'
| 'shouldTransformCachedModule';

export type SequentialPluginHooks =
| 'augmentChunkHash'
Expand Down
1 change: 1 addition & 0 deletions src/utils/PluginDriver.ts
Expand Up @@ -55,6 +55,7 @@ const inputHookNames: {
options: 1,
resolveDynamicImport: 1,
resolveId: 1,
shouldTransformCachedModule: 1,
transform: 1,
watchChange: 1
};
Expand Down
1 change: 0 additions & 1 deletion src/utils/transform.ts
Expand Up @@ -165,7 +165,6 @@ export default async function transform(
ast,
code,
customTransformCache,
meta: module.info.meta,
originalCode,
originalSourcemap,
sourcemapChain,
Expand Down
6 changes: 3 additions & 3 deletions 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;

Expand All @@ -26,7 +26,7 @@ module.exports = {
format: 'es'
}
}),
3000
2000
);
});`
);
Expand All @@ -36,7 +36,7 @@ module.exports = {
},
abortOnStderr(data) {
if (data === 'initial\n') {
atomicWriteFileSync(
writeAndSync(
configFile,
`
console.error('updated');
Expand Down
50 changes: 50 additions & 0 deletions test/incremental/index.js
Expand Up @@ -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' } });
});
});
9 changes: 9 additions & 0 deletions test/utils.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}