-
Notifications
You must be signed in to change notification settings - Fork 96
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: ensure consistent
ids across hooks and bundlers (#145)
Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
- Loading branch information
Showing
19 changed files
with
416 additions
and
42 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| /** | ||
| * Flag that is replaced with a boolean during build time. | ||
| * __DEV__ is false in the final library output, and it is | ||
| * true when the library is ad-hoc transpiled, ie. during tests. | ||
| * | ||
| * See "tsup.config.ts" and "vitest.config.ts" for more info. | ||
| */ | ||
| declare const __DEV__: boolean |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import { isAbsolute, normalize } from 'path' | ||
|
|
||
| /** | ||
| * Normalizes a given path when it's absolute. Normalizing means returning a new path by converting | ||
| * the input path to the native os format. This is useful in cases where we want to normalize | ||
| * the `id` argument of a hook. Any absolute ids should be in the default format | ||
| * of the operating system. Any relative imports or node_module imports should remain | ||
| * untouched. | ||
| * | ||
| * @param path - Path to normalize. | ||
| * @returns a new normalized path. | ||
| */ | ||
| export function normalizeAbsolutePath (path: string) { | ||
| if (isAbsolute(path)) { | ||
| return normalize(path) | ||
| } else { | ||
| return path | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| import * as path from 'path' | ||
| import { it, describe, expect, vi, afterEach, Mock } from 'vitest' | ||
| import { createUnplugin, UnpluginOptions } from '../../../src' | ||
| import { build } from '../utils' | ||
|
|
||
| const entryFilePath = path.resolve(__dirname, './test-src/entry.js') | ||
|
|
||
| function createUnpluginWithCallback ( | ||
| resolveIdCallback: UnpluginOptions['resolveId'], | ||
| transformIncludeCallback: UnpluginOptions['transformInclude'], | ||
| transformCallback: UnpluginOptions['transform'], | ||
| loadCallback: UnpluginOptions['load'] | ||
| ) { | ||
| return createUnplugin(() => ({ | ||
| name: 'test-plugin', | ||
| resolveId: resolveIdCallback, | ||
| transformInclude: transformIncludeCallback, | ||
| transform: transformCallback, | ||
| load: loadCallback | ||
| })) | ||
| } | ||
|
|
||
| // We extract this check because all bundlers should behave the same | ||
| function checkHookCalls ( | ||
| resolveIdCallback: Mock, | ||
| transformIncludeCallback: Mock, | ||
| transformCallback: Mock, | ||
| loadCallback: Mock | ||
| ): void { | ||
| // Ensure that all bundlers call the hooks the same amount of times | ||
| expect(resolveIdCallback).toHaveBeenCalledTimes(4) | ||
| expect(transformIncludeCallback).toHaveBeenCalledTimes(4) | ||
| expect(transformCallback).toHaveBeenCalledTimes(4) | ||
| expect(loadCallback).toHaveBeenCalledTimes(4) | ||
|
|
||
| // Ensure that each hook was called with 4 unique ids | ||
| expect(new Set(resolveIdCallback.mock.calls.map(call => call[0]))).toHaveLength(4) | ||
| expect(new Set(transformIncludeCallback.mock.calls.map(call => call[0]))).toHaveLength(4) | ||
| expect(new Set(transformCallback.mock.calls.map(call => call[1]))).toHaveLength(4) | ||
| expect(new Set(loadCallback.mock.calls.map(call => call[0]))).toHaveLength(4) | ||
|
|
||
| // Ensure that the `resolveId` hook was called with expected values | ||
| expect(resolveIdCallback).toHaveBeenCalledWith(entryFilePath, undefined, expect.anything()) | ||
| expect(resolveIdCallback).toHaveBeenCalledWith('./proxy-export', expect.anything(), expect.anything()) | ||
| expect(resolveIdCallback).toHaveBeenCalledWith('./sub-folder/named-export', expect.anything(), expect.anything()) | ||
| expect(resolveIdCallback).toHaveBeenCalledWith('./default-export', expect.anything(), expect.anything()) | ||
|
|
||
| // Ensure that the `transformInclude`, `transform` and `load` hooks were called with the same (absolute) ids | ||
| const ids = transformIncludeCallback.mock.calls.map(call => call[0]) | ||
| ids.forEach((id) => { | ||
| expect(path.isAbsolute(id)).toBe(true) | ||
| expect(transformCallback).toHaveBeenCalledWith(expect.anything(), id) | ||
| expect(loadCallback).toHaveBeenCalledWith(id) | ||
| }) | ||
| } | ||
|
|
||
| describe('id parameter should be consistent accross hooks and plugins', () => { | ||
| afterEach(() => { | ||
| vi.restoreAllMocks() | ||
| }) | ||
|
|
||
| it('vite', async () => { | ||
| const mockResolveIdHook = vi.fn(() => undefined) | ||
| const mockTransformIncludeHook = vi.fn(() => true) | ||
| const mockTransformHook = vi.fn(() => undefined) | ||
| const mockLoadHook = vi.fn(() => undefined) | ||
|
|
||
| const plugin = createUnpluginWithCallback( | ||
| mockResolveIdHook, | ||
| mockTransformIncludeHook, | ||
| mockTransformHook, | ||
| mockLoadHook | ||
| ).vite | ||
|
|
||
| await build.vite({ | ||
| clearScreen: false, | ||
| plugins: [{ ...plugin(), enforce: 'pre' }], // we need to define `enforce` here for the plugin to be run | ||
| build: { | ||
| lib: { | ||
| entry: entryFilePath, | ||
| name: 'TestLib' | ||
| }, | ||
| rollupOptions: { | ||
| external: ['path'] | ||
| }, | ||
| write: false // don't output anything | ||
| } | ||
| }) | ||
|
|
||
| checkHookCalls(mockResolveIdHook, mockTransformIncludeHook, mockTransformHook, mockLoadHook) | ||
| }) | ||
|
|
||
| it('rollup', async () => { | ||
| const mockResolveIdHook = vi.fn(() => undefined) | ||
| const mockTransformIncludeHook = vi.fn(() => true) | ||
| const mockTransformHook = vi.fn(() => undefined) | ||
| const mockLoadHook = vi.fn(() => undefined) | ||
|
|
||
| const plugin = createUnpluginWithCallback( | ||
| mockResolveIdHook, | ||
| mockTransformIncludeHook, | ||
| mockTransformHook, | ||
| mockLoadHook | ||
| ).rollup | ||
|
|
||
| await build.rollup({ | ||
| input: entryFilePath, | ||
| plugins: [plugin()], | ||
| external: ['path'] | ||
| }) | ||
|
|
||
| checkHookCalls(mockResolveIdHook, mockTransformIncludeHook, mockTransformHook, mockLoadHook) | ||
| }) | ||
|
|
||
| it('webpack', async () => { | ||
| const mockResolveIdHook = vi.fn(() => undefined) | ||
| const mockTransformIncludeHook = vi.fn(() => true) | ||
| const mockTransformHook = vi.fn(() => undefined) | ||
| const mockLoadHook = vi.fn(() => undefined) | ||
|
|
||
| const plugin = createUnpluginWithCallback( | ||
| mockResolveIdHook, | ||
| mockTransformIncludeHook, | ||
| mockTransformHook, | ||
| mockLoadHook | ||
| ).webpack | ||
|
|
||
| await new Promise<void>((resolve) => { | ||
| build.webpack( | ||
| { | ||
| entry: entryFilePath, | ||
| plugins: [plugin()], | ||
| externals: ['path'], | ||
| mode: 'production', | ||
| target: 'node' // needed for webpack 4 so it doesn't try to "browserify" any node externals and load addtional modules | ||
| }, | ||
| () => { | ||
| resolve() | ||
| } | ||
| ) | ||
| }) | ||
|
|
||
| checkHookCalls(mockResolveIdHook, mockTransformIncludeHook, mockTransformHook, mockLoadHook) | ||
| }) | ||
|
|
||
| it('esbuild', async () => { | ||
| const mockResolveIdHook = vi.fn(() => undefined) | ||
| const mockTransformIncludeHook = vi.fn(() => true) | ||
| const mockTransformHook = vi.fn(() => undefined) | ||
| const mockLoadHook = vi.fn(() => undefined) | ||
|
|
||
| const plugin = createUnpluginWithCallback( | ||
| mockResolveIdHook, | ||
| mockTransformIncludeHook, | ||
| mockTransformHook, | ||
| mockLoadHook | ||
| ).esbuild | ||
|
|
||
| await build.esbuild({ | ||
| entryPoints: [entryFilePath], | ||
| plugins: [plugin()], | ||
| bundle: true, // actually traverse imports | ||
| write: false, // don't pollute console | ||
| external: ['path'] | ||
| }) | ||
|
|
||
| checkHookCalls(mockResolveIdHook, mockTransformIncludeHook, mockTransformHook, mockLoadHook) | ||
| }) | ||
| }) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export default 'some string' |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import * as path from 'path' // test external modules | ||
| import { named, proxiedDefault } from './proxy-export' | ||
|
|
||
| // just some random code to use the imports | ||
| process.stdout.write(JSON.stringify({ | ||
| named, | ||
| proxiedDefault, | ||
| path: path.join(__dirname, __filename) | ||
| })) |
Oops, something went wrong.