From 2c1836a7a8d3c7a90228ddb7ff6174ef42064cfd Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 19 Oct 2022 17:41:26 +0200 Subject: [PATCH 1/9] Add typedefinition for writeBundle hook --- src/types.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/types.ts b/src/types.ts index b4f261a0..a88fb337 100644 --- a/src/types.ts +++ b/src/types.ts @@ -29,6 +29,7 @@ export interface UnpluginOptions { name: string; enforce?: 'post' | 'pre' | undefined; + // Build Hooks buildStart?: (this: UnpluginBuildContext) => Promise | void; buildEnd?: (this: UnpluginBuildContext) => Promise | void; transform?: (this: UnpluginBuildContext & UnpluginContext, code: string, id: string) => Thenable; @@ -36,6 +37,9 @@ export interface UnpluginOptions { resolveId?: (id: string, importer: string | undefined, options: { isEntry: boolean }) => Thenable watchChange?: (this: UnpluginBuildContext, id: string, change: { event: 'create' | 'update' | 'delete' }) => void + // Output Generation Hooks + writeBundle?: () => Promise | void + /** * Custom predicate function to filter modules to be loaded. * When omitted, all modules will be included (might have potential perf impact on Webpack). From 77ab46d5c83177d4938c0bf6eba64e8c1291645a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 19 Oct 2022 17:41:36 +0200 Subject: [PATCH 2/9] Add tests for writeBundle hook --- test/unit-tests/write-bundle/.gitignore | 1 + .../unit-tests/write-bundle/test-src/entry.js | 3 + .../write-bundle/test-src/import.js | 3 + .../write-bundle/write-bundle.test.ts | 131 ++++++++++++++++++ 4 files changed, 138 insertions(+) create mode 100644 test/unit-tests/write-bundle/.gitignore create mode 100644 test/unit-tests/write-bundle/test-src/entry.js create mode 100644 test/unit-tests/write-bundle/test-src/import.js create mode 100644 test/unit-tests/write-bundle/write-bundle.test.ts diff --git a/test/unit-tests/write-bundle/.gitignore b/test/unit-tests/write-bundle/.gitignore new file mode 100644 index 00000000..44cb9419 --- /dev/null +++ b/test/unit-tests/write-bundle/.gitignore @@ -0,0 +1 @@ +test-out diff --git a/test/unit-tests/write-bundle/test-src/entry.js b/test/unit-tests/write-bundle/test-src/entry.js new file mode 100644 index 00000000..d19867fe --- /dev/null +++ b/test/unit-tests/write-bundle/test-src/entry.js @@ -0,0 +1,3 @@ +import someString, { someOtherString } from './import' + +process.stdout.write(JSON.stringify({ someString, someOtherString })) diff --git a/test/unit-tests/write-bundle/test-src/import.js b/test/unit-tests/write-bundle/test-src/import.js new file mode 100644 index 00000000..4e3c30c2 --- /dev/null +++ b/test/unit-tests/write-bundle/test-src/import.js @@ -0,0 +1,3 @@ +export default 'some string' + +export const someOtherString = 'some other string' diff --git a/test/unit-tests/write-bundle/write-bundle.test.ts b/test/unit-tests/write-bundle/write-bundle.test.ts new file mode 100644 index 00000000..f485d50e --- /dev/null +++ b/test/unit-tests/write-bundle/write-bundle.test.ts @@ -0,0 +1,131 @@ +import * as path from 'path' +import * as fs from 'fs' +import { it, describe, expect, vi, afterEach, Mock, beforeAll } from 'vitest' +import { build, toArray } from '../utils' +import { createUnplugin, UnpluginOptions, VitePlugin } from 'unplugin' + +function createUnpluginWithCallback (writeBundleCallback: UnpluginOptions['writeBundle']) { + return createUnplugin(() => ({ + name: 'test-plugin', + writeBundle: writeBundleCallback + })) +} + +function generateMockWriteBundleHook (outputPath: string) { + return () => { + // We want to check that at the time the `writeBundle` hook is called, all + // build-artifacts have already been written to disk. + + const bundleExists = fs.existsSync(path.join(outputPath, 'output.js')) + const sourceMapExists = fs.existsSync(path.join(outputPath, 'output.js.map')) + + expect(bundleExists).toBe(true) + expect(sourceMapExists).toBe(true) + + return undefined + } +} + +// We extract this check because all bundlers should behave the same +function checkWriteBundleHook (writeBundleCallback: Mock): void { + expect(writeBundleCallback).toHaveBeenCalledOnce() +} + +describe('writeBundle hook', () => { + beforeAll(() => { + fs.rmSync(path.resolve(__dirname, 'test-out'), { recursive: true, force: true }) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + it('vite', async () => { + expect.assertions(3) + const mockWriteBundleHook = vi.fn(generateMockWriteBundleHook(path.resolve(__dirname, 'test-out/vite'))) + const plugin = createUnpluginWithCallback(mockWriteBundleHook).vite + // we need to define `enforce` here for the plugin to be run + const plugins = toArray(plugin()).map((plugin): VitePlugin => ({ ...plugin, enforce: 'pre' })) + + await build.vite({ + clearScreen: false, + plugins: [plugins], + build: { + lib: { + entry: path.resolve(__dirname, 'test-src/entry.js'), + name: 'TestLib', + fileName: 'output', + formats: ['cjs'] + }, + outDir: path.resolve(__dirname, 'test-out/vite'), + sourcemap: true + } + }) + + checkWriteBundleHook(mockWriteBundleHook) + }) + + it('rollup', async () => { + expect.assertions(3) + const mockResolveIdHook = vi.fn(generateMockWriteBundleHook(path.resolve(__dirname, 'test-out/rollup'))) + const plugin = createUnpluginWithCallback(mockResolveIdHook).rollup + + const rollupBuild = await build.rollup({ + input: path.resolve(__dirname, 'test-src/entry.js') + }) + + await rollupBuild.write({ + plugins: [plugin()], + file: path.resolve(__dirname, 'test-out/rollup/output.js'), + format: 'cjs', + exports: 'named', + sourcemap: true + }) + + checkWriteBundleHook(mockResolveIdHook) + }) + + it('webpack', async () => { + expect.assertions(3) + const mockResolveIdHook = vi.fn(generateMockWriteBundleHook(path.resolve(__dirname, 'test-out/webpack'))) + const plugin = createUnpluginWithCallback(mockResolveIdHook).webpack + + await new Promise((resolve) => { + build.webpack( + { + entry: path.resolve(__dirname, 'test-src/entry.js'), + plugins: [plugin()], + devtool: 'source-map', + output: { + path: path.resolve(__dirname, 'test-out/webpack'), + filename: 'output.js', + libraryTarget: 'commonjs', + library: { + type: 'commonjs' + } + } + }, + resolve + ) + }) + + checkWriteBundleHook(mockResolveIdHook) + }) + + it('esbuild', async () => { + expect.assertions(3) + const mockResolveIdHook = vi.fn(generateMockWriteBundleHook(path.resolve(__dirname, 'test-out/esbuild'))) + const plugin = createUnpluginWithCallback(mockResolveIdHook).esbuild + + await build.esbuild({ + entryPoints: [path.resolve(__dirname, 'test-src/entry.js')], + plugins: [plugin()], + bundle: true, // actually traverse imports + outfile: path.resolve(__dirname, 'test-out/esbuild/output.js'), + format: 'cjs', + sourcemap: true + }) + + checkWriteBundleHook(mockResolveIdHook) + }) +}) From 1289c4946a0e3936a10a1503eb5106cfd6653ff6 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 19 Oct 2022 18:04:55 +0200 Subject: [PATCH 3/9] Add hook call in esbuild --- src/esbuild/index.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/esbuild/index.ts b/src/esbuild/index.ts index a509052c..5feb962f 100644 --- a/src/esbuild/index.ts +++ b/src/esbuild/index.ts @@ -60,14 +60,21 @@ export function getEsbuildPlugin ( onStart(() => plugin.buildStart!.call(context)) } - if (plugin.buildEnd || initialOptions.watch) { + if (plugin.buildEnd || plugin.writeBundle || initialOptions.watch) { const rebuild = () => build({ ...initialOptions, watch: false }) onEnd(async () => { - await plugin.buildEnd!.call(context) + if (plugin.buildEnd) { + await plugin.buildEnd.call(context) + } + + if (plugin.writeBundle) { + await plugin.writeBundle() + } + if (initialOptions.watch) { Object.keys(watchListRecord).forEach((id) => { if (!watchList.has(id)) { From 359adf5c40a4964b8ebdd21fa00d3c12702ce229 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 19 Oct 2022 18:05:02 +0200 Subject: [PATCH 4/9] Add hook call in webpack --- src/webpack/index.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/webpack/index.ts b/src/webpack/index.ts index e5fa123d..b44f3bc7 100644 --- a/src/webpack/index.ts +++ b/src/webpack/index.ts @@ -218,6 +218,12 @@ export function getWebpackPlugin ( await plugin.buildEnd!.call(createContext(compilation)) }) } + + if (plugin.writeBundle) { + compiler.hooks.afterEmit.tap(plugin.name, () => { + plugin.writeBundle!() + }) + } } } } From 94dabcb5189c32958f899c918551c47db8c222f5 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 19 Oct 2022 18:08:45 +0200 Subject: [PATCH 5/9] Add writeBundle hook to README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 167fa7cf..50630645 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,7 @@ Currently supports: | [`transform`](https://rollupjs.org/guide/en/#transformers) | ✅ | ✅ | ✅ | ✅ | ✅ 3 | | [`watchChange`](https://rollupjs.org/guide/en/#watchchange) | ✅ | ✅ | ✅ | ✅ | ✅ | | [`buildEnd`](https://rollupjs.org/guide/en/#buildend) | ✅ | ✅ | ✅ | ✅ | ✅ | +| [`writeBundle`](https://rollupjs.org/guide/en/#writebundle) | ✅ | ✅ | ✅ | ✅ | ✅ | 1. Rollup and esbuild do not support using `enforce` to control the order of plugins. Users need to maintain the order manually. 2. Webpack's id filter is outside of loader logic; an additional hook is needed for better perf on Webpack. In Rollup and Vite, this hook has been polyfilled to match the behaviors. See for following usage examples. From 0b964997818b72459e52742afaa7505420bb7111 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 19 Oct 2022 18:42:45 +0200 Subject: [PATCH 6/9] Try fix tests for webpack 4 vs. 5 --- test/unit-tests/utils.ts | 2 + .../write-bundle/write-bundle.test.ts | 42 ++++++++++++------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/test/unit-tests/utils.ts b/test/unit-tests/utils.ts index 74647c60..d594f31e 100644 --- a/test/unit-tests/utils.ts +++ b/test/unit-tests/utils.ts @@ -10,6 +10,8 @@ export const rollupBuild = rollup.rollup export const esbuildBuild = esbuild.build export const webpackBuild = (webpack.webpack || (webpack as any).default || webpack) as typeof webpack.webpack +export const webpackVersion = ((webpack as any).default || webpack).version + export const build = { webpack: webpackBuild, rollup: rollupBuild, diff --git a/test/unit-tests/write-bundle/write-bundle.test.ts b/test/unit-tests/write-bundle/write-bundle.test.ts index f485d50e..af498937 100644 --- a/test/unit-tests/write-bundle/write-bundle.test.ts +++ b/test/unit-tests/write-bundle/write-bundle.test.ts @@ -1,7 +1,7 @@ import * as path from 'path' import * as fs from 'fs' import { it, describe, expect, vi, afterEach, Mock, beforeAll } from 'vitest' -import { build, toArray } from '../utils' +import { build, toArray, webpackVersion } from '../utils' import { createUnplugin, UnpluginOptions, VitePlugin } from 'unplugin' function createUnpluginWithCallback (writeBundleCallback: UnpluginOptions['writeBundle']) { @@ -90,21 +90,33 @@ describe('writeBundle hook', () => { const mockResolveIdHook = vi.fn(generateMockWriteBundleHook(path.resolve(__dirname, 'test-out/webpack'))) const plugin = createUnpluginWithCallback(mockResolveIdHook).webpack + const webpack4Options = { + entry: path.resolve(__dirname, 'test-src/entry.js'), + cache: false, + output: { + path: path.resolve(__dirname, 'test-out/webpack'), + filename: 'output.js', + libraryTarget: 'commonjs' + }, + plugins: [plugin()], + devtool: 'source-map' + } + + const webpack5Options = { + entry: path.resolve(__dirname, 'test-src/entry.js'), + plugins: [plugin()], + devtool: 'source-map', + output: { + path: path.resolve(__dirname, 'test-out/webpack'), + filename: 'output.js', + library: { + type: 'commonjs' + } + } + } + await new Promise((resolve) => { - build.webpack( - { - entry: path.resolve(__dirname, 'test-src/entry.js'), - plugins: [plugin()], - devtool: 'source-map', - output: { - path: path.resolve(__dirname, 'test-out/webpack'), - filename: 'output.js', - libraryTarget: 'commonjs', - library: { - type: 'commonjs' - } - } - }, + build.webpack(webpackVersion!.startsWith('4') ? webpack4Options : webpack5Options, resolve ) }) From 75cdc114ca6f7a6e783bdba223e027e44556b849 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 20 Oct 2022 12:28:16 +0200 Subject: [PATCH 7/9] Retrigger CI From d1f9fd09bdda3b6bbe6dfb1698fa6a18a20024e1 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sat, 22 Oct 2022 12:50:14 +0200 Subject: [PATCH 8/9] docs: add note in README that `writeBundle` currently doesn't pass arguments --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 50630645..a67730d5 100644 --- a/README.md +++ b/README.md @@ -27,11 +27,12 @@ Currently supports: | [`transform`](https://rollupjs.org/guide/en/#transformers) | ✅ | ✅ | ✅ | ✅ | ✅ 3 | | [`watchChange`](https://rollupjs.org/guide/en/#watchchange) | ✅ | ✅ | ✅ | ✅ | ✅ | | [`buildEnd`](https://rollupjs.org/guide/en/#buildend) | ✅ | ✅ | ✅ | ✅ | ✅ | -| [`writeBundle`](https://rollupjs.org/guide/en/#writebundle) | ✅ | ✅ | ✅ | ✅ | ✅ | +| [`writeBundle`](https://rollupjs.org/guide/en/#writebundle)4 | ✅ | ✅ | ✅ | ✅ | ✅ | 1. Rollup and esbuild do not support using `enforce` to control the order of plugins. Users need to maintain the order manually. 2. Webpack's id filter is outside of loader logic; an additional hook is needed for better perf on Webpack. In Rollup and Vite, this hook has been polyfilled to match the behaviors. See for following usage examples. 3. Although esbuild can handle both JavaScript and CSS and many other file formats, you can only return JavaScript in `load` and `transform` results. +4. The `writeBundle` hook currently doesn't pass any arguments. ### Hook Context From 7e1824f5ea7c703c81b037661987bcd998d61705 Mon Sep 17 00:00:00 2001 From: Anthony Fu Date: Sat, 22 Oct 2022 19:07:42 +0800 Subject: [PATCH 9/9] chore: update --- README.md | 12 ++++++------ src/types.ts | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index a67730d5..e1a8a11f 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ Currently supports: 1. Rollup and esbuild do not support using `enforce` to control the order of plugins. Users need to maintain the order manually. 2. Webpack's id filter is outside of loader logic; an additional hook is needed for better perf on Webpack. In Rollup and Vite, this hook has been polyfilled to match the behaviors. See for following usage examples. 3. Although esbuild can handle both JavaScript and CSS and many other file formats, you can only return JavaScript in `load` and `transform` results. -4. The `writeBundle` hook currently doesn't pass any arguments. +4. Currently, `writeBundle` is only serves as a hook for the timing. It doesn't pass any arguments. ### Hook Context @@ -42,13 +42,13 @@ Currently supports: | ---- | :----: | :--: | :-------: | :-------: | :-----: | | [`this.parse`](https://rollupjs.org/guide/en/#thisparse) | ✅ | ✅ | ✅ | ✅ | ✅ | | [`this.addWatchFile`](https://rollupjs.org/guide/en/#thisaddwatchfile) | ✅ | ✅ | ✅ | ✅ | ✅ | -| [`this.emitFile`](https://rollupjs.org/guide/en/#thisemitfile)4 | ✅ | ✅ | ✅ | ✅ | ✅ | +| [`this.emitFile`](https://rollupjs.org/guide/en/#thisemitfile)5 | ✅ | ✅ | ✅ | ✅ | ✅ | | [`this.getWatchFiles`](https://rollupjs.org/guide/en/#thisgetwatchfiles) | ✅ | ✅ | ✅ | ✅ | ✅ | | [`this.warn`](https://rollupjs.org/guide/en/#thiswarn) | ✅ | ✅ | ✅ | ✅ | ✅ | | [`this.error`](https://rollupjs.org/guide/en/#thiserror) | ✅ | ✅ | ✅ | ✅ | ✅ | -4. Currently, [`this.emitFile`](https://rollupjs.org/guide/en/#thisemitfile) only supports the `EmittedAsset` variant. +5. Currently, [`this.emitFile`](https://rollupjs.org/guide/en/#thisemitfile) only supports the `EmittedAsset` variant. ## Usage @@ -85,10 +85,10 @@ Since `v0.10.0`, unplugin supports constructing multiple nested plugins to behav | Rollup | Vite | Webpack 4 | Webpack 5 | esbuild | | :----: | :--: | :-------: | :-------: | :-----: | -| ✅ `>=3.1`5 | ✅ | ✅ | ✅ | ⚠️6 | +| ✅ `>=3.1`6 | ✅ | ✅ | ✅ | ⚠️7 | -5. Rollup supports nested plugins since [v3.1.0](https://github.com/rollup/rollup/releases/tag/v3.1.0). Plugin aurthor should ask users to a have a Rollup version of `>=3.1.0` when using nested plugins. For singe plugin format, unplugin works for any versions of Rollup. -6. Since esbuild does not have a built-in transform phase, the `transform` hook of nested plugin will not work on esbuild yet. Other hooks like `load` or `resolveId` work fine. We will try to find a way to support it in the future. +6. Rollup supports nested plugins since [v3.1.0](https://github.com/rollup/rollup/releases/tag/v3.1.0). Plugin aurthor should ask users to a have a Rollup version of `>=3.1.0` when using nested plugins. For singe plugin format, unplugin works for any versions of Rollup. +7. Since esbuild does not have a built-in transform phase, the `transform` hook of nested plugin will not work on esbuild yet. Other hooks like `load` or `resolveId` work fine. We will try to find a way to support it in the future. ###### Usage diff --git a/src/types.ts b/src/types.ts index a88fb337..04f09252 100644 --- a/src/types.ts +++ b/src/types.ts @@ -38,7 +38,7 @@ export interface UnpluginOptions { watchChange?: (this: UnpluginBuildContext, id: string, change: { event: 'create' | 'update' | 'delete' }) => void // Output Generation Hooks - writeBundle?: () => Promise | void + writeBundle?: (this: void) => Promise | void /** * Custom predicate function to filter modules to be loaded.