From 8d0665dbcaa51692574a2684b6f531a6246dbb39 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 2 Aug 2022 12:35:03 +0200 Subject: [PATCH 01/15] Improve type for async hooks --- src/rollup/types.d.ts | 141 ++++++++++++++++++-------------------- src/utils/PluginDriver.ts | 61 +++++++++-------- 2 files changed, 97 insertions(+), 105 deletions(-) diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 3528f98209b..d8d12eee22e 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -244,7 +244,7 @@ export type ResolveIdHook = ( source: string, importer: string | undefined, options: { custom?: CustomPluginOptions; isEntry: boolean } -) => Promise | ResolveIdResult; +) => ResolveIdResult; export type ShouldTransformCachedModuleHook = ( this: PluginContext, @@ -257,7 +257,7 @@ export type ShouldTransformCachedModuleHook = ( resolvedSources: ResolvedIdMap; syntheticNamedExports: boolean | string; } -) => Promise | boolean; +) => boolean; export type IsExternal = ( source: string, @@ -269,9 +269,9 @@ export type IsPureModule = (id: string) => boolean | null | void; export type HasModuleSideEffects = (id: string, external: boolean) => boolean; -type LoadResult = SourceDescription | string | null | void; +export type LoadResult = SourceDescription | string | null | void; -export type LoadHook = (this: PluginContext, id: string) => Promise | LoadResult; +export type LoadHook = (this: PluginContext, id: string) => LoadResult; export interface TransformPluginContext extends PluginContext { getCombinedSourcemap: () => SourceMap; @@ -283,27 +283,22 @@ export type TransformHook = ( this: TransformPluginContext, code: string, id: string -) => Promise | TransformResult; +) => TransformResult; -export type ModuleParsedHook = (this: PluginContext, info: ModuleInfo) => Promise | void; +export type ModuleParsedHook = (this: PluginContext, info: ModuleInfo) => void; export type RenderChunkHook = ( this: PluginContext, code: string, chunk: RenderedChunk, options: NormalizedOutputOptions -) => - | Promise<{ code: string; map?: SourceMapInput } | null> - | { code: string; map?: SourceMapInput } - | string - | null - | undefined; +) => { code: string; map?: SourceMapInput } | string | null | undefined; export type ResolveDynamicImportHook = ( this: PluginContext, specifier: string | AcornNode, importer: string -) => Promise | ResolveIdResult; +) => ResolveIdResult; export type ResolveImportMetaHook = ( this: PluginContext, @@ -344,7 +339,7 @@ export type WatchChangeHook = ( this: PluginContext, id: string, change: { event: ChangeEvent } -) => Promise | void; +) => void; /** * use this type for plugin annotation @@ -371,32 +366,21 @@ export interface OutputBundleWithPlaceholders { [fileName: string]: OutputAsset | OutputChunk | FilePlaceholder; } -export interface PluginHooks extends OutputPluginHooks { - buildEnd: (this: PluginContext, err?: Error) => Promise | void; - buildStart: (this: PluginContext, options: NormalizedInputOptions) => Promise | void; - closeBundle: (this: PluginContext) => Promise | void; - closeWatcher: (this: PluginContext) => Promise | void; - load: LoadHook; - moduleParsed: ModuleParsedHook; - options: ( - this: MinimalPluginContext, - options: InputOptions - ) => Promise | InputOptions | null | void; - resolveDynamicImport: ResolveDynamicImportHook; - resolveId: ResolveIdHook; - shouldTransformCachedModule: ShouldTransformCachedModuleHook; - transform: TransformHook; - watchChange: WatchChangeHook; -} - -interface OutputPluginHooks { +export interface BasicPluginHooks { augmentChunkHash: (this: PluginContext, chunk: PreRenderedChunk) => string | void; + buildEnd: (this: PluginContext, err?: Error) => void; + buildStart: (this: PluginContext, options: NormalizedInputOptions) => void; + closeBundle: (this: PluginContext) => void; + closeWatcher: (this: PluginContext) => void; generateBundle: ( this: PluginContext, options: NormalizedOutputOptions, bundle: OutputBundle, isWrite: boolean - ) => void | Promise; + ) => void; + load: LoadHook; + moduleParsed: ModuleParsedHook; + options: (this: MinimalPluginContext, options: InputOptions) => InputOptions | null | void; outputOptions: (this: PluginContext, options: OutputOptions) => OutputOptions | null | void; renderChunk: RenderChunkHook; renderDynamicImport: ( @@ -408,45 +392,52 @@ interface OutputPluginHooks { targetModuleId: string | null; } ) => { left: string; right: string } | null | void; - renderError: (this: PluginContext, err?: Error) => Promise | void; + renderError: (this: PluginContext, err?: Error) => void; renderStart: ( this: PluginContext, outputOptions: NormalizedOutputOptions, inputOptions: NormalizedInputOptions - ) => Promise | void; + ) => void; /** @deprecated Use `resolveFileUrl` instead */ resolveAssetUrl: ResolveAssetUrlHook; + resolveDynamicImport: ResolveDynamicImportHook; resolveFileUrl: ResolveFileUrlHook; + resolveId: ResolveIdHook; resolveImportMeta: ResolveImportMetaHook; + shouldTransformCachedModule: ShouldTransformCachedModuleHook; + transform: TransformHook; + watchChange: WatchChangeHook; writeBundle: ( this: PluginContext, options: NormalizedOutputOptions, bundle: OutputBundle - ) => void | Promise; + ) => void; } -export type AsyncPluginHooks = - | 'options' - | 'buildEnd' - | 'buildStart' +export type OutputPluginHooks = + | 'augmentChunkHash' | 'generateBundle' - | 'load' - | 'moduleParsed' + | 'outputOptions' | 'renderChunk' + | 'renderDynamicImport' | 'renderError' | 'renderStart' - | 'resolveDynamicImport' - | 'resolveId' - | 'shouldTransformCachedModule' - | 'transform' - | 'writeBundle' - | 'closeBundle' - | 'closeWatcher' - | 'watchChange'; + | 'resolveAssetUrl' + | 'resolveFileUrl' + | 'resolveImportMeta' + | 'writeBundle'; -export type PluginValueHooks = 'banner' | 'footer' | 'intro' | 'outro'; +export type InputPluginHooks = Exclude; + +export type SyncPluginHooks = + | 'augmentChunkHash' + | 'outputOptions' + | 'renderDynamicImport' + | 'resolveAssetUrl' + | 'resolveFileUrl' + | 'resolveImportMeta'; -export type SyncPluginHooks = Exclude; +export type AsyncPluginHooks = Exclude; export type FirstPluginHooks = | 'load' @@ -466,36 +457,34 @@ export type SequentialPluginHooks = | 'renderChunk' | 'transform'; -export type ParallelPluginHooks = - | 'banner' - | 'buildEnd' - | 'buildStart' - | 'footer' - | 'intro' - | 'moduleParsed' - | 'outro' - | 'renderError' - | 'renderStart' - | 'writeBundle' - | 'closeBundle' - | 'closeWatcher' - | 'watchChange'; +export type ParallelPluginHooks = Exclude< + keyof BasicPluginHooks | AddonHooks, + FirstPluginHooks | SequentialPluginHooks +>; -interface OutputPluginValueHooks { - banner: AddonHook; - cacheKey: string; - footer: AddonHook; - intro: AddonHook; - outro: AddonHook; -} +export type AddonHooks = 'banner' | 'footer' | 'intro' | 'outro'; + +type MakeAsync any> = ( + ...a: Parameters +) => ReturnType | Promise>; + +export type PluginHooks = { + [K in keyof BasicPluginHooks]: K extends AsyncPluginHooks + ? MakeAsync + : BasicPluginHooks[K]; +}; -export interface Plugin extends Partial, Partial { +export interface Plugin extends Partial, Partial<{ [K in AddonHooks]: AddonHook }> { // for inter-plugin communication api?: any; + cacheKey?: string; name: string; } -export interface OutputPlugin extends Partial, Partial { +export interface OutputPlugin + extends Partial<{ [K in OutputPluginHooks]: PluginHooks[K] }>, + Partial<{ [K in AddonHooks]: AddonHook }> { + cacheKey?: string; name: string; } diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index a436bc35586..8dc33b2069e 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -1,20 +1,20 @@ import type Chunk from '../Chunk'; import type Graph from '../Graph'; import type Module from '../Module'; +import { InputPluginHooks } from '../rollup/types'; import type { AddonHookFunction, + AddonHooks, AsyncPluginHooks, + BasicPluginHooks, EmitFile, FirstPluginHooks, NormalizedInputOptions, NormalizedOutputOptions, OutputBundleWithPlaceholders, - OutputPluginHooks, ParallelPluginHooks, Plugin, PluginContext, - PluginHooks, - PluginValueHooks, SequentialPluginHooks, SerializablePluginCache, SyncPluginHooks @@ -38,12 +38,11 @@ type EnsurePromise = Promise>; * Get the type of the first argument in a function. * @example Arg0<(a: string, b: number) => void> -> string */ -type Arg0 = Parameters[0]; +type Arg0 = Parameters[0]; // This will make sure no input hook is omitted -type Subtract = T extends U ? never : T; const inputHookNames: { - [P in Subtract]: 1; + [P in InputPluginHooks]: 1; } = { buildEnd: 1, buildStart: 1, @@ -137,11 +136,11 @@ export class PluginDriver { // chains, first non-null result stops and returns hookFirst( hookName: H, - args: Parameters, + args: Parameters, replaceContext?: ReplaceContext | null, skipped?: ReadonlySet | null - ): EnsurePromise> { - let promise: EnsurePromise> = Promise.resolve(undefined as any); + ): Promise> { + let promise: Promise> = Promise.resolve(undefined as any); for (const plugin of this.plugins) { if (skipped && skipped.has(plugin)) continue; promise = promise.then(result => { @@ -155,9 +154,9 @@ export class PluginDriver { // chains synchronously, first non-null result stops and returns hookFirstSync( hookName: H, - args: Parameters, + args: Parameters, replaceContext?: ReplaceContext - ): ReturnType { + ): ReturnType { for (const plugin of this.plugins) { const result = this.runHookSync(hookName, args, plugin, replaceContext); if (result != null) return result; @@ -168,7 +167,7 @@ export class PluginDriver { // parallel, ignores returns hookParallel( hookName: H, - args: Parameters, + args: Parameters, replaceContext?: ReplaceContext ): Promise { const promises: Promise[] = []; @@ -183,10 +182,10 @@ export class PluginDriver { // chains, reduces returned value, handling the reduced value as the first hook argument hookReduceArg0( hookName: H, - [arg0, ...rest]: Parameters, + [arg0, ...rest]: Parameters, reduce: ( reduction: Arg0, - result: ResolveValue>, + result: ReturnType, plugin: Plugin ) => Arg0, replaceContext?: ReplaceContext @@ -194,7 +193,7 @@ export class PluginDriver { let promise = Promise.resolve(arg0); for (const plugin of this.plugins) { promise = promise.then(arg0 => { - const args = [arg0, ...rest] as Parameters; + const args = [arg0, ...rest] as Parameters; const hookPromise = this.runHook(hookName, args, plugin, false, replaceContext); if (!hookPromise) return arg0; return hookPromise.then(result => @@ -208,12 +207,16 @@ export class PluginDriver { // chains synchronously, reduces returned value, handling the reduced value as the first hook argument hookReduceArg0Sync( hookName: H, - [arg0, ...rest]: Parameters, - reduce: (reduction: Arg0, result: ReturnType, plugin: Plugin) => Arg0, + [arg0, ...rest]: Parameters, + reduce: ( + reduction: Arg0, + result: ReturnType, + plugin: Plugin + ) => Arg0, replaceContext?: ReplaceContext ): Arg0 { for (const plugin of this.plugins) { - const args = [arg0, ...rest] as Parameters; + const args = [arg0, ...rest] as Parameters; const result = this.runHookSync(hookName, args, plugin, replaceContext); arg0 = reduce.call(this.pluginContexts.get(plugin), arg0, result, plugin); } @@ -221,7 +224,7 @@ export class PluginDriver { } // chains, reduces returned value to type T, handling the reduced value separately. permits hooks as values. - hookReduceValue( + hookReduceValue( hookName: H, initialValue: T | Promise, args: Parameters, @@ -249,8 +252,8 @@ export class PluginDriver { hookReduceValueSync( hookName: H, initialValue: T, - args: Parameters, - reduce: (reduction: T, result: ReturnType, plugin: Plugin) => T, + args: Parameters, + reduce: (reduction: T, result: ReturnType, plugin: Plugin) => T, replaceContext?: ReplaceContext ): T { let acc = initialValue; @@ -264,7 +267,7 @@ export class PluginDriver { // chains, ignores returns hookSeq( hookName: H, - args: Parameters, + args: Parameters, replaceContext?: ReplaceContext ): Promise { let promise = Promise.resolve(); @@ -284,7 +287,7 @@ export class PluginDriver { * @param permitValues If true, values can be passed instead of functions for the plugin hook. * @param hookContext When passed, the plugin context can be overridden. */ - private runHook( + private runHook( hookName: H, args: Parameters, plugin: Plugin, @@ -293,18 +296,18 @@ export class PluginDriver { ): EnsurePromise>; private runHook( hookName: H, - args: Parameters, + args: Parameters, plugin: Plugin, permitValues: false, hookContext?: ReplaceContext | null - ): EnsurePromise>; + ): Promise>; private runHook( hookName: H, - args: Parameters, + args: Parameters, plugin: Plugin, permitValues: boolean, hookContext?: ReplaceContext | null - ): EnsurePromise> { + ): Promise> { const hook = plugin[hookName]; if (!hook) return undefined as any; @@ -364,10 +367,10 @@ export class PluginDriver { */ private runHookSync( hookName: H, - args: Parameters, + args: Parameters, plugin: Plugin, hookContext?: ReplaceContext - ): ReturnType { + ): ReturnType { const hook = plugin[hookName]; if (!hook) return undefined as any; From f8f34c6b9cc23cbe45a4311d3f2c55ad55ede014 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 3 Aug 2022 06:49:02 +0200 Subject: [PATCH 02/15] Support object hooks --- src/rollup/rollup.ts | 4 +- src/rollup/types.d.ts | 9 +++- src/utils/PluginDriver.ts | 8 +-- test/function/samples/object-hooks/_config.js | 49 +++++++++++++++++++ test/function/samples/object-hooks/main.js | 1 + 5 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 test/function/samples/object-hooks/_config.js create mode 100644 test/function/samples/object-hooks/main.js diff --git a/src/rollup/rollup.ts b/src/rollup/rollup.ts index f3809b8bf8d..4ca94b629d8 100644 --- a/src/rollup/rollup.ts +++ b/src/rollup/rollup.ts @@ -113,6 +113,7 @@ async function getInputOptions( throw new Error('You must supply an options object to rollup'); } const rawPlugins = ensureArray(rawInputOptions.plugins) as Plugin[]; + // TODO Lukas support hook ordering const { options, unsetOptions } = normalizeInputOptions( await rawPlugins.reduce(applyOptionHook(watchMode), Promise.resolve(rawInputOptions)) ); @@ -126,8 +127,9 @@ function applyOptionHook(watchMode: boolean) { plugin: Plugin ): Promise => { if (plugin.options) { + const handle = 'handle' in plugin.options ? plugin.options.handle : plugin.options; return ( - ((await plugin.options.call( + ((await handle.call( { meta: { rollupVersion, watchMode } }, await inputOptions )) as GenericConfigObject) || inputOptions diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index d8d12eee22e..039363ccb25 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -468,9 +468,16 @@ type MakeAsync any> = ( ...a: Parameters ) => ReturnType | Promise>; +type AllowObjectHook any> = T | { handle: T }; +type AllowEnforceOrderHook any> = + | T + | { enforceOrder?: 'pre' | 'post' | null; handle: T }; + export type PluginHooks = { [K in keyof BasicPluginHooks]: K extends AsyncPluginHooks - ? MakeAsync + ? K extends ParallelPluginHooks + ? AllowObjectHook> + : AllowEnforceOrderHook> : BasicPluginHooks[K]; }; diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index 8dc33b2069e..36b1eadab14 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -310,6 +310,7 @@ export class PluginDriver { ): Promise> { const hook = plugin[hookName]; if (!hook) return undefined as any; + const handle = 'handle' in hook ? hook.handle : hook; let context = this.pluginContexts.get(plugin)!; if (hookContext) { @@ -317,15 +318,16 @@ export class PluginDriver { } let action: [string, string, Parameters] | null = null; + // TODO Lukas support ordering return Promise.resolve() .then(() => { // permit values allows values to be returned instead of a functional hook - if (typeof hook !== 'function') { - if (permitValues) return hook; + if (typeof handle !== 'function') { + if (permitValues) return handle; return throwInvalidHookError(hookName, plugin.name); } // eslint-disable-next-line @typescript-eslint/ban-types - const hookResult = (hook as Function).apply(context, args); + const hookResult = (handle as Function).apply(context, args); if (!hookResult || !hookResult.then) { // short circuit for non-thenables and non-Promises diff --git a/test/function/samples/object-hooks/_config.js b/test/function/samples/object-hooks/_config.js new file mode 100644 index 00000000000..768808a1b7f --- /dev/null +++ b/test/function/samples/object-hooks/_config.js @@ -0,0 +1,49 @@ +const assert = require('assert'); + +const hooks = [ + 'buildEnd', + 'buildStart', + 'generateBundle', + 'load', + 'moduleParsed', + 'options', + 'renderChunk', + 'renderStart', + 'resolveId', + 'transform' +]; +// TODO Lukas +// closeBundle, closeWatcher, shouldTransformCachedModule, writeBundle, watchChange, renderError, resolveDynamicImport + +const plugin = { name: 'test' }; +const calledHooks = []; + +for (const hook of hooks) { + plugin[hook] = { + handle() { + calledHooks.push(hook); + } + }; +} + +module.exports = { + // solo: true, + description: 'supports using objects as hooks', + options: { + plugins: [plugin] + }, + exports() { + assert.deepStrictEqual(calledHooks, [ + 'options', + 'buildStart', + 'resolveId', + 'load', + 'transform', + 'moduleParsed', + 'buildEnd', + 'renderStart', + 'renderChunk', + 'generateBundle' + ]); + } +}; diff --git a/test/function/samples/object-hooks/main.js b/test/function/samples/object-hooks/main.js new file mode 100644 index 00000000000..cc1d88a24fa --- /dev/null +++ b/test/function/samples/object-hooks/main.js @@ -0,0 +1 @@ +assert.ok(true); From 95574f600a3f3731f14ce792238bc64bf86cf79c Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 3 Aug 2022 14:50:36 +0200 Subject: [PATCH 03/15] Support changing hook execution order --- src/rollup/rollup.ts | 4 +- src/utils/PluginContext.ts | 3 +- src/utils/PluginDriver.ts | 33 ++++++-- .../samples/enforce-plugin-order/_config.js | 83 +++++++++++++++++++ .../samples/enforce-plugin-order/dep.js | 1 + .../samples/enforce-plugin-order/main.js | 1 + test/function/samples/object-hooks/_config.js | 64 ++++++++++---- test/function/samples/object-hooks/dep.js | 1 + test/function/samples/object-hooks/main.js | 2 +- test/hooks/index.js | 62 +++++++++++++- 10 files changed, 224 insertions(+), 30 deletions(-) create mode 100644 test/function/samples/enforce-plugin-order/_config.js create mode 100644 test/function/samples/enforce-plugin-order/dep.js create mode 100644 test/function/samples/enforce-plugin-order/main.js create mode 100644 test/function/samples/object-hooks/dep.js diff --git a/src/rollup/rollup.ts b/src/rollup/rollup.ts index 4ca94b629d8..17e28ec7308 100644 --- a/src/rollup/rollup.ts +++ b/src/rollup/rollup.ts @@ -1,6 +1,7 @@ import { version as rollupVersion } from 'package.json'; import Bundle from '../Bundle'; import Graph from '../Graph'; +import { getSortedPlugins } from '../utils/PluginDriver'; import type { PluginDriver } from '../utils/PluginDriver'; import { ensureArray } from '../utils/ensureArray'; import { errAlreadyClosed, errCannotEmitFromOptionsHook, error } from '../utils/error'; @@ -112,8 +113,7 @@ async function getInputOptions( if (!rawInputOptions) { throw new Error('You must supply an options object to rollup'); } - const rawPlugins = ensureArray(rawInputOptions.plugins) as Plugin[]; - // TODO Lukas support hook ordering + const rawPlugins = getSortedPlugins('options', ensureArray(rawInputOptions.plugins) as Plugin[]); const { options, unsetOptions } = normalizeInputOptions( await rawPlugins.reduce(applyOptionHook(watchMode), Promise.resolve(rawInputOptions)) ); diff --git a/src/utils/PluginContext.ts b/src/utils/PluginContext.ts index f1ca5bc4dd9..88d06332889 100644 --- a/src/utils/PluginContext.ts +++ b/src/utils/PluginContext.ts @@ -80,7 +80,7 @@ export function getPluginContext( cacheInstance = getCacheForUncacheablePlugin(plugin.name); } - const context: PluginContext = { + return { addWatchFile(id) { if (graph.phase >= BuildPhase.GENERATE) { return this.error(errInvalidRollupPhaseForAddWatchFile()); @@ -193,5 +193,4 @@ export function getPluginContext( options.onwarn(warning); } }; - return context; } diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index 36b1eadab14..f29e42fd2c0 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -141,7 +141,7 @@ export class PluginDriver { skipped?: ReadonlySet | null ): Promise> { let promise: Promise> = Promise.resolve(undefined as any); - for (const plugin of this.plugins) { + for (const plugin of getSortedPlugins(hookName, this.plugins)) { if (skipped && skipped.has(plugin)) continue; promise = promise.then(result => { if (result != null) return result; @@ -191,7 +191,7 @@ export class PluginDriver { replaceContext?: ReplaceContext ): Promise> { let promise = Promise.resolve(arg0); - for (const plugin of this.plugins) { + for (const plugin of getSortedPlugins(hookName, this.plugins)) { promise = promise.then(arg0 => { const args = [arg0, ...rest] as Parameters; const hookPromise = this.runHook(hookName, args, plugin, false, replaceContext); @@ -271,7 +271,7 @@ export class PluginDriver { replaceContext?: ReplaceContext ): Promise { let promise = Promise.resolve(); - for (const plugin of this.plugins) { + for (const plugin of getSortedPlugins(hookName, this.plugins)) { promise = promise.then( () => this.runHook(hookName, args, plugin, false, replaceContext) as Promise ); @@ -310,7 +310,8 @@ export class PluginDriver { ): Promise> { const hook = plugin[hookName]; if (!hook) return undefined as any; - const handle = 'handle' in hook ? hook.handle : hook; + const handle = + typeof hook === 'object' && typeof hook.handle === 'function' ? hook.handle : hook; let context = this.pluginContexts.get(plugin)!; if (hookContext) { @@ -318,7 +319,6 @@ export class PluginDriver { } let action: [string, string, Parameters] | null = null; - // TODO Lukas support ordering return Promise.resolve() .then(() => { // permit values allows values to be returned instead of a functional hook @@ -393,3 +393,26 @@ export class PluginDriver { } } } + +export function getSortedPlugins(hookName: AsyncPluginHooks, plugins: readonly Plugin[]): Plugin[] { + const pre: Plugin[] = []; + const normal: Plugin[] = []; + const post: Plugin[] = []; + for (const plugin of plugins) { + const hook = plugin[hookName]; + if (hook) { + if (typeof hook === 'object' && 'enforceOrder' in hook) { + if (hook.enforceOrder === 'pre') { + pre.push(plugin); + continue; + } + if (hook.enforceOrder === 'post') { + post.push(plugin); + continue; + } + } + normal.push(plugin); + } + } + return [...pre, ...normal, ...post]; +} diff --git a/test/function/samples/enforce-plugin-order/_config.js b/test/function/samples/enforce-plugin-order/_config.js new file mode 100644 index 00000000000..6367b19781d --- /dev/null +++ b/test/function/samples/enforce-plugin-order/_config.js @@ -0,0 +1,83 @@ +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const acorn = require('acorn'); + +const ID_MAIN = path.join(__dirname, 'main.js'); +const code = fs.readFileSync(ID_MAIN, 'utf8'); + +const hooks = [ + 'generateBundle', + 'load', + 'options', + 'renderChunk', + 'resolveDynamicImport', + 'resolveId', + 'shouldTransformCachedModule', + 'transform' +]; + +const calledHooks = {}; + +for (const hook of hooks) { + calledHooks[hook] = []; +} + +const plugins = []; + +function addPlugin(enforceOrder) { + const name = `${enforceOrder}-${plugins.length + 1}`; + const plugin = { name }; + for (const hook of hooks) { + plugin[hook] = { + enforceOrder, + handle() { + if (!calledHooks[hook].includes(name)) { + calledHooks[hook].push(name); + } + } + }; + } + plugins.push(plugin); +} + +addPlugin(null); +addPlugin('pre'); +addPlugin('post'); +addPlugin('post'); +addPlugin('pre'); +addPlugin(undefined); + +module.exports = { + description: 'allows to enforce plugin order', + options: { + plugins, + cache: { + modules: [ + { + id: ID_MAIN, + ast: acorn.parse(code, { + ecmaVersion: 2020, + sourceType: 'module' + }), + code, + dependencies: [], + dynamicDependencies: [], + originalCode: code, + resolvedIds: {}, + sourcemapChain: [], + transformDependencies: [] + } + ] + } + }, + exports() { + for (const hook of hooks) { + assert.deepStrictEqual( + calledHooks[hook], + ['pre-2', 'pre-5', 'null-1', 'undefined-6', 'post-3', 'post-4'], + hook + ); + } + } +}; diff --git a/test/function/samples/enforce-plugin-order/dep.js b/test/function/samples/enforce-plugin-order/dep.js new file mode 100644 index 00000000000..cc1d88a24fa --- /dev/null +++ b/test/function/samples/enforce-plugin-order/dep.js @@ -0,0 +1 @@ +assert.ok(true); diff --git a/test/function/samples/enforce-plugin-order/main.js b/test/function/samples/enforce-plugin-order/main.js new file mode 100644 index 00000000000..c02cf40e2fc --- /dev/null +++ b/test/function/samples/enforce-plugin-order/main.js @@ -0,0 +1 @@ +import('./dep.js'); diff --git a/test/function/samples/object-hooks/_config.js b/test/function/samples/object-hooks/_config.js index 768808a1b7f..d04876d48b3 100644 --- a/test/function/samples/object-hooks/_config.js +++ b/test/function/samples/object-hooks/_config.js @@ -1,4 +1,10 @@ const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const acorn = require('acorn'); + +const ID_MAIN = path.join(__dirname, 'main.js'); +const code = fs.readFileSync(ID_MAIN, 'utf8'); const hooks = [ 'buildEnd', @@ -9,41 +15,63 @@ const hooks = [ 'options', 'renderChunk', 'renderStart', + 'resolveDynamicImport', 'resolveId', + 'shouldTransformCachedModule', 'transform' ]; -// TODO Lukas -// closeBundle, closeWatcher, shouldTransformCachedModule, writeBundle, watchChange, renderError, resolveDynamicImport const plugin = { name: 'test' }; -const calledHooks = []; +const calledHooks = new Set(); for (const hook of hooks) { plugin[hook] = { handle() { - calledHooks.push(hook); + calledHooks.add(hook); } }; } module.exports = { - // solo: true, description: 'supports using objects as hooks', options: { - plugins: [plugin] + plugins: [plugin], + cache: { + modules: [ + { + id: ID_MAIN, + ast: acorn.parse(code, { + ecmaVersion: 2020, + sourceType: 'module' + }), + code, + dependencies: [], + dynamicDependencies: [], + originalCode: code, + resolvedIds: {}, + sourcemapChain: [], + transformDependencies: [] + } + ] + } }, exports() { - assert.deepStrictEqual(calledHooks, [ - 'options', - 'buildStart', - 'resolveId', - 'load', - 'transform', - 'moduleParsed', - 'buildEnd', - 'renderStart', - 'renderChunk', - 'generateBundle' - ]); + assert.deepStrictEqual( + [...calledHooks], + [ + 'options', + 'buildStart', + 'resolveId', + 'load', + 'shouldTransformCachedModule', + 'resolveDynamicImport', + 'moduleParsed', + 'transform', + 'buildEnd', + 'renderStart', + 'renderChunk', + 'generateBundle' + ] + ); } }; diff --git a/test/function/samples/object-hooks/dep.js b/test/function/samples/object-hooks/dep.js new file mode 100644 index 00000000000..cc1d88a24fa --- /dev/null +++ b/test/function/samples/object-hooks/dep.js @@ -0,0 +1 @@ +assert.ok(true); diff --git a/test/function/samples/object-hooks/main.js b/test/function/samples/object-hooks/main.js index cc1d88a24fa..c02cf40e2fc 100644 --- a/test/function/samples/object-hooks/main.js +++ b/test/function/samples/object-hooks/main.js @@ -1 +1 @@ -assert.ok(true); +import('./dep.js'); diff --git a/test/hooks/index.js b/test/hooks/index.js index b776bb9e808..95430d696fc 100644 --- a/test/hooks/index.js +++ b/test/hooks/index.js @@ -1,9 +1,9 @@ const assert = require('assert'); const { readdirSync } = require('fs'); const path = require('path'); -const { removeSync } = require('fs-extra'); +const { removeSync, outputFileSync } = require('fs-extra'); const rollup = require('../../dist/rollup.js'); -const { loader } = require('../utils.js'); +const { loader, wait } = require('../utils.js'); const TEMP_DIR = path.join(__dirname, 'tmp'); @@ -1385,5 +1385,63 @@ describe('hooks', () => { assert.strictEqual(output.source, 'hello world'); }); }); + + it('supports object hooks in watch mode', () => { + const hooks = ['closeBundle', 'closeWatcher', 'renderError', 'watchChange', 'writeBundle']; + let first = true; + const plugin = { + name: 'test', + renderChunk() { + if (first) { + first = false; + throw new Error('Expected render error'); + } + } + }; + const calledHooks = []; + for (const hook of hooks) { + plugin[hook] = { + handle() { + calledHooks.push(hook); + } + }; + } + const ID_MAIN = path.join(TEMP_DIR, 'main.js'); + outputFileSync(ID_MAIN, 'console.log(42);'); + + const watcher = rollup.watch({ + input: ID_MAIN, + output: { + format: 'es', + dir: path.join(TEMP_DIR, 'out') + }, + plugins: [plugin] + }); + + return new Promise((resolve, reject) => { + watcher.on('event', async event => { + if (event.code === 'ERROR') { + if (event.error.message !== 'Expected render error') { + reject(event.error); + } + await wait(200); + outputFileSync(ID_MAIN, 'console.log(43);'); + } else if (event.code === 'BUNDLE_END') { + await event.result.close(); + resolve(); + } + }); + }).finally(async () => { + await watcher.close(); + removeSync(TEMP_DIR); + assert.deepStrictEqual(calledHooks, [ + 'renderError', + 'watchChange', + 'writeBundle', + 'closeBundle', + 'closeWatcher' + ]); + }); + }); }); }); From a2a729fe084179b0040b75506f21260355dfe92a Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Thu, 4 Aug 2022 06:45:04 +0200 Subject: [PATCH 04/15] Add caching --- src/utils/PluginDriver.ts | 18 ++++-- test/hooks/index.js | 120 ++++++++++++++++++-------------------- 2 files changed, 68 insertions(+), 70 deletions(-) diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index f29e42fd2c0..9ae3658b6a0 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -22,6 +22,7 @@ import type { import { FileEmitter } from './FileEmitter'; import { getPluginContext } from './PluginContext'; import { errInputHookInOutputPlugin, error } from './error'; +import { getOrCreate } from './getOrCreate'; import { throwPluginError, warnDeprecatedHooks } from './pluginUtils'; /** @@ -81,20 +82,19 @@ export class PluginDriver { ) => void; private readonly fileEmitter: FileEmitter; - private readonly pluginCache: Record | undefined; private readonly pluginContexts: ReadonlyMap; private readonly plugins: readonly Plugin[]; + private readonly sortedPlugins = new Map(); private readonly unfulfilledActions = new Set(); constructor( private readonly graph: Graph, private readonly options: NormalizedInputOptions, userPlugins: readonly Plugin[], - pluginCache: Record | undefined, + private readonly pluginCache: Record | undefined, basePluginDriver?: PluginDriver ) { warnDeprecatedHooks(userPlugins, options); - this.pluginCache = pluginCache; this.fileEmitter = new FileEmitter( graph, options, @@ -141,7 +141,7 @@ export class PluginDriver { skipped?: ReadonlySet | null ): Promise> { let promise: Promise> = Promise.resolve(undefined as any); - for (const plugin of getSortedPlugins(hookName, this.plugins)) { + for (const plugin of this.getSortedPlugins(hookName)) { if (skipped && skipped.has(plugin)) continue; promise = promise.then(result => { if (result != null) return result; @@ -191,7 +191,7 @@ export class PluginDriver { replaceContext?: ReplaceContext ): Promise> { let promise = Promise.resolve(arg0); - for (const plugin of getSortedPlugins(hookName, this.plugins)) { + for (const plugin of this.getSortedPlugins(hookName)) { promise = promise.then(arg0 => { const args = [arg0, ...rest] as Parameters; const hookPromise = this.runHook(hookName, args, plugin, false, replaceContext); @@ -271,7 +271,7 @@ export class PluginDriver { replaceContext?: ReplaceContext ): Promise { let promise = Promise.resolve(); - for (const plugin of getSortedPlugins(hookName, this.plugins)) { + for (const plugin of this.getSortedPlugins(hookName)) { promise = promise.then( () => this.runHook(hookName, args, plugin, false, replaceContext) as Promise ); @@ -279,6 +279,12 @@ export class PluginDriver { return promise; } + private getSortedPlugins(hookName: AsyncPluginHooks): Plugin[] { + return getOrCreate(this.sortedPlugins, hookName, () => + getSortedPlugins(hookName, this.plugins) + ); + } + /** * Run an async plugin hook and return the result. * @param hookName Name of the plugin hook. Must be either in `PluginHooks` or `OutputPluginValueHooks`. diff --git a/test/hooks/index.js b/test/hooks/index.js index 95430d696fc..5a6435a9a7b 100644 --- a/test/hooks/index.js +++ b/test/hooks/index.js @@ -1,43 +1,38 @@ const assert = require('assert'); -const { readdirSync } = require('fs'); const path = require('path'); -const { removeSync, outputFileSync } = require('fs-extra'); +const { outputFile, readdir, remove } = require('fs-extra'); const rollup = require('../../dist/rollup.js'); const { loader, wait } = require('../utils.js'); const TEMP_DIR = path.join(__dirname, 'tmp'); describe('hooks', () => { - it('allows to replace file with dir in the outputOptions hook', () => - rollup - .rollup({ - input: 'input', - treeshake: false, - plugins: [ - loader({ - input: `console.log('input');import('other');`, - other: `console.log('other');` - }), - { - outputOptions(options) { - const newOptions = { ...options, dir: TEMP_DIR, chunkFileNames: 'chunk.js' }; - delete newOptions.file; - return newOptions; - } + it('allows to replace file with dir in the outputOptions hook', async () => { + const bundle = await rollup.rollup({ + input: 'input', + treeshake: false, + plugins: [ + loader({ + input: `console.log('input');import('other');`, + other: `console.log('other');` + }), + { + outputOptions(options) { + const newOptions = { ...options, dir: TEMP_DIR, chunkFileNames: 'chunk.js' }; + delete newOptions.file; + return newOptions; } - ] - }) - .then(bundle => - bundle.write({ - file: path.join(TEMP_DIR, 'bundle.js'), - format: 'es' - }) - ) - .then(() => { - const fileNames = readdirSync(TEMP_DIR).sort(); - assert.deepStrictEqual(fileNames, ['chunk.js', 'input.js']); - return removeSync(TEMP_DIR); - })); + } + ] + }); + await bundle.write({ + file: path.join(TEMP_DIR, 'bundle.js'), + format: 'es' + }); + const fileNames = (await readdir(TEMP_DIR)).sort(); + assert.deepStrictEqual(fileNames, ['chunk.js', 'input.js']); + await remove(TEMP_DIR); + }); it('supports buildStart and buildEnd hooks', () => { let buildStartCnt = 0; @@ -545,38 +540,35 @@ describe('hooks', () => { }) .then(bundle => bundle.generate({ format: 'es' }))); - it('supports writeBundle hook', () => { + it('supports writeBundle hook', async () => { const file = path.join(TEMP_DIR, 'bundle.js'); - let bundle; + let generatedBundle; let callCount = 0; - return rollup - .rollup({ - input: 'input', - plugins: [ - loader({ - input: `export { a as default } from 'dep';`, - dep: `export var a = 1; export var b = 2;` - }), - { - generateBundle(options, outputBundle, isWrite) { - bundle = outputBundle; - assert.strictEqual(isWrite, true); - } - }, - { - writeBundle(options, outputBundle) { - assert.deepStrictEqual(options.file, file); - assert.deepStrictEqual(outputBundle, bundle); - callCount++; - } + const bundle = await rollup.rollup({ + input: 'input', + plugins: [ + loader({ + input: `export { a as default } from 'dep';`, + dep: `export var a = 1; export var b = 2;` + }), + { + generateBundle(options, outputBundle, isWrite) { + generatedBundle = outputBundle; + assert.strictEqual(isWrite, true); } - ] - }) - .then(bundle => bundle.write({ format: 'es', file })) - .then(() => { - assert.strictEqual(callCount, 1); - return removeSync(TEMP_DIR); - }); + }, + { + writeBundle(options, outputBundle) { + assert.deepStrictEqual(options.file, file); + assert.deepStrictEqual(outputBundle, generatedBundle); + callCount++; + } + } + ] + }); + await bundle.write({ format: 'es', file }); + assert.strictEqual(callCount, 1); + await remove(TEMP_DIR); }); it('supports this.cache for plugins', () => @@ -1386,7 +1378,7 @@ describe('hooks', () => { }); }); - it('supports object hooks in watch mode', () => { + it('supports object hooks in watch mode', async () => { const hooks = ['closeBundle', 'closeWatcher', 'renderError', 'watchChange', 'writeBundle']; let first = true; const plugin = { @@ -1407,7 +1399,7 @@ describe('hooks', () => { }; } const ID_MAIN = path.join(TEMP_DIR, 'main.js'); - outputFileSync(ID_MAIN, 'console.log(42);'); + await outputFile(ID_MAIN, 'console.log(42);'); const watcher = rollup.watch({ input: ID_MAIN, @@ -1425,7 +1417,7 @@ describe('hooks', () => { reject(event.error); } await wait(200); - outputFileSync(ID_MAIN, 'console.log(43);'); + await outputFile(ID_MAIN, 'console.log(43);'); } else if (event.code === 'BUNDLE_END') { await event.result.close(); resolve(); @@ -1433,7 +1425,7 @@ describe('hooks', () => { }); }).finally(async () => { await watcher.close(); - removeSync(TEMP_DIR); + await remove(TEMP_DIR); assert.deepStrictEqual(calledHooks, [ 'renderError', 'watchChange', From 3bc931c6e892b7f2eb5ae757bfc4975a8f02c5d0 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Fri, 5 Aug 2022 06:52:56 +0200 Subject: [PATCH 05/15] Change naming to "handler/order" and include parallel hooks --- src/rollup/rollup.ts | 4 +- src/rollup/types.d.ts | 7 +- src/utils/PluginDriver.ts | 19 ++--- .../samples/enforce-plugin-order/_config.js | 29 +++---- test/function/samples/object-hooks/_config.js | 77 ------------------- test/function/samples/object-hooks/dep.js | 1 - test/function/samples/object-hooks/main.js | 1 - test/hooks/index.js | 68 ++++++++++------ 8 files changed, 74 insertions(+), 132 deletions(-) delete mode 100644 test/function/samples/object-hooks/_config.js delete mode 100644 test/function/samples/object-hooks/dep.js delete mode 100644 test/function/samples/object-hooks/main.js diff --git a/src/rollup/rollup.ts b/src/rollup/rollup.ts index 17e28ec7308..c4f99b695b6 100644 --- a/src/rollup/rollup.ts +++ b/src/rollup/rollup.ts @@ -127,9 +127,9 @@ function applyOptionHook(watchMode: boolean) { plugin: Plugin ): Promise => { if (plugin.options) { - const handle = 'handle' in plugin.options ? plugin.options.handle : plugin.options; + const handler = 'handler' in plugin.options ? plugin.options.handler : plugin.options; return ( - ((await handle.call( + ((await handler.call( { meta: { rollupVersion, watchMode } }, await inputOptions )) as GenericConfigObject) || inputOptions diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 039363ccb25..2c197a0b7cd 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -468,16 +468,13 @@ type MakeAsync any> = ( ...a: Parameters ) => ReturnType | Promise>; -type AllowObjectHook any> = T | { handle: T }; type AllowEnforceOrderHook any> = | T - | { enforceOrder?: 'pre' | 'post' | null; handle: T }; + | { handler: T; order?: 'pre' | 'post' | null }; export type PluginHooks = { [K in keyof BasicPluginHooks]: K extends AsyncPluginHooks - ? K extends ParallelPluginHooks - ? AllowObjectHook> - : AllowEnforceOrderHook> + ? AllowEnforceOrderHook> : BasicPluginHooks[K]; }; diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index 9ae3658b6a0..b335c0a1a1a 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -171,7 +171,7 @@ export class PluginDriver { replaceContext?: ReplaceContext ): Promise { const promises: Promise[] = []; - for (const plugin of this.plugins) { + for (const plugin of this.getSortedPlugins(hookName)) { const hookPromise = this.runHook(hookName, args, plugin, false, replaceContext); if (!hookPromise) continue; promises.push(hookPromise); @@ -316,8 +316,8 @@ export class PluginDriver { ): Promise> { const hook = plugin[hookName]; if (!hook) return undefined as any; - const handle = - typeof hook === 'object' && typeof hook.handle === 'function' ? hook.handle : hook; + const handler = + typeof hook === 'object' && typeof hook.handler === 'function' ? hook.handler : hook; let context = this.pluginContexts.get(plugin)!; if (hookContext) { @@ -328,12 +328,12 @@ export class PluginDriver { return Promise.resolve() .then(() => { // permit values allows values to be returned instead of a functional hook - if (typeof handle !== 'function') { - if (permitValues) return handle; + if (typeof handler !== 'function') { + if (permitValues) return handler; return throwInvalidHookError(hookName, plugin.name); } // eslint-disable-next-line @typescript-eslint/ban-types - const hookResult = (handle as Function).apply(context, args); + const hookResult = (handler as Function).apply(context, args); if (!hookResult || !hookResult.then) { // short circuit for non-thenables and non-Promises @@ -400,6 +400,7 @@ export class PluginDriver { } } +// TODO Lukas we can do plugin hook validation in this function export function getSortedPlugins(hookName: AsyncPluginHooks, plugins: readonly Plugin[]): Plugin[] { const pre: Plugin[] = []; const normal: Plugin[] = []; @@ -407,12 +408,12 @@ export function getSortedPlugins(hookName: AsyncPluginHooks, plugins: readonly P for (const plugin of plugins) { const hook = plugin[hookName]; if (hook) { - if (typeof hook === 'object' && 'enforceOrder' in hook) { - if (hook.enforceOrder === 'pre') { + if (typeof hook === 'object' && 'order' in hook) { + if (hook.order === 'pre') { pre.push(plugin); continue; } - if (hook.enforceOrder === 'post') { + if (hook.order === 'post') { post.push(plugin); continue; } diff --git a/test/function/samples/enforce-plugin-order/_config.js b/test/function/samples/enforce-plugin-order/_config.js index 6367b19781d..819555ad916 100644 --- a/test/function/samples/enforce-plugin-order/_config.js +++ b/test/function/samples/enforce-plugin-order/_config.js @@ -7,10 +7,14 @@ const ID_MAIN = path.join(__dirname, 'main.js'); const code = fs.readFileSync(ID_MAIN, 'utf8'); const hooks = [ + 'buildEnd', + 'buildStart', 'generateBundle', 'load', + 'moduleParsed', 'options', 'renderChunk', + 'renderStart', 'resolveDynamicImport', 'resolveId', 'shouldTransformCachedModule', @@ -18,20 +22,24 @@ const hooks = [ ]; const calledHooks = {}; - for (const hook of hooks) { calledHooks[hook] = []; } const plugins = []; - -function addPlugin(enforceOrder) { - const name = `${enforceOrder}-${plugins.length + 1}`; +addPlugin(null); +addPlugin('pre'); +addPlugin('post'); +addPlugin('post'); +addPlugin('pre'); +addPlugin(undefined); +function addPlugin(order) { + const name = `${order}-${plugins.length + 1}`; const plugin = { name }; for (const hook of hooks) { plugin[hook] = { - enforceOrder, - handle() { + order, + handler() { if (!calledHooks[hook].includes(name)) { calledHooks[hook].push(name); } @@ -41,15 +49,8 @@ function addPlugin(enforceOrder) { plugins.push(plugin); } -addPlugin(null); -addPlugin('pre'); -addPlugin('post'); -addPlugin('post'); -addPlugin('pre'); -addPlugin(undefined); - module.exports = { - description: 'allows to enforce plugin order', + description: 'allows to enforce plugin hook order', options: { plugins, cache: { diff --git a/test/function/samples/object-hooks/_config.js b/test/function/samples/object-hooks/_config.js deleted file mode 100644 index d04876d48b3..00000000000 --- a/test/function/samples/object-hooks/_config.js +++ /dev/null @@ -1,77 +0,0 @@ -const assert = require('assert'); -const fs = require('fs'); -const path = require('path'); -const acorn = require('acorn'); - -const ID_MAIN = path.join(__dirname, 'main.js'); -const code = fs.readFileSync(ID_MAIN, 'utf8'); - -const hooks = [ - 'buildEnd', - 'buildStart', - 'generateBundle', - 'load', - 'moduleParsed', - 'options', - 'renderChunk', - 'renderStart', - 'resolveDynamicImport', - 'resolveId', - 'shouldTransformCachedModule', - 'transform' -]; - -const plugin = { name: 'test' }; -const calledHooks = new Set(); - -for (const hook of hooks) { - plugin[hook] = { - handle() { - calledHooks.add(hook); - } - }; -} - -module.exports = { - description: 'supports using objects as hooks', - options: { - plugins: [plugin], - cache: { - modules: [ - { - id: ID_MAIN, - ast: acorn.parse(code, { - ecmaVersion: 2020, - sourceType: 'module' - }), - code, - dependencies: [], - dynamicDependencies: [], - originalCode: code, - resolvedIds: {}, - sourcemapChain: [], - transformDependencies: [] - } - ] - } - }, - exports() { - assert.deepStrictEqual( - [...calledHooks], - [ - 'options', - 'buildStart', - 'resolveId', - 'load', - 'shouldTransformCachedModule', - 'resolveDynamicImport', - 'moduleParsed', - 'transform', - 'buildEnd', - 'renderStart', - 'renderChunk', - 'generateBundle' - ] - ); - } -}; diff --git a/test/function/samples/object-hooks/dep.js b/test/function/samples/object-hooks/dep.js deleted file mode 100644 index cc1d88a24fa..00000000000 --- a/test/function/samples/object-hooks/dep.js +++ /dev/null @@ -1 +0,0 @@ -assert.ok(true); diff --git a/test/function/samples/object-hooks/main.js b/test/function/samples/object-hooks/main.js deleted file mode 100644 index c02cf40e2fc..00000000000 --- a/test/function/samples/object-hooks/main.js +++ /dev/null @@ -1 +0,0 @@ -import('./dep.js'); diff --git a/test/hooks/index.js b/test/hooks/index.js index 5a6435a9a7b..7ae444216b0 100644 --- a/test/hooks/index.js +++ b/test/hooks/index.js @@ -1378,26 +1378,48 @@ describe('hooks', () => { }); }); - it('supports object hooks in watch mode', async () => { + it('allows to enforce plugin hook order in watch mode', async () => { const hooks = ['closeBundle', 'closeWatcher', 'renderError', 'watchChange', 'writeBundle']; + + const calledHooks = {}; + for (const hook of hooks) { + calledHooks[hook] = []; + } + let first = true; - const plugin = { - name: 'test', - renderChunk() { - if (first) { - first = false; - throw new Error('Expected render error'); + const plugins = [ + { + name: 'render-error', + renderChunk() { + if (first) { + first = false; + throw new Error('Expected render error'); + } } } - }; - const calledHooks = []; - for (const hook of hooks) { - plugin[hook] = { - handle() { - calledHooks.push(hook); - } - }; + ]; + addPlugin(null); + addPlugin('pre'); + addPlugin('post'); + addPlugin('post'); + addPlugin('pre'); + addPlugin(undefined); + function addPlugin(order) { + const name = `${order}-${plugins.length}`; + const plugin = { name }; + for (const hook of hooks) { + plugin[hook] = { + order, + handler() { + if (!calledHooks[hook].includes(name)) { + calledHooks[hook].push(name); + } + } + }; + } + plugins.push(plugin); } + const ID_MAIN = path.join(TEMP_DIR, 'main.js'); await outputFile(ID_MAIN, 'console.log(42);'); @@ -1407,7 +1429,7 @@ describe('hooks', () => { format: 'es', dir: path.join(TEMP_DIR, 'out') }, - plugins: [plugin] + plugins }); return new Promise((resolve, reject) => { @@ -1426,13 +1448,13 @@ describe('hooks', () => { }).finally(async () => { await watcher.close(); await remove(TEMP_DIR); - assert.deepStrictEqual(calledHooks, [ - 'renderError', - 'watchChange', - 'writeBundle', - 'closeBundle', - 'closeWatcher' - ]); + for (const hook of hooks) { + assert.deepStrictEqual( + calledHooks[hook], + ['pre-2', 'pre-5', 'null-1', 'undefined-6', 'post-3', 'post-4'], + hook + ); + } }); }); }); From c031c04997afa5080bfafab0d21b82b4f44701c5 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Fri, 5 Aug 2022 12:03:02 +0200 Subject: [PATCH 06/15] Support object form for banner/footer/intro/outro --- src/rollup/types.d.ts | 28 ++++--- src/utils/PluginDriver.ts | 63 +++++++-------- .../samples/enforce-addon-order/_config.js | 40 ++++++++++ .../samples/enforce-addon-order/_expected.js | 77 +++++++++++++++++++ test/form/samples/enforce-addon-order/main.js | 1 + 5 files changed, 164 insertions(+), 45 deletions(-) create mode 100644 test/form/samples/enforce-addon-order/_config.js create mode 100644 test/form/samples/enforce-addon-order/_expected.js create mode 100644 test/form/samples/enforce-addon-order/main.js diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 2c197a0b7cd..8ae2e6a0511 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -366,7 +366,7 @@ export interface OutputBundleWithPlaceholders { [fileName: string]: OutputAsset | OutputChunk | FilePlaceholder; } -export interface BasicPluginHooks { +export interface FunctionPluginHooks { augmentChunkHash: (this: PluginContext, chunk: PreRenderedChunk) => string | void; buildEnd: (this: PluginContext, err?: Error) => void; buildStart: (this: PluginContext, options: NormalizedInputOptions) => void; @@ -427,7 +427,7 @@ export type OutputPluginHooks = | 'resolveImportMeta' | 'writeBundle'; -export type InputPluginHooks = Exclude; +export type InputPluginHooks = Exclude; export type SyncPluginHooks = | 'augmentChunkHash' @@ -437,7 +437,7 @@ export type SyncPluginHooks = | 'resolveFileUrl' | 'resolveImportMeta'; -export type AsyncPluginHooks = Exclude; +export type AsyncPluginHooks = Exclude; export type FirstPluginHooks = | 'load' @@ -458,7 +458,7 @@ export type SequentialPluginHooks = | 'transform'; export type ParallelPluginHooks = Exclude< - keyof BasicPluginHooks | AddonHooks, + keyof FunctionPluginHooks | AddonHooks, FirstPluginHooks | SequentialPluginHooks >; @@ -473,25 +473,23 @@ type AllowEnforceOrderHook any> = | { handler: T; order?: 'pre' | 'post' | null }; export type PluginHooks = { - [K in keyof BasicPluginHooks]: K extends AsyncPluginHooks - ? AllowEnforceOrderHook> - : BasicPluginHooks[K]; + [K in keyof FunctionPluginHooks]: K extends AsyncPluginHooks + ? AllowEnforceOrderHook> + : FunctionPluginHooks[K]; }; -export interface Plugin extends Partial, Partial<{ [K in AddonHooks]: AddonHook }> { - // for inter-plugin communication - api?: any; - cacheKey?: string; - name: string; -} - export interface OutputPlugin extends Partial<{ [K in OutputPluginHooks]: PluginHooks[K] }>, - Partial<{ [K in AddonHooks]: AddonHook }> { + Partial<{ [K in AddonHooks]: AllowEnforceOrderHook }> { cacheKey?: string; name: string; } +export interface Plugin extends OutputPlugin, Partial { + // for inter-plugin communication + api?: any; +} + type TreeshakingPreset = 'smallest' | 'safest' | 'recommended'; export interface NormalizedTreeshakingOptions { diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index b335c0a1a1a..a99e7528fca 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -6,9 +6,9 @@ import type { AddonHookFunction, AddonHooks, AsyncPluginHooks, - BasicPluginHooks, EmitFile, FirstPluginHooks, + FunctionPluginHooks, NormalizedInputOptions, NormalizedOutputOptions, OutputBundleWithPlaceholders, @@ -39,7 +39,7 @@ type EnsurePromise = Promise>; * Get the type of the first argument in a function. * @example Arg0<(a: string, b: number) => void> -> string */ -type Arg0 = Parameters[0]; +type Arg0 = Parameters[0]; // This will make sure no input hook is omitted const inputHookNames: { @@ -136,11 +136,11 @@ export class PluginDriver { // chains, first non-null result stops and returns hookFirst( hookName: H, - args: Parameters, + args: Parameters, replaceContext?: ReplaceContext | null, skipped?: ReadonlySet | null - ): Promise> { - let promise: Promise> = Promise.resolve(undefined as any); + ): Promise> { + let promise: Promise> = Promise.resolve(undefined as any); for (const plugin of this.getSortedPlugins(hookName)) { if (skipped && skipped.has(plugin)) continue; promise = promise.then(result => { @@ -154,9 +154,9 @@ export class PluginDriver { // chains synchronously, first non-null result stops and returns hookFirstSync( hookName: H, - args: Parameters, + args: Parameters, replaceContext?: ReplaceContext - ): ReturnType { + ): ReturnType { for (const plugin of this.plugins) { const result = this.runHookSync(hookName, args, plugin, replaceContext); if (result != null) return result; @@ -167,7 +167,7 @@ export class PluginDriver { // parallel, ignores returns hookParallel( hookName: H, - args: Parameters, + args: Parameters, replaceContext?: ReplaceContext ): Promise { const promises: Promise[] = []; @@ -182,10 +182,10 @@ export class PluginDriver { // chains, reduces returned value, handling the reduced value as the first hook argument hookReduceArg0( hookName: H, - [arg0, ...rest]: Parameters, + [arg0, ...rest]: Parameters, reduce: ( reduction: Arg0, - result: ReturnType, + result: ReturnType, plugin: Plugin ) => Arg0, replaceContext?: ReplaceContext @@ -193,7 +193,7 @@ export class PluginDriver { let promise = Promise.resolve(arg0); for (const plugin of this.getSortedPlugins(hookName)) { promise = promise.then(arg0 => { - const args = [arg0, ...rest] as Parameters; + const args = [arg0, ...rest] as Parameters; const hookPromise = this.runHook(hookName, args, plugin, false, replaceContext); if (!hookPromise) return arg0; return hookPromise.then(result => @@ -207,16 +207,16 @@ export class PluginDriver { // chains synchronously, reduces returned value, handling the reduced value as the first hook argument hookReduceArg0Sync( hookName: H, - [arg0, ...rest]: Parameters, + [arg0, ...rest]: Parameters, reduce: ( reduction: Arg0, - result: ReturnType, + result: ReturnType, plugin: Plugin ) => Arg0, replaceContext?: ReplaceContext ): Arg0 { for (const plugin of this.plugins) { - const args = [arg0, ...rest] as Parameters; + const args = [arg0, ...rest] as Parameters; const result = this.runHookSync(hookName, args, plugin, replaceContext); arg0 = reduce.call(this.pluginContexts.get(plugin), arg0, result, plugin); } @@ -236,7 +236,7 @@ export class PluginDriver { replaceContext?: ReplaceContext ): Promise { let promise = Promise.resolve(initialValue); - for (const plugin of this.plugins) { + for (const plugin of this.getSortedPlugins(hookName)) { promise = promise.then(value => { const hookPromise = this.runHook(hookName, args, plugin, true, replaceContext); if (!hookPromise) return value; @@ -252,8 +252,8 @@ export class PluginDriver { hookReduceValueSync( hookName: H, initialValue: T, - args: Parameters, - reduce: (reduction: T, result: ReturnType, plugin: Plugin) => T, + args: Parameters, + reduce: (reduction: T, result: ReturnType, plugin: Plugin) => T, replaceContext?: ReplaceContext ): T { let acc = initialValue; @@ -267,7 +267,7 @@ export class PluginDriver { // chains, ignores returns hookSeq( hookName: H, - args: Parameters, + args: Parameters, replaceContext?: ReplaceContext ): Promise { let promise = Promise.resolve(); @@ -279,7 +279,7 @@ export class PluginDriver { return promise; } - private getSortedPlugins(hookName: AsyncPluginHooks): Plugin[] { + private getSortedPlugins(hookName: AsyncPluginHooks | AddonHooks): Plugin[] { return getOrCreate(this.sortedPlugins, hookName, () => getSortedPlugins(hookName, this.plugins) ); @@ -302,22 +302,21 @@ export class PluginDriver { ): EnsurePromise>; private runHook( hookName: H, - args: Parameters, + args: Parameters, plugin: Plugin, permitValues: false, hookContext?: ReplaceContext | null - ): Promise>; + ): Promise>; private runHook( hookName: H, - args: Parameters, + args: Parameters, plugin: Plugin, permitValues: boolean, hookContext?: ReplaceContext | null - ): Promise> { + ): Promise> { const hook = plugin[hookName]; if (!hook) return undefined as any; - const handler = - typeof hook === 'object' && typeof hook.handler === 'function' ? hook.handler : hook; + const handler = typeof hook === 'object' ? hook.handler : hook; let context = this.pluginContexts.get(plugin)!; if (hookContext) { @@ -327,6 +326,7 @@ export class PluginDriver { let action: [string, string, Parameters] | null = null; return Promise.resolve() .then(() => { + // TODO Lukas move validation to sort function // permit values allows values to be returned instead of a functional hook if (typeof handler !== 'function') { if (permitValues) return handler; @@ -335,7 +335,7 @@ export class PluginDriver { // eslint-disable-next-line @typescript-eslint/ban-types const hookResult = (handler as Function).apply(context, args); - if (!hookResult || !hookResult.then) { + if (!hookResult?.then) { // short circuit for non-thenables and non-Promises return hookResult; } @@ -375,10 +375,10 @@ export class PluginDriver { */ private runHookSync( hookName: H, - args: Parameters, + args: Parameters, plugin: Plugin, hookContext?: ReplaceContext - ): ReturnType { + ): ReturnType { const hook = plugin[hookName]; if (!hook) return undefined as any; @@ -401,14 +401,17 @@ export class PluginDriver { } // TODO Lukas we can do plugin hook validation in this function -export function getSortedPlugins(hookName: AsyncPluginHooks, plugins: readonly Plugin[]): Plugin[] { +export function getSortedPlugins( + hookName: AsyncPluginHooks | AddonHooks, + plugins: readonly Plugin[] +): Plugin[] { const pre: Plugin[] = []; const normal: Plugin[] = []; const post: Plugin[] = []; for (const plugin of plugins) { const hook = plugin[hookName]; if (hook) { - if (typeof hook === 'object' && 'order' in hook) { + if (typeof hook === 'object') { if (hook.order === 'pre') { pre.push(plugin); continue; diff --git a/test/form/samples/enforce-addon-order/_config.js b/test/form/samples/enforce-addon-order/_config.js new file mode 100644 index 00000000000..730f6eda454 --- /dev/null +++ b/test/form/samples/enforce-addon-order/_config.js @@ -0,0 +1,40 @@ +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const acorn = require('acorn'); + +const ID_MAIN = path.join(__dirname, 'main.js'); +const code = fs.readFileSync(ID_MAIN, 'utf8'); + +const hooks = ['banner', 'footer', 'intro', 'outro']; + +const plugins = []; +addPlugin(null); +addPlugin('pre'); +addPlugin('post'); +addPlugin('post'); +addPlugin('pre'); +addPlugin(undefined); +function addPlugin(order) { + const name = `${order}-${(plugins.length >> 1) + 1}`; + const plugin = { name }; + const stringPlugin = { name: `string-${name}` }; + for (const hook of hooks) { + plugin[hook] = { + order, + handler: () => `// ${hook}-${name}` + }; + stringPlugin[hook] = { + order, + handler: `// ${hook}-string-${name}` + }; + } + plugins.push(plugin, stringPlugin); +} + +module.exports = { + description: 'allows to enforce addon order', + options: { + plugins + } +}; diff --git a/test/form/samples/enforce-addon-order/_expected.js b/test/form/samples/enforce-addon-order/_expected.js new file mode 100644 index 00000000000..ac10e4bb458 --- /dev/null +++ b/test/form/samples/enforce-addon-order/_expected.js @@ -0,0 +1,77 @@ + +// banner-pre-2 +// banner-string-pre-2 +// banner-pre-5 +// banner-string-pre-5 +// banner-null-1 +// banner-string-null-1 +// banner-undefined-6 +// banner-string-undefined-6 +// banner-post-3 +// banner-string-post-3 +// banner-post-4 +// banner-string-post-4 +// intro-pre-2 + +// intro-string-pre-2 + +// intro-pre-5 + +// intro-string-pre-5 + +// intro-null-1 + +// intro-string-null-1 + +// intro-undefined-6 + +// intro-string-undefined-6 + +// intro-post-3 + +// intro-string-post-3 + +// intro-post-4 + +// intro-string-post-4 + +console.log('main'); + + + +// outro-pre-2 + +// outro-string-pre-2 + +// outro-pre-5 + +// outro-string-pre-5 + +// outro-null-1 + +// outro-string-null-1 + +// outro-undefined-6 + +// outro-string-undefined-6 + +// outro-post-3 + +// outro-string-post-3 + +// outro-post-4 + +// outro-string-post-4 + +// footer-pre-2 +// footer-string-pre-2 +// footer-pre-5 +// footer-string-pre-5 +// footer-null-1 +// footer-string-null-1 +// footer-undefined-6 +// footer-string-undefined-6 +// footer-post-3 +// footer-string-post-3 +// footer-post-4 +// footer-string-post-4 diff --git a/test/form/samples/enforce-addon-order/main.js b/test/form/samples/enforce-addon-order/main.js new file mode 100644 index 00000000000..c0b933d7b56 --- /dev/null +++ b/test/form/samples/enforce-addon-order/main.js @@ -0,0 +1 @@ +console.log('main'); From 3e7147e97510793e19381725f4ba79f422f657cf Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sat, 6 Aug 2022 07:04:27 +0200 Subject: [PATCH 07/15] Allow object form for addon hooks --- src/rollup/rollup.ts | 7 +- src/rollup/types.d.ts | 4 +- src/utils/PluginDriver.ts | 107 +++++++++--------- src/utils/error.ts | 18 +++ .../samples/invalid-addon-hook/_config.js | 13 +++ .../samples/invalid-addon-hook/foo.js | 1 + .../samples/invalid-addon-hook/main.js | 1 + .../non-function-hook-async/_config.js | 8 +- test/hooks/index.js | 4 +- 9 files changed, 101 insertions(+), 62 deletions(-) create mode 100644 test/function/samples/invalid-addon-hook/_config.js create mode 100644 test/function/samples/invalid-addon-hook/foo.js create mode 100644 test/function/samples/invalid-addon-hook/main.js diff --git a/src/rollup/rollup.ts b/src/rollup/rollup.ts index c4f99b695b6..f34d06ad11b 100644 --- a/src/rollup/rollup.ts +++ b/src/rollup/rollup.ts @@ -1,7 +1,7 @@ import { version as rollupVersion } from 'package.json'; import Bundle from '../Bundle'; import Graph from '../Graph'; -import { getSortedPlugins } from '../utils/PluginDriver'; +import { getSortedValidatedPlugins } from '../utils/PluginDriver'; import type { PluginDriver } from '../utils/PluginDriver'; import { ensureArray } from '../utils/ensureArray'; import { errAlreadyClosed, errCannotEmitFromOptionsHook, error } from '../utils/error'; @@ -113,7 +113,10 @@ async function getInputOptions( if (!rawInputOptions) { throw new Error('You must supply an options object to rollup'); } - const rawPlugins = getSortedPlugins('options', ensureArray(rawInputOptions.plugins) as Plugin[]); + const rawPlugins = getSortedValidatedPlugins( + 'options', + ensureArray(rawInputOptions.plugins) as Plugin[] + ); const { options, unsetOptions } = normalizeInputOptions( await rawPlugins.reduce(applyOptionHook(watchMode), Promise.resolve(rawInputOptions)) ); diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 8ae2e6a0511..baae42b5daa 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -468,9 +468,7 @@ type MakeAsync any> = ( ...a: Parameters ) => ReturnType | Promise>; -type AllowEnforceOrderHook any> = - | T - | { handler: T; order?: 'pre' | 'post' | null }; +type AllowEnforceOrderHook = T | { handler: T; order?: 'pre' | 'post' | null }; export type PluginHooks = { [K in keyof FunctionPluginHooks]: K extends AsyncPluginHooks diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index a99e7528fca..d2070899883 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -1,7 +1,6 @@ import type Chunk from '../Chunk'; import type Graph from '../Graph'; import type Module from '../Module'; -import { InputPluginHooks } from '../rollup/types'; import type { AddonHookFunction, AddonHooks, @@ -19,22 +18,23 @@ import type { SerializablePluginCache, SyncPluginHooks } from '../rollup/types'; +import { InputPluginHooks } from '../rollup/types'; import { FileEmitter } from './FileEmitter'; import { getPluginContext } from './PluginContext'; -import { errInputHookInOutputPlugin, error } from './error'; +import { + errInputHookInOutputPlugin, + errInvalidAddonPluginHook, + errInvalidAsyncPluginHook, + error +} from './error'; import { getOrCreate } from './getOrCreate'; import { throwPluginError, warnDeprecatedHooks } from './pluginUtils'; -/** - * Get the inner type from a promise - * @example ResolveValue> -> string - */ -type ResolveValue = T extends Promise ? K : T; /** * Coerce a promise union to always be a promise. * @example EnsurePromise> -> Promise */ -type EnsurePromise = Promise>; +type EnsurePromise = Promise>; /** * Get the type of the first argument in a function. * @example Arg0<(a: string, b: number) => void> -> string @@ -139,13 +139,13 @@ export class PluginDriver { args: Parameters, replaceContext?: ReplaceContext | null, skipped?: ReadonlySet | null - ): Promise> { - let promise: Promise> = Promise.resolve(undefined as any); + ): Promise | null> { + let promise: Promise | null> = Promise.resolve(null); for (const plugin of this.getSortedPlugins(hookName)) { if (skipped && skipped.has(plugin)) continue; promise = promise.then(result => { if (result != null) return result; - return this.runHook(hookName, args, plugin, false, replaceContext); + return this.runHook(hookName, args, plugin, replaceContext); }); } return promise; @@ -172,7 +172,7 @@ export class PluginDriver { ): Promise { const promises: Promise[] = []; for (const plugin of this.getSortedPlugins(hookName)) { - const hookPromise = this.runHook(hookName, args, plugin, false, replaceContext); + const hookPromise = this.runHook(hookName, args, plugin, replaceContext); if (!hookPromise) continue; promises.push(hookPromise); } @@ -194,7 +194,7 @@ export class PluginDriver { for (const plugin of this.getSortedPlugins(hookName)) { promise = promise.then(arg0 => { const args = [arg0, ...rest] as Parameters; - const hookPromise = this.runHook(hookName, args, plugin, false, replaceContext); + const hookPromise = this.runHook(hookName, args, plugin, replaceContext); if (!hookPromise) return arg0; return hookPromise.then(result => reduce.call(this.pluginContexts.get(plugin), arg0, result, plugin) @@ -228,17 +228,19 @@ export class PluginDriver { hookName: H, initialValue: T | Promise, args: Parameters, - reduce: ( - reduction: T, - result: ResolveValue>, - plugin: Plugin - ) => T, - replaceContext?: ReplaceContext + reduce: (reduction: T, result: Awaited>, plugin: Plugin) => T ): Promise { let promise = Promise.resolve(initialValue); - for (const plugin of this.getSortedPlugins(hookName)) { + for (const plugin of this.getSortedPlugins( + hookName, + (handler: unknown, hookName: string, plugin: Plugin) => { + if (typeof handler !== 'string' && typeof handler !== 'function') { + return error(errInvalidAddonPluginHook(hookName, plugin.name)); + } + } + )) { promise = promise.then(value => { - const hookPromise = this.runHook(hookName, args, plugin, true, replaceContext); + const hookPromise = this.runHook(hookName, args, plugin); if (!hookPromise) return value; return hookPromise.then(result => reduce.call(this.pluginContexts.get(plugin), value, result, plugin) @@ -273,15 +275,18 @@ export class PluginDriver { let promise = Promise.resolve(); for (const plugin of this.getSortedPlugins(hookName)) { promise = promise.then( - () => this.runHook(hookName, args, plugin, false, replaceContext) as Promise + () => this.runHook(hookName, args, plugin, replaceContext) as Promise ); } return promise; } - private getSortedPlugins(hookName: AsyncPluginHooks | AddonHooks): Plugin[] { + private getSortedPlugins( + hookName: AsyncPluginHooks | AddonHooks, + validateHandler?: (handler: unknown, hookName: string, plugin: Plugin) => void + ): Plugin[] { return getOrCreate(this.sortedPlugins, hookName, () => - getSortedPlugins(hookName, this.plugins) + getSortedValidatedPlugins(hookName, this.plugins, validateHandler) ); } @@ -290,47 +295,40 @@ export class PluginDriver { * @param hookName Name of the plugin hook. Must be either in `PluginHooks` or `OutputPluginValueHooks`. * @param args Arguments passed to the plugin hook. * @param plugin The actual pluginObject to run. - * @param permitValues If true, values can be passed instead of functions for the plugin hook. - * @param hookContext When passed, the plugin context can be overridden. + * @param replaceContext When passed, the plugin context can be overridden. */ private runHook( hookName: H, args: Parameters, - plugin: Plugin, - permitValues: true, - hookContext?: ReplaceContext | null + plugin: Plugin ): EnsurePromise>; private runHook( hookName: H, args: Parameters, plugin: Plugin, - permitValues: false, - hookContext?: ReplaceContext | null + replaceContext?: ReplaceContext | null ): Promise>; - private runHook( + // Implementation signature + private runHook( hookName: H, - args: Parameters, + args: unknown[], plugin: Plugin, - permitValues: boolean, - hookContext?: ReplaceContext | null - ): Promise> { - const hook = plugin[hookName]; - if (!hook) return undefined as any; + replaceContext?: ReplaceContext | null + ): Promise | undefined { + // We always filter for plugins that support the hook before running it + const hook = plugin[hookName]!; const handler = typeof hook === 'object' ? hook.handler : hook; let context = this.pluginContexts.get(plugin)!; - if (hookContext) { - context = hookContext(context, plugin); + if (replaceContext) { + context = replaceContext(context, plugin); } let action: [string, string, Parameters] | null = null; return Promise.resolve() .then(() => { - // TODO Lukas move validation to sort function - // permit values allows values to be returned instead of a functional hook if (typeof handler !== 'function') { - if (permitValues) return handler; - return throwInvalidHookError(hookName, plugin.name); + return handler; } // eslint-disable-next-line @typescript-eslint/ban-types const hookResult = (handler as Function).apply(context, args); @@ -371,20 +369,20 @@ export class PluginDriver { * @param hookName Name of the plugin hook. Must be in `PluginHooks`. * @param args Arguments passed to the plugin hook. * @param plugin The acutal plugin - * @param hookContext When passed, the plugin context can be overridden. + * @param replaceContext When passed, the plugin context can be overridden. */ private runHookSync( hookName: H, args: Parameters, plugin: Plugin, - hookContext?: ReplaceContext + replaceContext?: ReplaceContext ): ReturnType { const hook = plugin[hookName]; if (!hook) return undefined as any; let context = this.pluginContexts.get(plugin)!; - if (hookContext) { - context = hookContext(context, plugin); + if (replaceContext) { + context = replaceContext(context, plugin); } try { @@ -400,10 +398,14 @@ export class PluginDriver { } } -// TODO Lukas we can do plugin hook validation in this function -export function getSortedPlugins( +export function getSortedValidatedPlugins( hookName: AsyncPluginHooks | AddonHooks, - plugins: readonly Plugin[] + plugins: readonly Plugin[], + validateHandler = (handler: unknown, hookName: string, plugin: Plugin) => { + if (typeof handler !== 'function') { + error(errInvalidAsyncPluginHook(hookName, plugin.name)); + } + } ): Plugin[] { const pre: Plugin[] = []; const normal: Plugin[] = []; @@ -412,6 +414,7 @@ export function getSortedPlugins( const hook = plugin[hookName]; if (hook) { if (typeof hook === 'object') { + validateHandler(hook.handler, hookName, plugin); if (hook.order === 'pre') { pre.push(plugin); continue; @@ -420,6 +423,8 @@ export function getSortedPlugins( post.push(plugin); continue; } + } else { + validateHandler(hook, hookName, plugin); } normal.push(plugin); } diff --git a/src/utils/error.ts b/src/utils/error.ts index f17ea17f0ab..8f9116d9da9 100644 --- a/src/utils/error.ts +++ b/src/utils/error.ts @@ -255,6 +255,24 @@ export function errInvalidOption( }; } +export function errInvalidAddonPluginHook(hook: string, plugin: string): RollupLogProps { + return { + code: Errors.INVALID_PLUGIN_HOOK, + hook, + message: `Error running plugin hook ${hook} for plugin ${plugin}, expected a string, a function hook or an object with a "handler" string or function.`, + plugin + }; +} + +export function errInvalidAsyncPluginHook(hook: string, plugin: string): RollupLogProps { + return { + code: Errors.INVALID_PLUGIN_HOOK, + hook, + message: `Error running plugin hook ${hook} for plugin ${plugin}, expected a function hook or an object with a "handler" function.`, + plugin + }; +} + export function errInvalidRollupPhaseForAddWatchFile(): RollupLogProps { return { code: Errors.INVALID_ROLLUP_PHASE, diff --git a/test/function/samples/invalid-addon-hook/_config.js b/test/function/samples/invalid-addon-hook/_config.js new file mode 100644 index 00000000000..95617db72ea --- /dev/null +++ b/test/function/samples/invalid-addon-hook/_config.js @@ -0,0 +1,13 @@ +module.exports = { + description: 'throws when providing a non-string value for an addon hook', + options: { + plugins: { + intro: 42 + } + }, + generateError: { + code: 'ADDON_ERROR', + message: + 'Could not retrieve intro. Check configuration of plugin at position 1.\n\tError Message: Error running plugin hook intro for plugin at position 1, expected a string, a function hook or an object with a "handler" string or function.' + } +}; diff --git a/test/function/samples/invalid-addon-hook/foo.js b/test/function/samples/invalid-addon-hook/foo.js new file mode 100644 index 00000000000..7a4e8a723a4 --- /dev/null +++ b/test/function/samples/invalid-addon-hook/foo.js @@ -0,0 +1 @@ +export default 42; diff --git a/test/function/samples/invalid-addon-hook/main.js b/test/function/samples/invalid-addon-hook/main.js new file mode 100644 index 00000000000..a25cfbd9058 --- /dev/null +++ b/test/function/samples/invalid-addon-hook/main.js @@ -0,0 +1 @@ +export default () => import('./foo.js'); diff --git a/test/function/samples/non-function-hook-async/_config.js b/test/function/samples/non-function-hook-async/_config.js index d17469d3926..bfba8d840ea 100644 --- a/test/function/samples/non-function-hook-async/_config.js +++ b/test/function/samples/non-function-hook-async/_config.js @@ -6,10 +6,10 @@ module.exports = { } }, error: { - code: 'PLUGIN_ERROR', + code: 'INVALID_PLUGIN_HOOK', hook: 'resolveId', - message: 'Error running plugin hook resolveId for at position 1, expected a function hook.', - plugin: 'at position 1', - pluginCode: 'INVALID_PLUGIN_HOOK' + message: + 'Error running plugin hook resolveId for plugin at position 1, expected a function hook or an object with a "handler" function.', + plugin: 'at position 1' } }; diff --git a/test/hooks/index.js b/test/hooks/index.js index 7ae444216b0..e0c9e0fc874 100644 --- a/test/hooks/index.js +++ b/test/hooks/index.js @@ -30,8 +30,8 @@ describe('hooks', () => { format: 'es' }); const fileNames = (await readdir(TEMP_DIR)).sort(); - assert.deepStrictEqual(fileNames, ['chunk.js', 'input.js']); await remove(TEMP_DIR); + assert.deepStrictEqual(fileNames, ['chunk.js', 'input.js']); }); it('supports buildStart and buildEnd hooks', () => { @@ -567,8 +567,8 @@ describe('hooks', () => { ] }); await bundle.write({ format: 'es', file }); - assert.strictEqual(callCount, 1); await remove(TEMP_DIR); + assert.strictEqual(callCount, 1); }); it('supports this.cache for plugins', () => From b703d1bd4baa93d747b255a06bf6b76475105d45 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sat, 6 Aug 2022 07:36:11 +0200 Subject: [PATCH 08/15] Improve coverage --- src/rollup/rollup.ts | 18 +++++++----------- src/utils/PluginDriver.ts | 40 ++++++++++++++++++--------------------- 2 files changed, 25 insertions(+), 33 deletions(-) diff --git a/src/rollup/rollup.ts b/src/rollup/rollup.ts index f34d06ad11b..8f9ec428c28 100644 --- a/src/rollup/rollup.ts +++ b/src/rollup/rollup.ts @@ -129,17 +129,13 @@ function applyOptionHook(watchMode: boolean) { inputOptions: Promise, plugin: Plugin ): Promise => { - if (plugin.options) { - const handler = 'handler' in plugin.options ? plugin.options.handler : plugin.options; - return ( - ((await handler.call( - { meta: { rollupVersion, watchMode } }, - await inputOptions - )) as GenericConfigObject) || inputOptions - ); - } - - return inputOptions; + const handler = 'handler' in plugin.options! ? plugin.options.handler : plugin.options!; + return ( + ((await handler.call( + { meta: { rollupVersion, watchMode } }, + await inputOptions + )) as GenericConfigObject) || inputOptions + ); }; } diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index d2070899883..7cd92ad75b9 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -170,13 +170,11 @@ export class PluginDriver { args: Parameters, replaceContext?: ReplaceContext ): Promise { - const promises: Promise[] = []; - for (const plugin of this.getSortedPlugins(hookName)) { - const hookPromise = this.runHook(hookName, args, plugin, replaceContext); - if (!hookPromise) continue; - promises.push(hookPromise); - } - return Promise.all(promises).then(() => {}); + return Promise.all( + this.getSortedPlugins(hookName).map(plugin => + this.runHook(hookName, args, plugin, replaceContext) + ) + ).then(() => {}); } // chains, reduces returned value, handling the reduced value as the first hook argument @@ -192,14 +190,14 @@ export class PluginDriver { ): Promise> { let promise = Promise.resolve(arg0); for (const plugin of this.getSortedPlugins(hookName)) { - promise = promise.then(arg0 => { - const args = [arg0, ...rest] as Parameters; - const hookPromise = this.runHook(hookName, args, plugin, replaceContext); - if (!hookPromise) return arg0; - return hookPromise.then(result => - reduce.call(this.pluginContexts.get(plugin), arg0, result, plugin) - ); - }); + promise = promise.then(arg0 => + this.runHook( + hookName, + [arg0, ...rest] as Parameters, + plugin, + replaceContext + ).then(result => reduce.call(this.pluginContexts.get(plugin), arg0, result, plugin)) + ); } return promise; } @@ -239,13 +237,11 @@ export class PluginDriver { } } )) { - promise = promise.then(value => { - const hookPromise = this.runHook(hookName, args, plugin); - if (!hookPromise) return value; - return hookPromise.then(result => + promise = promise.then(value => + this.runHook(hookName, args, plugin).then(result => reduce.call(this.pluginContexts.get(plugin), value, result, plugin) - ); - }); + ) + ); } return promise; } @@ -314,7 +310,7 @@ export class PluginDriver { args: unknown[], plugin: Plugin, replaceContext?: ReplaceContext | null - ): Promise | undefined { + ): Promise { // We always filter for plugins that support the hook before running it const hook = plugin[hookName]!; const handler = typeof hook === 'object' ? hook.handler : hook; From 8a8f609d98e065da951ef63e0f79875a054f72a5 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sat, 6 Aug 2022 14:45:15 +0200 Subject: [PATCH 09/15] Allow odering for sync hooks --- src/rollup/types.d.ts | 10 +++---- src/utils/PluginDriver.ts | 27 ++++++------------ .../samples/enforce-plugin-order/_config.js | 28 +++++++++++++++++-- .../samples/enforce-plugin-order/dep.js | 1 - .../samples/enforce-plugin-order/main.js | 2 +- .../samples/non-function-hook-sync/_config.js | 8 +++--- 6 files changed, 44 insertions(+), 32 deletions(-) delete mode 100644 test/function/samples/enforce-plugin-order/dep.js diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index baae42b5daa..0017e04eef7 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -468,17 +468,17 @@ type MakeAsync any> = ( ...a: Parameters ) => ReturnType | Promise>; -type AllowEnforceOrderHook = T | { handler: T; order?: 'pre' | 'post' | null }; +type ObjectHook = T | { handler: T; order?: 'pre' | 'post' | null }; export type PluginHooks = { - [K in keyof FunctionPluginHooks]: K extends AsyncPluginHooks - ? AllowEnforceOrderHook> - : FunctionPluginHooks[K]; + [K in keyof FunctionPluginHooks]: ObjectHook< + K extends AsyncPluginHooks ? MakeAsync : FunctionPluginHooks[K] + >; }; export interface OutputPlugin extends Partial<{ [K in OutputPluginHooks]: PluginHooks[K] }>, - Partial<{ [K in AddonHooks]: AllowEnforceOrderHook }> { + Partial<{ [K in AddonHooks]: ObjectHook }> { cacheKey?: string; name: string; } diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index 7cd92ad75b9..a3256a14f90 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -62,13 +62,6 @@ const inputHooks = Object.keys(inputHookNames); export type ReplaceContext = (context: PluginContext, plugin: Plugin) => PluginContext; -function throwInvalidHookError(hookName: string, pluginName: string): never { - return error({ - code: 'INVALID_PLUGIN_HOOK', - message: `Error running plugin hook ${hookName} for ${pluginName}, expected a function hook.` - }); -} - export type HookAction = [plugin: string, hook: string, args: unknown[]]; export class PluginDriver { @@ -157,7 +150,7 @@ export class PluginDriver { args: Parameters, replaceContext?: ReplaceContext ): ReturnType { - for (const plugin of this.plugins) { + for (const plugin of this.getSortedPlugins(hookName)) { const result = this.runHookSync(hookName, args, plugin, replaceContext); if (result != null) return result; } @@ -213,7 +206,7 @@ export class PluginDriver { ) => Arg0, replaceContext?: ReplaceContext ): Arg0 { - for (const plugin of this.plugins) { + for (const plugin of this.getSortedPlugins(hookName)) { const args = [arg0, ...rest] as Parameters; const result = this.runHookSync(hookName, args, plugin, replaceContext); arg0 = reduce.call(this.pluginContexts.get(plugin), arg0, result, plugin); @@ -255,7 +248,7 @@ export class PluginDriver { replaceContext?: ReplaceContext ): T { let acc = initialValue; - for (const plugin of this.plugins) { + for (const plugin of this.getSortedPlugins(hookName)) { const result = this.runHookSync(hookName, args, plugin, replaceContext); acc = reduce.call(this.pluginContexts.get(plugin), acc, result, plugin); } @@ -278,7 +271,7 @@ export class PluginDriver { } private getSortedPlugins( - hookName: AsyncPluginHooks | AddonHooks, + hookName: keyof FunctionPluginHooks | AddonHooks, validateHandler?: (handler: unknown, hookName: string, plugin: Plugin) => void ): Plugin[] { return getOrCreate(this.sortedPlugins, hookName, () => @@ -373,8 +366,8 @@ export class PluginDriver { plugin: Plugin, replaceContext?: ReplaceContext ): ReturnType { - const hook = plugin[hookName]; - if (!hook) return undefined as any; + const hook = plugin[hookName]!; + const handler = typeof hook === 'object' ? hook.handler : hook; let context = this.pluginContexts.get(plugin)!; if (replaceContext) { @@ -382,12 +375,8 @@ export class PluginDriver { } try { - // permit values allows values to be returned instead of a functional hook - if (typeof hook !== 'function') { - return throwInvalidHookError(hookName, plugin.name); - } // eslint-disable-next-line @typescript-eslint/ban-types - return (hook as Function).apply(context, args); + return (handler as Function).apply(context, args); } catch (err: any) { return throwPluginError(err, plugin.name, { hook: hookName }); } @@ -395,7 +384,7 @@ export class PluginDriver { } export function getSortedValidatedPlugins( - hookName: AsyncPluginHooks | AddonHooks, + hookName: keyof FunctionPluginHooks | AddonHooks, plugins: readonly Plugin[], validateHandler = (handler: unknown, hookName: string, plugin: Plugin) => { if (typeof handler !== 'function') { diff --git a/test/function/samples/enforce-plugin-order/_config.js b/test/function/samples/enforce-plugin-order/_config.js index 819555ad916..edc2b2faf7c 100644 --- a/test/function/samples/enforce-plugin-order/_config.js +++ b/test/function/samples/enforce-plugin-order/_config.js @@ -7,16 +7,21 @@ const ID_MAIN = path.join(__dirname, 'main.js'); const code = fs.readFileSync(ID_MAIN, 'utf8'); const hooks = [ + 'augmentChunkHash', 'buildEnd', 'buildStart', 'generateBundle', 'load', 'moduleParsed', 'options', + 'outputOptions', + 'renderDynamicImport', 'renderChunk', 'renderStart', 'resolveDynamicImport', + 'resolveFileUrl', 'resolveId', + 'resolveImportMeta', 'shouldTransformCachedModule', 'transform' ]; @@ -26,7 +31,26 @@ for (const hook of hooks) { calledHooks[hook] = []; } -const plugins = []; +const plugins = [ + { + name: 'emitter', + resolveId(source) { + if (source === 'dep') { + return source; + } + }, + load(source) { + if (source === 'dep') { + return `assert.okt(import.meta.url);\nassert.ok(import.meta.ROLLUP_FILE_URL_${this.emitFile( + { + type: 'asset', + source: 'test' + } + )});`; + } + } + } +]; addPlugin(null); addPlugin('pre'); addPlugin('post'); @@ -34,7 +58,7 @@ addPlugin('post'); addPlugin('pre'); addPlugin(undefined); function addPlugin(order) { - const name = `${order}-${plugins.length + 1}`; + const name = `${order}-${plugins.length}`; const plugin = { name }; for (const hook of hooks) { plugin[hook] = { diff --git a/test/function/samples/enforce-plugin-order/dep.js b/test/function/samples/enforce-plugin-order/dep.js deleted file mode 100644 index cc1d88a24fa..00000000000 --- a/test/function/samples/enforce-plugin-order/dep.js +++ /dev/null @@ -1 +0,0 @@ -assert.ok(true); diff --git a/test/function/samples/enforce-plugin-order/main.js b/test/function/samples/enforce-plugin-order/main.js index c02cf40e2fc..17f9d42facd 100644 --- a/test/function/samples/enforce-plugin-order/main.js +++ b/test/function/samples/enforce-plugin-order/main.js @@ -1 +1 @@ -import('./dep.js'); +import('dep'); diff --git a/test/function/samples/non-function-hook-sync/_config.js b/test/function/samples/non-function-hook-sync/_config.js index c447f84ec77..0349ba53f78 100644 --- a/test/function/samples/non-function-hook-sync/_config.js +++ b/test/function/samples/non-function-hook-sync/_config.js @@ -6,10 +6,10 @@ module.exports = { } }, generateError: { - code: 'PLUGIN_ERROR', + code: 'INVALID_PLUGIN_HOOK', hook: 'outputOptions', - message: 'Error running plugin hook outputOptions for at position 1, expected a function hook.', - plugin: 'at position 1', - pluginCode: 'INVALID_PLUGIN_HOOK' + message: + 'Error running plugin hook outputOptions for plugin at position 1, expected a function hook or an object with a "handler" function.', + plugin: 'at position 1' } }; From be348b7d5f6a31149ceb7edb34b5f187f233e66c Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 7 Aug 2022 06:53:51 +0200 Subject: [PATCH 10/15] Add support for making async parallel hooks sequential --- src/rollup/types.d.ts | 7 +- src/utils/PluginDriver.ts | 78 ++++++++----- src/utils/error.ts | 2 +- .../_config.js | 78 +++++++++++++ .../enforce-sequential-plugin-order/main.js | 1 + test/hooks/index.js | 110 ++++++++++++++++++ 6 files changed, 247 insertions(+), 29 deletions(-) create mode 100644 test/function/samples/enforce-sequential-plugin-order/_config.js create mode 100644 test/function/samples/enforce-sequential-plugin-order/main.js diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 0017e04eef7..e37f162761e 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -468,11 +468,14 @@ type MakeAsync any> = ( ...a: Parameters ) => ReturnType | Promise>; -type ObjectHook = T | { handler: T; order?: 'pre' | 'post' | null }; +type ObjectHook> = + | T + | ({ handler: T; order?: 'pre' | 'post' | null } & O); export type PluginHooks = { [K in keyof FunctionPluginHooks]: ObjectHook< - K extends AsyncPluginHooks ? MakeAsync : FunctionPluginHooks[K] + K extends AsyncPluginHooks ? MakeAsync : FunctionPluginHooks[K], + K extends ParallelPluginHooks ? { sequential?: boolean } : Record >; }; diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index a3256a14f90..6f6ec743cee 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -24,7 +24,7 @@ import { getPluginContext } from './PluginContext'; import { errInputHookInOutputPlugin, errInvalidAddonPluginHook, - errInvalidAsyncPluginHook, + errInvalidFunctionPluginHook, error } from './error'; import { getOrCreate } from './getOrCreate'; @@ -77,6 +77,7 @@ export class PluginDriver { private readonly fileEmitter: FileEmitter; private readonly pluginContexts: ReadonlyMap; private readonly plugins: readonly Plugin[]; + private readonly sortedParallelPlugins = new Map(); private readonly sortedPlugins = new Map(); private readonly unfulfilledActions = new Set(); @@ -149,12 +150,12 @@ export class PluginDriver { hookName: H, args: Parameters, replaceContext?: ReplaceContext - ): ReturnType { + ): ReturnType | null { for (const plugin of this.getSortedPlugins(hookName)) { const result = this.runHookSync(hookName, args, plugin, replaceContext); if (result != null) return result; } - return null as any; + return null; } // parallel, ignores returns @@ -163,11 +164,14 @@ export class PluginDriver { args: Parameters, replaceContext?: ReplaceContext ): Promise { - return Promise.all( - this.getSortedPlugins(hookName).map(plugin => - this.runHook(hookName, args, plugin, replaceContext) - ) - ).then(() => {}); + const [parallel, sequential] = this.getSortedParallelAndSequentialPlugins(hookName); + let promise: Promise = Promise.all( + parallel.map(plugin => this.runHook(hookName, args, plugin, replaceContext)) + ); + for (const plugin of sequential) { + promise = promise.then(() => this.runHook(hookName, args, plugin, replaceContext)); + } + return promise.then(noReturn); } // chains, reduces returned value, handling the reduced value as the first hook argument @@ -222,14 +226,7 @@ export class PluginDriver { reduce: (reduction: T, result: Awaited>, plugin: Plugin) => T ): Promise { let promise = Promise.resolve(initialValue); - for (const plugin of this.getSortedPlugins( - hookName, - (handler: unknown, hookName: string, plugin: Plugin) => { - if (typeof handler !== 'string' && typeof handler !== 'function') { - return error(errInvalidAddonPluginHook(hookName, plugin.name)); - } - } - )) { + for (const plugin of this.getSortedPlugins(hookName, validateAddonPluginHandler)) { promise = promise.then(value => this.runHook(hookName, args, plugin).then(result => reduce.call(this.pluginContexts.get(plugin), value, result, plugin) @@ -261,13 +258,32 @@ export class PluginDriver { args: Parameters, replaceContext?: ReplaceContext ): Promise { - let promise = Promise.resolve(); + let promise: Promise = Promise.resolve(); for (const plugin of this.getSortedPlugins(hookName)) { - promise = promise.then( - () => this.runHook(hookName, args, plugin, replaceContext) as Promise - ); + promise = promise.then(() => this.runHook(hookName, args, plugin, replaceContext)); } - return promise; + return promise.then(noReturn); + } + + private getSortedParallelAndSequentialPlugins(hookName: AsyncPluginHooks): [Plugin[], Plugin[]] { + return getOrCreate(this.sortedParallelPlugins, hookName, () => { + const parallel: Plugin[] = []; + const sequential: Plugin[] = []; + for (const plugin of this.plugins) { + const hook = plugin[hookName]; + if (hook) { + if (typeof hook === 'object' && hook.sequential) { + sequential.push(plugin); + } else { + parallel.push(plugin); + } + } + } + return [ + getSortedValidatedPlugins(hookName, parallel), + getSortedValidatedPlugins(hookName, sequential) + ]; + }); } private getSortedPlugins( @@ -386,11 +402,7 @@ export class PluginDriver { export function getSortedValidatedPlugins( hookName: keyof FunctionPluginHooks | AddonHooks, plugins: readonly Plugin[], - validateHandler = (handler: unknown, hookName: string, plugin: Plugin) => { - if (typeof handler !== 'function') { - error(errInvalidAsyncPluginHook(hookName, plugin.name)); - } - } + validateHandler = validateFunctionPluginHandler ): Plugin[] { const pre: Plugin[] = []; const normal: Plugin[] = []; @@ -416,3 +428,17 @@ export function getSortedValidatedPlugins( } return [...pre, ...normal, ...post]; } + +function validateFunctionPluginHandler(handler: unknown, hookName: string, plugin: Plugin) { + if (typeof handler !== 'function') { + error(errInvalidFunctionPluginHook(hookName, plugin.name)); + } +} + +function validateAddonPluginHandler(handler: unknown, hookName: string, plugin: Plugin) { + if (typeof handler !== 'string' && typeof handler !== 'function') { + return error(errInvalidAddonPluginHook(hookName, plugin.name)); + } +} + +function noReturn() {} diff --git a/src/utils/error.ts b/src/utils/error.ts index 8f9116d9da9..44f1972700e 100644 --- a/src/utils/error.ts +++ b/src/utils/error.ts @@ -264,7 +264,7 @@ export function errInvalidAddonPluginHook(hook: string, plugin: string): RollupL }; } -export function errInvalidAsyncPluginHook(hook: string, plugin: string): RollupLogProps { +export function errInvalidFunctionPluginHook(hook: string, plugin: string): RollupLogProps { return { code: Errors.INVALID_PLUGIN_HOOK, hook, diff --git a/test/function/samples/enforce-sequential-plugin-order/_config.js b/test/function/samples/enforce-sequential-plugin-order/_config.js new file mode 100644 index 00000000000..f2bce4afb1c --- /dev/null +++ b/test/function/samples/enforce-sequential-plugin-order/_config.js @@ -0,0 +1,78 @@ +const assert = require('assert'); +const { wait } = require('../../../utils'); + +const hooks = ['buildEnd', 'buildStart', 'moduleParsed', 'renderStart']; + +const calledHooks = {}; +for (const hook of hooks) { + calledHooks[hook] = []; +} + +const plugins = []; +addPlugin(null, false); +addPlugin('pre', false); +addPlugin('post', true); +addPlugin('post', false); +addPlugin('pre', true); +addPlugin(undefined, true); +addPlugin(null, false); +addPlugin('pre', true); +addPlugin('post', false); +addPlugin('post', true); +addPlugin('pre', false); +addPlugin(undefined, true); + +let hookActive = false; +function addPlugin(order, sequential) { + const name = `${order}-${sequential ? 'seq-' : ''}${plugins.length + 1}`; + const plugin = { name }; + for (const hook of hooks) { + plugin[hook] = { + order, + async handler() { + if (!calledHooks[hook].includes(name)) { + calledHooks[hook].push(name); + } + if (sequential) { + if (hookActive) { + throw new Error(`Detected parallel hook runs in ${hook}.`); + } + hookActive = true; + await wait(0); + hookActive = false; + } + }, + sequential + }; + } + plugins.push(plugin); +} + +module.exports = { + description: 'allows to enforce sequential plugin hook order for parallel plugin hooks', + options: { + plugins + }, + exports() { + for (const hook of hooks) { + assert.deepStrictEqual( + calledHooks[hook], + [ + 'pre-2', + 'pre-11', + 'null-1', + 'null-7', + 'post-4', + 'post-9', + 'pre-seq-5', + 'pre-seq-8', + 'undefined-seq-6', + 'undefined-seq-12', + 'post-seq-3', + 'post-seq-10' + ], + hook + ); + } + } +}; diff --git a/test/function/samples/enforce-sequential-plugin-order/main.js b/test/function/samples/enforce-sequential-plugin-order/main.js new file mode 100644 index 00000000000..cc1d88a24fa --- /dev/null +++ b/test/function/samples/enforce-sequential-plugin-order/main.js @@ -0,0 +1 @@ +assert.ok(true); diff --git a/test/hooks/index.js b/test/hooks/index.js index e0c9e0fc874..276fc7f9db7 100644 --- a/test/hooks/index.js +++ b/test/hooks/index.js @@ -1457,5 +1457,115 @@ describe('hooks', () => { } }); }); + + it('allows to enforce sequential plugin hook order in watch mode', async () => { + const hooks = ['closeBundle', 'closeWatcher', 'renderError', 'watchChange', 'writeBundle']; + + const calledHooks = {}; + for (const hook of hooks) { + calledHooks[hook] = []; + } + + let first = true; + const plugins = [ + { + name: 'render-error', + renderChunk() { + if (first) { + first = false; + throw new Error('Expected render error'); + } + } + } + ]; + addPlugin(null, false); + addPlugin('pre', false); + addPlugin('post', true); + addPlugin('post', false); + addPlugin('pre', true); + addPlugin(undefined, true); + addPlugin(null, false); + addPlugin('pre', true); + addPlugin('post', false); + addPlugin('post', true); + addPlugin('pre', false); + addPlugin(undefined, true); + + let hookActive = false; + function addPlugin(order, sequential) { + const name = `${order}-${sequential ? 'seq-' : ''}${plugins.length}`; + const plugin = { name }; + for (const hook of hooks) { + plugin[hook] = { + order, + async handler() { + if (!calledHooks[hook].includes(name)) { + calledHooks[hook].push(name); + } + if (sequential) { + if (hookActive) { + throw new Error(`Detected parallel hook runs in ${hook}.`); + } + hookActive = true; + await wait(0); + hookActive = false; + } + }, + sequential + }; + } + plugins.push(plugin); + } + + const ID_MAIN = path.join(TEMP_DIR, 'main.js'); + await outputFile(ID_MAIN, 'console.log(42);'); + + const watcher = rollup.watch({ + input: ID_MAIN, + output: { + format: 'es', + dir: path.join(TEMP_DIR, 'out') + }, + plugins + }); + + return new Promise((resolve, reject) => { + watcher.on('event', async event => { + if (event.code === 'ERROR') { + if (event.error.message !== 'Expected render error') { + reject(event.error); + } + await wait(200); + await outputFile(ID_MAIN, 'console.log(43);'); + } else if (event.code === 'BUNDLE_END') { + await event.result.close(); + resolve(); + } + }); + }).finally(async () => { + await watcher.close(); + await remove(TEMP_DIR); + for (const hook of hooks) { + assert.deepStrictEqual( + calledHooks[hook], + [ + 'pre-2', + 'pre-11', + 'null-1', + 'null-7', + 'post-4', + 'post-9', + 'pre-seq-5', + 'pre-seq-8', + 'undefined-seq-6', + 'undefined-seq-12', + 'post-seq-3', + 'post-seq-10' + ], + hook + ); + } + }); + }); }); }); From 85b4411617b92f904be9247b0c79bfe48265cb2f Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 9 Aug 2022 06:55:15 +0200 Subject: [PATCH 11/15] Change parallel behaviour to not change order --- src/utils/PluginDriver.ts | 41 +- .../_config.js | 46 ++- test/hooks/index.js | 384 +++++++++--------- 3 files changed, 230 insertions(+), 241 deletions(-) diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index 6f6ec743cee..ee92a445c0c 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -77,7 +77,6 @@ export class PluginDriver { private readonly fileEmitter: FileEmitter; private readonly pluginContexts: ReadonlyMap; private readonly plugins: readonly Plugin[]; - private readonly sortedParallelPlugins = new Map(); private readonly sortedPlugins = new Map(); private readonly unfulfilledActions = new Set(); @@ -159,19 +158,22 @@ export class PluginDriver { } // parallel, ignores returns - hookParallel( + async hookParallel( hookName: H, args: Parameters, replaceContext?: ReplaceContext ): Promise { - const [parallel, sequential] = this.getSortedParallelAndSequentialPlugins(hookName); - let promise: Promise = Promise.all( - parallel.map(plugin => this.runHook(hookName, args, plugin, replaceContext)) - ); - for (const plugin of sequential) { - promise = promise.then(() => this.runHook(hookName, args, plugin, replaceContext)); + const parallelPromises: Promise[] = []; + for (const plugin of this.getSortedPlugins(hookName)) { + if ((plugin[hookName] as { sequential?: boolean }).sequential) { + await Promise.all(parallelPromises); + parallelPromises.length = 0; + await this.runHook(hookName, args, plugin, replaceContext); + } else { + parallelPromises.push(this.runHook(hookName, args, plugin, replaceContext)); + } } - return promise.then(noReturn); + await Promise.all(parallelPromises); } // chains, reduces returned value, handling the reduced value as the first hook argument @@ -265,27 +267,6 @@ export class PluginDriver { return promise.then(noReturn); } - private getSortedParallelAndSequentialPlugins(hookName: AsyncPluginHooks): [Plugin[], Plugin[]] { - return getOrCreate(this.sortedParallelPlugins, hookName, () => { - const parallel: Plugin[] = []; - const sequential: Plugin[] = []; - for (const plugin of this.plugins) { - const hook = plugin[hookName]; - if (hook) { - if (typeof hook === 'object' && hook.sequential) { - sequential.push(plugin); - } else { - parallel.push(plugin); - } - } - } - return [ - getSortedValidatedPlugins(hookName, parallel), - getSortedValidatedPlugins(hookName, sequential) - ]; - }); - } - private getSortedPlugins( hookName: keyof FunctionPluginHooks | AddonHooks, validateHandler?: (handler: unknown, hookName: string, plugin: Plugin) => void diff --git a/test/function/samples/enforce-sequential-plugin-order/_config.js b/test/function/samples/enforce-sequential-plugin-order/_config.js index f2bce4afb1c..ffcd4c00d7f 100644 --- a/test/function/samples/enforce-sequential-plugin-order/_config.js +++ b/test/function/samples/enforce-sequential-plugin-order/_config.js @@ -4,25 +4,26 @@ const { wait } = require('../../../utils'); const hooks = ['buildEnd', 'buildStart', 'moduleParsed', 'renderStart']; const calledHooks = {}; +const activeHooks = {}; for (const hook of hooks) { calledHooks[hook] = []; + activeHooks[hook] = new Set(); } const plugins = []; -addPlugin(null, false); +addPlugin(null, true); addPlugin('pre', false); -addPlugin('post', true); addPlugin('post', false); -addPlugin('pre', true); +addPlugin('post', false); +addPlugin('pre', false); addPlugin(undefined, true); addPlugin(null, false); addPlugin('pre', true); -addPlugin('post', false); addPlugin('post', true); -addPlugin('pre', false); -addPlugin(undefined, true); +addPlugin('post', true); +addPlugin('pre', true); +addPlugin(undefined, false); -let hookActive = false; function addPlugin(order, sequential) { const name = `${order}-${sequential ? 'seq-' : ''}${plugins.length + 1}`; const plugin = { name }; @@ -30,17 +31,20 @@ function addPlugin(order, sequential) { plugin[hook] = { order, async handler() { + const active = activeHooks[hook]; if (!calledHooks[hook].includes(name)) { - calledHooks[hook].push(name); + calledHooks[hook].push(sequential ? name : [name, [...active]]); } if (sequential) { - if (hookActive) { + if (active.size > 0) { throw new Error(`Detected parallel hook runs in ${hook}.`); } - hookActive = true; - await wait(0); - hookActive = false; } + active.add(name); + // A setTimeout always takes longer than any chain of immediately + // resolved promises + await wait(0); + active.delete(name); }, sequential }; @@ -58,17 +62,17 @@ module.exports = { assert.deepStrictEqual( calledHooks[hook], [ - 'pre-2', - 'pre-11', - 'null-1', - 'null-7', - 'post-4', - 'post-9', - 'pre-seq-5', + ['pre-2', []], + ['pre-5', ['pre-2']], 'pre-seq-8', + 'pre-seq-11', + 'null-seq-1', 'undefined-seq-6', - 'undefined-seq-12', - 'post-seq-3', + ['null-7', []], + ['undefined-12', ['null-7']], + ['post-3', ['null-7', 'undefined-12']], + ['post-4', ['null-7', 'undefined-12', 'post-3']], + 'post-seq-9', 'post-seq-10' ], hook diff --git a/test/hooks/index.js b/test/hooks/index.js index 276fc7f9db7..8a22711c8be 100644 --- a/test/hooks/index.js +++ b/test/hooks/index.js @@ -1113,6 +1113,200 @@ describe('hooks', () => { }); }); + it('allows to enforce plugin hook order in watch mode', async () => { + const hooks = ['closeBundle', 'closeWatcher', 'renderError', 'watchChange', 'writeBundle']; + + const calledHooks = {}; + for (const hook of hooks) { + calledHooks[hook] = []; + } + + let first = true; + const plugins = [ + { + name: 'render-error', + renderChunk() { + if (first) { + first = false; + throw new Error('Expected render error'); + } + } + } + ]; + addPlugin(null); + addPlugin('pre'); + addPlugin('post'); + addPlugin('post'); + addPlugin('pre'); + addPlugin(undefined); + function addPlugin(order) { + const name = `${order}-${plugins.length}`; + const plugin = { name }; + for (const hook of hooks) { + plugin[hook] = { + order, + handler() { + if (!calledHooks[hook].includes(name)) { + calledHooks[hook].push(name); + } + } + }; + } + plugins.push(plugin); + } + + const ID_MAIN = path.join(TEMP_DIR, 'main.js'); + await outputFile(ID_MAIN, 'console.log(42);'); + + const watcher = rollup.watch({ + input: ID_MAIN, + output: { + format: 'es', + dir: path.join(TEMP_DIR, 'out') + }, + plugins + }); + + return new Promise((resolve, reject) => { + watcher.on('event', async event => { + if (event.code === 'ERROR') { + if (event.error.message !== 'Expected render error') { + reject(event.error); + } + await wait(200); + await outputFile(ID_MAIN, 'console.log(43);'); + } else if (event.code === 'BUNDLE_END') { + await event.result.close(); + resolve(); + } + }); + }).finally(async () => { + await watcher.close(); + await remove(TEMP_DIR); + for (const hook of hooks) { + assert.deepStrictEqual( + calledHooks[hook], + ['pre-2', 'pre-5', 'null-1', 'undefined-6', 'post-3', 'post-4'], + hook + ); + } + }); + }); + + it('allows to enforce sequential plugin hook order in watch mode', async () => { + const hooks = ['closeBundle', 'closeWatcher', 'renderError', 'watchChange', 'writeBundle']; + + const calledHooks = {}; + const activeHooks = {}; + for (const hook of hooks) { + calledHooks[hook] = []; + activeHooks[hook] = new Set(); + } + + let first = true; + const plugins = [ + { + name: 'render-error', + renderChunk() { + if (first) { + first = false; + throw new Error('Expected render error'); + } + } + } + ]; + addPlugin(null, true); + addPlugin('pre', false); + addPlugin('post', false); + addPlugin('post', false); + addPlugin('pre', false); + addPlugin(undefined, true); + addPlugin(null, false); + addPlugin('pre', true); + addPlugin('post', true); + addPlugin('post', true); + addPlugin('pre', true); + addPlugin(undefined, false); + + function addPlugin(order, sequential) { + const name = `${order}-${sequential ? 'seq-' : ''}${plugins.length}`; + const plugin = { name }; + for (const hook of hooks) { + plugin[hook] = { + order, + async handler() { + const active = activeHooks[hook]; + if (!calledHooks[hook].includes(name)) { + calledHooks[hook].push(sequential ? name : [name, [...active]]); + } + if (sequential) { + if (active.size > 0) { + throw new Error(`Detected parallel hook runs in ${hook}.`); + } + } + active.add(name); + // A setTimeout always takes longer than any chain of immediately + // resolved promises + await wait(0); + active.delete(name); + }, + sequential + }; + } + plugins.push(plugin); + } + + const ID_MAIN = path.join(TEMP_DIR, 'main.js'); + await outputFile(ID_MAIN, 'console.log(42);'); + + const watcher = rollup.watch({ + input: ID_MAIN, + output: { + format: 'es', + dir: path.join(TEMP_DIR, 'out') + }, + plugins + }); + + return new Promise((resolve, reject) => { + watcher.on('event', async event => { + if (event.code === 'ERROR') { + if (event.error.message !== 'Expected render error') { + reject(event.error); + } + await wait(200); + await outputFile(ID_MAIN, 'console.log(43);'); + } else if (event.code === 'BUNDLE_END') { + await event.result.close(); + resolve(); + } + }); + }).finally(async () => { + await watcher.close(); + await remove(TEMP_DIR); + for (const hook of hooks) { + assert.deepStrictEqual( + calledHooks[hook], + [ + ['pre-2', []], + ['pre-5', ['pre-2']], + 'pre-seq-8', + 'pre-seq-11', + 'null-seq-1', + 'undefined-seq-6', + ['null-7', []], + ['undefined-12', ['null-7']], + ['post-3', ['null-7', 'undefined-12']], + ['post-4', ['null-7', 'undefined-12', 'post-3']], + 'post-seq-9', + 'post-seq-10' + ], + hook + ); + } + }); + }); + describe('deprecated', () => { it('caches chunk emission in transform hook', () => { let cache; @@ -1377,195 +1571,5 @@ describe('hooks', () => { assert.strictEqual(output.source, 'hello world'); }); }); - - it('allows to enforce plugin hook order in watch mode', async () => { - const hooks = ['closeBundle', 'closeWatcher', 'renderError', 'watchChange', 'writeBundle']; - - const calledHooks = {}; - for (const hook of hooks) { - calledHooks[hook] = []; - } - - let first = true; - const plugins = [ - { - name: 'render-error', - renderChunk() { - if (first) { - first = false; - throw new Error('Expected render error'); - } - } - } - ]; - addPlugin(null); - addPlugin('pre'); - addPlugin('post'); - addPlugin('post'); - addPlugin('pre'); - addPlugin(undefined); - function addPlugin(order) { - const name = `${order}-${plugins.length}`; - const plugin = { name }; - for (const hook of hooks) { - plugin[hook] = { - order, - handler() { - if (!calledHooks[hook].includes(name)) { - calledHooks[hook].push(name); - } - } - }; - } - plugins.push(plugin); - } - - const ID_MAIN = path.join(TEMP_DIR, 'main.js'); - await outputFile(ID_MAIN, 'console.log(42);'); - - const watcher = rollup.watch({ - input: ID_MAIN, - output: { - format: 'es', - dir: path.join(TEMP_DIR, 'out') - }, - plugins - }); - - return new Promise((resolve, reject) => { - watcher.on('event', async event => { - if (event.code === 'ERROR') { - if (event.error.message !== 'Expected render error') { - reject(event.error); - } - await wait(200); - await outputFile(ID_MAIN, 'console.log(43);'); - } else if (event.code === 'BUNDLE_END') { - await event.result.close(); - resolve(); - } - }); - }).finally(async () => { - await watcher.close(); - await remove(TEMP_DIR); - for (const hook of hooks) { - assert.deepStrictEqual( - calledHooks[hook], - ['pre-2', 'pre-5', 'null-1', 'undefined-6', 'post-3', 'post-4'], - hook - ); - } - }); - }); - - it('allows to enforce sequential plugin hook order in watch mode', async () => { - const hooks = ['closeBundle', 'closeWatcher', 'renderError', 'watchChange', 'writeBundle']; - - const calledHooks = {}; - for (const hook of hooks) { - calledHooks[hook] = []; - } - - let first = true; - const plugins = [ - { - name: 'render-error', - renderChunk() { - if (first) { - first = false; - throw new Error('Expected render error'); - } - } - } - ]; - addPlugin(null, false); - addPlugin('pre', false); - addPlugin('post', true); - addPlugin('post', false); - addPlugin('pre', true); - addPlugin(undefined, true); - addPlugin(null, false); - addPlugin('pre', true); - addPlugin('post', false); - addPlugin('post', true); - addPlugin('pre', false); - addPlugin(undefined, true); - - let hookActive = false; - function addPlugin(order, sequential) { - const name = `${order}-${sequential ? 'seq-' : ''}${plugins.length}`; - const plugin = { name }; - for (const hook of hooks) { - plugin[hook] = { - order, - async handler() { - if (!calledHooks[hook].includes(name)) { - calledHooks[hook].push(name); - } - if (sequential) { - if (hookActive) { - throw new Error(`Detected parallel hook runs in ${hook}.`); - } - hookActive = true; - await wait(0); - hookActive = false; - } - }, - sequential - }; - } - plugins.push(plugin); - } - - const ID_MAIN = path.join(TEMP_DIR, 'main.js'); - await outputFile(ID_MAIN, 'console.log(42);'); - - const watcher = rollup.watch({ - input: ID_MAIN, - output: { - format: 'es', - dir: path.join(TEMP_DIR, 'out') - }, - plugins - }); - - return new Promise((resolve, reject) => { - watcher.on('event', async event => { - if (event.code === 'ERROR') { - if (event.error.message !== 'Expected render error') { - reject(event.error); - } - await wait(200); - await outputFile(ID_MAIN, 'console.log(43);'); - } else if (event.code === 'BUNDLE_END') { - await event.result.close(); - resolve(); - } - }); - }).finally(async () => { - await watcher.close(); - await remove(TEMP_DIR); - for (const hook of hooks) { - assert.deepStrictEqual( - calledHooks[hook], - [ - 'pre-2', - 'pre-11', - 'null-1', - 'null-7', - 'post-4', - 'post-9', - 'pre-seq-5', - 'pre-seq-8', - 'undefined-seq-6', - 'undefined-seq-12', - 'post-seq-3', - 'post-seq-10' - ], - hook - ); - } - }); - }); }); }); From b2af48a0fe83c6773b54c290cf72b355185b78b6 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 9 Aug 2022 07:24:41 +0200 Subject: [PATCH 12/15] Also allow to make banner/footer/intro/outro parallel --- src/utils/PluginDriver.ts | 28 +++++++++++-------- .../_config.js | 11 +++++++- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/utils/PluginDriver.ts b/src/utils/PluginDriver.ts index ee92a445c0c..daf20271a1c 100644 --- a/src/utils/PluginDriver.ts +++ b/src/utils/PluginDriver.ts @@ -220,22 +220,26 @@ export class PluginDriver { return arg0; } - // chains, reduces returned value to type T, handling the reduced value separately. permits hooks as values. - hookReduceValue( + // chains, reduces returned value to type string, handling the reduced value separately. permits hooks as values. + async hookReduceValue( hookName: H, - initialValue: T | Promise, + initialValue: string | Promise, args: Parameters, - reduce: (reduction: T, result: Awaited>, plugin: Plugin) => T - ): Promise { - let promise = Promise.resolve(initialValue); + reducer: (result: string, next: string) => string + ): Promise { + const results: string[] = []; + const parallelResults: (string | Promise)[] = []; for (const plugin of this.getSortedPlugins(hookName, validateAddonPluginHandler)) { - promise = promise.then(value => - this.runHook(hookName, args, plugin).then(result => - reduce.call(this.pluginContexts.get(plugin), value, result, plugin) - ) - ); + if ((plugin[hookName] as { sequential?: boolean }).sequential) { + results.push(...(await Promise.all(parallelResults))); + parallelResults.length = 0; + results.push(await this.runHook(hookName, args, plugin)); + } else { + parallelResults.push(this.runHook(hookName, args, plugin)); + } } - return promise; + results.push(...(await Promise.all(parallelResults))); + return results.reduce(reducer, await initialValue); } // chains synchronously, reduces returned value to type T, handling the reduced value separately. permits hooks as values. diff --git a/test/function/samples/enforce-sequential-plugin-order/_config.js b/test/function/samples/enforce-sequential-plugin-order/_config.js index ffcd4c00d7f..f774ed6bad6 100644 --- a/test/function/samples/enforce-sequential-plugin-order/_config.js +++ b/test/function/samples/enforce-sequential-plugin-order/_config.js @@ -1,7 +1,16 @@ const assert = require('assert'); const { wait } = require('../../../utils'); -const hooks = ['buildEnd', 'buildStart', 'moduleParsed', 'renderStart']; +const hooks = [ + 'banner', + 'buildEnd', + 'buildStart', + 'footer', + 'intro', + 'moduleParsed', + 'outro', + 'renderStart' +]; const calledHooks = {}; const activeHooks = {}; From f66d997e9f7dd85eda24787c13deaeb23805df2f Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 10 Aug 2022 07:20:39 +0200 Subject: [PATCH 13/15] Add documentation --- docs/05-plugin-development.md | 54 ++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/docs/05-plugin-development.md b/docs/05-plugin-development.md index 9d6fa5447a3..3fb8f7eed8f 100644 --- a/docs/05-plugin-development.md +++ b/docs/05-plugin-development.md @@ -51,7 +51,7 @@ export default ({ - Plugins should have a clear name with `rollup-plugin-` prefix. - Include `rollup-plugin` keyword in `package.json`. - Plugins should be tested. We recommend [mocha](https://github.com/mochajs/mocha) or [ava](https://github.com/avajs/ava) which support Promises out of the box. -- Use asynchronous methods when it is possible. +- Use asynchronous methods when it is possible, e.g. `fs.readFile` instead of `fs.readFileSync`. - Document your plugin in English. - Make sure your plugin outputs correct source mappings if appropriate. - If your plugin uses 'virtual modules' (e.g. for helper functions), prefix the module ID with `\0`. This prevents other plugins from trying to process it. @@ -66,12 +66,58 @@ The name of the plugin, for use in error messages and warnings. ### Build Hooks -To interact with the build process, your plugin object includes 'hooks'. Hooks are functions which are called at various stages of the build. Hooks can affect how a build is run, provide information about a build, or modify a build once complete. There are different kinds of hooks: +To interact with the build process, your plugin object includes "hooks". Hooks are functions which are called at various stages of the build. Hooks can affect how a build is run, provide information about a build, or modify a build once complete. There are different kinds of hooks: - `async`: The hook may also return a Promise resolving to the same type of value; otherwise, the hook is marked as `sync`. - `first`: If several plugins implement this hook, the hooks are run sequentially until a hook returns a value other than `null` or `undefined`. -- `sequential`: If several plugins implement this hook, all of them will be run in the specified plugin order. If a hook is async, subsequent hooks of this kind will wait until the current hook is resolved. -- `parallel`: If several plugins implement this hook, all of them will be run in the specified plugin order. If a hook is async, subsequent hooks of this kind will be run in parallel and not wait for the current hook. +- `sequential`: If several plugins implement this hook, all of them will be run in the specified plugin order. If a hook is `async`, subsequent hooks of this kind will wait until the current hook is resolved. +- `parallel`: If several plugins implement this hook, all of them will be run in the specified plugin order. If a hook is `async`, subsequent hooks of this kind will be run in parallel and not wait for the current hook. + +Instead of a function, hooks can also be objects. In that case, the actual hook function (or value for `banner/footer/intro/outro`) must be specified as `handler`. This allows you to provide additional optional properties that change hook execution: + +- `order: "pre" | "post" | null`
If there are several plugins implementing this hook, either run this plugin first (`"pre"`), last (`"post"`), or in the user-specified position (no value or `null`). + + ```js + export default function resolveFirst() { + return { + name: 'resolve-first', + resolveId: { + order: 'pre', + handler(source) { + if (source === 'external') { + return { id: source, external: true }; + } + return null; + } + } + }; + } + ``` + + If several plugins use `"pre"` or `"post"`, Rollup runs them in the user-specified order. This option can be used for all plugin hooks. For parallel hooks, it changes the order in which the synchronous part of the hook is run. + +- `sequential: boolean`
Do not run this hook in parallel with the same hook of other plugins. Can only be used for `parallel` hooks. Using this option will make Rollup await the results of all previous plugins, then execute the plugin hook, and then run the remaining plugins in parallel again. E.g. when you have plugins `A`, `B`, `C`, `D`, `E` that all implement the same parallel hook and the middle plugin `C` has `sequential: true`, then Rollup will first run `A + B` in parallel, then `C` on its own, then `D + E` in parallel. + + This can be useful when you need to run several command line tools in different [`writeBundle`](guide/en/#writebundle) hooks that depend on each other (note that if possible, it is recommended to add/remove files in the sequential [`generateBundle`](guide/en/#generatebundle) hook, though, which is faster, works with pure in-memory builds and permits other in-memory build plugins to see the files). You can combine this option with `order` for additional sorting. + + ```js + import { resolve } from 'node:path'; + import { readdir } from 'node:fs/promises'; + + export default function getFilesOnDisk() { + return { + name: 'getFilesOnDisk', + writeBundle: { + sequential: true, + order: 'post', + async handler({ dir }) { + const topLevelFiles = await readdir(resolve(dir)); + console.log(topLevelFiles); + } + } + }; + } + ``` Build hooks are run during the build phase, which is triggered by `rollup.rollup(inputOptions)`. They are mainly concerned with locating, providing and transforming input files before they are processed by Rollup. The first hook of the build phase is [`options`](guide/en/#options), the last one is always [`buildEnd`](guide/en/#buildend). If there is a build error, [`closeBundle`](guide/en/#closebundle) will be called after that. From 5ef9428464ea230c3a339022f8f11ec9f5068a54 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Fri, 12 Aug 2022 07:02:40 +0200 Subject: [PATCH 14/15] Fix types --- src/rollup/types.d.ts | 14 +++++++------- test/typescript/index.ts | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index e37f162761e..29e6cb4db37 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -464,18 +464,18 @@ export type ParallelPluginHooks = Exclude< export type AddonHooks = 'banner' | 'footer' | 'intro' | 'outro'; -type MakeAsync any> = ( - ...a: Parameters -) => ReturnType | Promise>; +type MakeAsync = Fn extends (this: infer This, ...args: infer Args) => infer Return + ? (this: This, ...args: Args) => Return | Promise + : never; -type ObjectHook> = - | T - | ({ handler: T; order?: 'pre' | 'post' | null } & O); +// eslint-disable-next-line @typescript-eslint/ban-types +type ObjectHook = T | ({ handler: T; order?: 'pre' | 'post' | null } & O); export type PluginHooks = { [K in keyof FunctionPluginHooks]: ObjectHook< K extends AsyncPluginHooks ? MakeAsync : FunctionPluginHooks[K], - K extends ParallelPluginHooks ? { sequential?: boolean } : Record + // eslint-disable-next-line @typescript-eslint/ban-types + K extends ParallelPluginHooks ? { sequential?: boolean } : {} >; }; diff --git a/test/typescript/index.ts b/test/typescript/index.ts index a8b838b388e..ee9d433fbe9 100644 --- a/test/typescript/index.ts +++ b/test/typescript/index.ts @@ -5,14 +5,45 @@ import * as rollup from './dist/rollup'; interface Options { extensions?: string | string[]; } + const plugin: rollup.PluginImpl = (options = {}) => { const extensions = options.extensions || ['.js']; - return { name: 'my-plugin' }; + return { + name: 'my-plugin', + resolveId: { + handler(source, importer, options) { + // @ts-expect-error source is typed as string + const s: number = source; + } + } + }; }; plugin(); plugin({ extensions: ['.js', 'json'] }); +const pluginHooks: rollup.Plugin = { + buildStart: { + handler() {}, + sequential: true + }, + async load(id) { + // @ts-expect-error id is typed as string + const i: number = id; + await this.resolve('rollup'); + }, + name: 'test', + resolveId: { + async handler(source, importer, options) { + await this.resolve('rollup'); + // @ts-expect-error source is typed as string + const s: number = source; + }, + // @ts-expect-error sequential is only supported for parallel hooks + sequential: true + } +}; + const amdOutputOptions: rollup.OutputOptions['amd'][] = [ {}, { From fd3d91f3b4249387d3f8e89f84c387a86eb96945 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Fri, 12 Aug 2022 07:35:02 +0200 Subject: [PATCH 15/15] Make tests more stable --- test/hooks/index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/hooks/index.js b/test/hooks/index.js index 8a22711c8be..1a6f71919f0 100644 --- a/test/hooks/index.js +++ b/test/hooks/index.js @@ -1157,6 +1157,7 @@ describe('hooks', () => { const ID_MAIN = path.join(TEMP_DIR, 'main.js'); await outputFile(ID_MAIN, 'console.log(42);'); + await wait(100); const watcher = rollup.watch({ input: ID_MAIN, @@ -1173,7 +1174,7 @@ describe('hooks', () => { if (event.error.message !== 'Expected render error') { reject(event.error); } - await wait(200); + await wait(300); await outputFile(ID_MAIN, 'console.log(43);'); } else if (event.code === 'BUNDLE_END') { await event.result.close(); @@ -1258,6 +1259,7 @@ describe('hooks', () => { const ID_MAIN = path.join(TEMP_DIR, 'main.js'); await outputFile(ID_MAIN, 'console.log(42);'); + await wait(100); const watcher = rollup.watch({ input: ID_MAIN, @@ -1274,7 +1276,7 @@ describe('hooks', () => { if (event.error.message !== 'Expected render error') { reject(event.error); } - await wait(200); + await wait(300); await outputFile(ID_MAIN, 'console.log(43);'); } else if (event.code === 'BUNDLE_END') { await event.result.close();