From 59db787bfcffaeb1394d8417b561122e0be0c4cb Mon Sep 17 00:00:00 2001 From: Ryota Watanabe <43837308+wattanx@users.noreply.github.com> Date: Thu, 3 Aug 2023 20:17:20 +0900 Subject: [PATCH] fix: do not inject imports within `node_modules` (#873) --- .github/workflows/ci.yml | 23 +++++++ package.json | 3 +- packages/bridge/src/imports/module.ts | 1 + packages/bridge/src/imports/transform.ts | 22 +++++-- packages/bridge/test/auto-imports.test.ts | 74 +++++++++++++++++++++++ 5 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 packages/bridge/test/auto-imports.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 021ed638..ece97dbc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -117,6 +117,28 @@ jobs: - name: Test (fixtures with dev) run: pnpm test:fixtures:webpack:dev + test-unit: + runs-on: ${{ matrix.os }} + + strategy: + matrix: + os: [ubuntu-latest, windows-latest] + node: [16] + + steps: + - uses: actions/checkout@v3 + - run: corepack enable + - uses: actions/setup-node@v3 + with: + node-version: ${{ matrix.node }} + cache: "pnpm" + + - name: Install dependencies + run: pnpm install + + - name: Test (Unit) + run: pnpm test:unit + build-release: permissions: id-token: write @@ -130,6 +152,7 @@ jobs: - build - test-fixtures - test-fixtures-webpack + - test-unit runs-on: ${{ matrix.os }} strategy: diff --git a/package.json b/package.json index 323615b9..8394abf1 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,8 @@ "test:fixtures": "pnpm dev:prepare && JITI_ESM_RESOLVE=1 vitest run --dir test", "test:fixtures:dev": "NUXT_TEST_DEV=true pnpm test:fixtures", "test:fixtures:webpack": "TEST_WITH_WEBPACK=1 pnpm test:fixtures", - "test:fixtures:webpack:dev": "TEST_WITH_WEBPACK=1 NUXT_TEST_DEV=true pnpm test:fixtures" + "test:fixtures:webpack:dev": "TEST_WITH_WEBPACK=1 NUXT_TEST_DEV=true pnpm test:fixtures", + "test:unit": "vitest run --dir packages" }, "devDependencies": { "@nuxt/test-utils": "^3.6.5", diff --git a/packages/bridge/src/imports/module.ts b/packages/bridge/src/imports/module.ts index 77223097..89473a2f 100644 --- a/packages/bridge/src/imports/module.ts +++ b/packages/bridge/src/imports/module.ts @@ -18,6 +18,7 @@ export default defineNuxtModule>({ imports: [], dirs: [], transform: { + include: [], exclude: undefined } }, diff --git a/packages/bridge/src/imports/transform.ts b/packages/bridge/src/imports/transform.ts index 071024fe..6f00685e 100644 --- a/packages/bridge/src/imports/transform.ts +++ b/packages/bridge/src/imports/transform.ts @@ -3,6 +3,10 @@ import { createUnplugin } from 'unplugin' import { parseQuery, parseURL } from 'ufo' import { Unimport } from 'unimport' import { ImportsOptions } from '@nuxt/schema' +import { normalize } from 'pathe' + +const NODE_MODULES_RE = /[\\/]node_modules[\\/]/ +const IMPORTS_RE = /(['"])#imports\1/ export const TransformPlugin = createUnplugin(({ ctx, options }: { ctx: Unimport, options: Partial }) => { return { @@ -12,10 +16,12 @@ export const TransformPlugin = createUnplugin(({ ctx, options }: { ctx: Unimport const { pathname, search } = parseURL(decodeURIComponent(pathToFileURL(id).href)) const { type, macro } = parseQuery(search) - const exclude = options.transform?.exclude || [/[\\/]node_modules[\\/]/] - - // Exclude node_modules by default - if (exclude.some(pattern => id.match(pattern))) { + // Included + if (options.transform?.include?.some(pattern => pattern.test(id))) { + return true + } + // Excluded + if (options.transform?.exclude?.some(pattern => pattern.test(id))) { return false } @@ -33,7 +39,13 @@ export const TransformPlugin = createUnplugin(({ ctx, options }: { ctx: Unimport } }, async transform (_code, id) { - const { code, s } = await ctx.injectImports(_code) + id = normalize(id) + const isNodeModule = NODE_MODULES_RE.test(id) && !options.transform?.include?.some(pattern => pattern.test(id)) + // For modules in node_modules, we only transform `#imports` but not doing imports + if (isNodeModule && !IMPORTS_RE.test(_code)) { + return + } + const { code, s } = await ctx.injectImports(_code, id, { autoImport: options.autoImport && !isNodeModule }) if (code === _code) { return } diff --git a/packages/bridge/test/auto-imports.test.ts b/packages/bridge/test/auto-imports.test.ts new file mode 100644 index 00000000..3652e803 --- /dev/null +++ b/packages/bridge/test/auto-imports.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from 'vitest' +import * as VueFunctions from 'vue' +import type { Import } from 'unimport' +import { createUnimport } from 'unimport' +import type { Plugin } from 'vite' +import { TransformPlugin } from '../src/imports/transform' +import { defaultPresets } from '../src/imports/presets' + +describe('imports:transform', () => { + const imports: Import[] = [ + { name: 'ref', as: 'ref', from: 'vue' }, + { name: 'computed', as: 'computed', from: 'bar' }, + { name: 'foo', as: 'foo', from: 'excluded' } + ] + + const ctx = createUnimport({ + imports + }) + + const transformPlugin = TransformPlugin.raw({ ctx, options: { transform: { exclude: [/node_modules/] } } }, { framework: 'rollup' }) as Plugin + const transform = async (source: string) => { + const result = await (transformPlugin.transform! as Function).call({ error: null, warn: null } as any, source, '') + return typeof result === 'string' ? result : result?.code + } + + it('should correct inject', async () => { + expect(await transform('const a = ref(0)')).toMatchInlineSnapshot('"import { ref } from \'vue\';\nconst a = ref(0)"') + }) + + it('should ignore existing imported', async () => { + expect(await transform('import { ref } from "foo"; const a = ref(0)')).to.equal(undefined) + expect(await transform('import { computed as ref } from "foo"; const a = ref(0)')).to.equal(undefined) + expect(await transform('import ref from "foo"; const a = ref(0)')).to.equal(undefined) + expect(await transform('import { z as ref } from "foo"; const a = ref(0)')).to.equal(undefined) + expect(await transform('let ref = () => {}; const a = ref(0)')).to.equal(undefined) + expect(await transform('let { ref } = Vue; const a = ref(0)')).to.equal(undefined) + expect(await transform('let [\ncomputed,\nref\n] = Vue; const a = ref(0); const b = ref(0)')).to.equal(undefined) + }) + + it('should ignore comments', async () => { + const result = await transform('// import { computed } from "foo"\n;const a = computed(0)') + expect(result).toMatchInlineSnapshot(` + "import { computed } from 'bar'; + // import { computed } from \\"foo\\" + ;const a = computed(0)" + `) + }) + + it('should exclude files from transform', async () => { + expect(await transform('excluded')).toEqual(undefined) + }) +}) + +const excludedVueHelpers = [ + 'mergeDefaults', + 'version', + 'warn', + 'watchPostEffect', + 'watchSyncEffect', + 'set', + 'del', + 'default' +] + +describe('imports:vue', () => { + for (const name of Object.keys(VueFunctions)) { + if (excludedVueHelpers.includes(name)) { + continue + } + it(`should register ${name} globally`, () => { + expect(defaultPresets.find(a => a.from === 'vue')!.imports).toContain(name) + }) + } +})