From 5f10b65ed15f353eeffbc70ffdf4ece31a7becf9 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Wed, 18 Dec 2024 12:09:35 -0500 Subject: [PATCH 1/9] fix(ssrTransform): preserve line offset when transforming imports --- packages/vite/src/node/ssr/ssrTransform.ts | 31 +++++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrTransform.ts b/packages/vite/src/node/ssr/ssrTransform.ts index c4552f73229663..077e2c2026ed33 100644 --- a/packages/vite/src/node/ssr/ssrTransform.ts +++ b/packages/vite/src/node/ssr/ssrTransform.ts @@ -137,21 +137,38 @@ async function ssrTransformScript( } const metadataStr = metadata ? `, ${JSON.stringify(metadata)}` : '' + // Track how many lines the original import statement spans, so we can preserve the line offset. + let linesSpanned = 1 + for (let i = importNode.start; i < importNode.end; i++) { + if (code[i] === '\n') { + linesSpanned++ + } + } + s.update( importNode.start, importNode.end, `const ${importId} = await ${ssrImportKey}(${JSON.stringify( source, - )}${metadataStr});\n`, + )}${metadataStr});${'\n'.repeat(linesSpanned - 1)}`, ) - if (importNode.start === index) { - // no need to hoist, but update hoistIndex to keep the order - hoistIndex = importNode.end - } else { - // There will be an error if the module is called before it is imported, - // so the module import statement is hoisted to the top + // Check for non-whitespace characters between the last import and the + // current one. + const nonWhitespaceRegex = /\S/g + nonWhitespaceRegex.lastIndex = index + nonWhitespaceRegex.exec(code) + + // TODO: Account for comments between imports. + if (importNode.start > nonWhitespaceRegex.lastIndex) { + // By moving the import to the top of the module, we ensure that it's + // imported before it's used. s.move(importNode.start, importNode.end, index) + } else { + // Only update hoistIndex when not hoisting the current import. This + // ensures that once any import in this module has been hoisted, all + // remaining imports will also be hoisted. + hoistIndex = importNode.end } return importId From b8c5ffeeb752f7839b1b8650bf4d6c75e1ca927b Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Wed, 18 Dec 2024 13:20:02 -0500 Subject: [PATCH 2/9] update test snapshots --- .../node/ssr/__tests__/ssrTransform.spec.ts | 210 +++++------------- 1 file changed, 59 insertions(+), 151 deletions(-) diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index e8570c6d4afedf..4b79eca970497d 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts @@ -14,10 +14,7 @@ const ssrTransformSimpleCode = async (code: string, url?: string) => test('default import', async () => { expect( await ssrTransformSimpleCode(`import foo from 'vue';console.log(foo.bar)`), - ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["default"]}); - console.log(__vite_ssr_import_0__.default.bar)" - `) + ).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["default"]});console.log(__vite_ssr_import_0__.default.bar)"`) }) test('named import', async () => { @@ -25,10 +22,7 @@ test('named import', async () => { await ssrTransformSimpleCode( `import { ref } from 'vue';function foo() { return ref(0) }`, ), - ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["ref"]}); - function foo() { return (0,__vite_ssr_import_0__.ref)(0) }" - `) + ).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["ref"]});function foo() { return (0,__vite_ssr_import_0__.ref)(0) }"`) }) test('named import: arbitrary module namespace specifier', async () => { @@ -36,10 +30,7 @@ test('named import: arbitrary module namespace specifier', async () => { await ssrTransformSimpleCode( `import { "some thing" as ref } from 'vue';function foo() { return ref(0) }`, ), - ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["some thing"]}); - function foo() { return (0,__vite_ssr_import_0__["some thing"])(0) }" - `) + ).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["some thing"]});function foo() { return (0,__vite_ssr_import_0__["some thing"])(0) }"`) }) test('namespace import', async () => { @@ -47,10 +38,7 @@ test('namespace import', async () => { await ssrTransformSimpleCode( `import * as vue from 'vue';function foo() { return vue.ref(0) }`, ), - ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue"); - function foo() { return __vite_ssr_import_0__.ref(0) }" - `) + ).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue");function foo() { return __vite_ssr_import_0__.ref(0) }"`) }) test('export function declaration', async () => { @@ -93,7 +81,6 @@ test('export named from', async () => { await ssrTransformSimpleCode(`export { ref, computed as c } from 'vue'`), ).toMatchInlineSnapshot(` "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["ref","computed"]}); - Object.defineProperty(__vite_ssr_exports__, "ref", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.ref }}); Object.defineProperty(__vite_ssr_exports__, "c", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.computed }});" `) @@ -106,7 +93,6 @@ test('named exports of imported binding', async () => { ), ).toMatchInlineSnapshot(` "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["createApp"]}); - Object.defineProperty(__vite_ssr_exports__, "createApp", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.createApp }});" `) }) @@ -117,11 +103,9 @@ test('export * from', async () => { `export * from 'vue'\n` + `export * from 'react'`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue"); - __vite_ssr_exportAll__(__vite_ssr_import_0__); + "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue");__vite_ssr_exportAll__(__vite_ssr_import_0__); ; - const __vite_ssr_import_1__ = await __vite_ssr_import__("react"); - __vite_ssr_exportAll__(__vite_ssr_import_1__); + const __vite_ssr_import_1__ = await __vite_ssr_import__("react");__vite_ssr_exportAll__(__vite_ssr_import_1__); " `) }) @@ -130,7 +114,6 @@ test('export * as from', async () => { expect(await ssrTransformSimpleCode(`export * as foo from 'vue'`)) .toMatchInlineSnapshot(` "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue"); - Object.defineProperty(__vite_ssr_exports__, "foo", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__ }});" `) }) @@ -140,7 +123,6 @@ test('export * as from arbitrary module namespace identifier', async () => { await ssrTransformSimpleCode(`export * as "arbitrary string" from 'vue'`), ).toMatchInlineSnapshot(` "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue"); - Object.defineProperty(__vite_ssr_exports__, "arbitrary string", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__ }});" `) }) @@ -163,7 +145,6 @@ test('export as from arbitrary module namespace identifier', async () => { ), ).toMatchInlineSnapshot(` "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["arbitrary string2"]}); - Object.defineProperty(__vite_ssr_exports__, "arbitrary string", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__["arbitrary string2"] }});" `) }) @@ -180,9 +161,7 @@ test('export then import minified', async () => { `export * from 'vue';import {createApp} from 'vue';`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["createApp"]}); - const __vite_ssr_import_1__ = await __vite_ssr_import__("vue"); - __vite_ssr_exportAll__(__vite_ssr_import_1__); + "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["createApp"]});const __vite_ssr_import_1__ = await __vite_ssr_import__("vue");__vite_ssr_exportAll__(__vite_ssr_import_1__); " `) }) @@ -192,10 +171,7 @@ test('hoist import to top', async () => { await ssrTransformSimpleCode( `path.resolve('server.js');import path from 'node:path';`, ), - ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("node:path", {"importedNames":["default"]}); - __vite_ssr_import_0__.default.resolve('server.js');" - `) + ).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("node:path", {"importedNames":["default"]});__vite_ssr_import_0__.default.resolve('server.js');"`) }) test('import.meta', async () => { @@ -220,10 +196,7 @@ test('do not rewrite method definition', async () => { const result = await ssrTransformSimple( `import { fn } from 'vue';class A { fn() { fn() } }`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]}); - class A { fn() { (0,__vite_ssr_import_0__.fn)() } }" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});class A { fn() { (0,__vite_ssr_import_0__.fn)() } }"`) expect(result?.deps).toEqual(['vue']) }) @@ -231,10 +204,7 @@ test('do not rewrite when variable is in scope', async () => { const result = await ssrTransformSimple( `import { fn } from 'vue';function A(){ const fn = () => {}; return { fn }; }`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]}); - function A(){ const fn = () => {}; return { fn }; }" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ const fn = () => {}; return { fn }; }"`) expect(result?.deps).toEqual(['vue']) }) @@ -243,10 +213,7 @@ test('do not rewrite when variable is in scope with object destructuring', async const result = await ssrTransformSimple( `import { fn } from 'vue';function A(){ let {fn, test} = {fn: 'foo', test: 'bar'}; return { fn }; }`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]}); - function A(){ let {fn, test} = {fn: 'foo', test: 'bar'}; return { fn }; }" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ let {fn, test} = {fn: 'foo', test: 'bar'}; return { fn }; }"`) expect(result?.deps).toEqual(['vue']) }) @@ -255,10 +222,7 @@ test('do not rewrite when variable is in scope with array destructuring', async const result = await ssrTransformSimple( `import { fn } from 'vue';function A(){ let [fn, test] = ['foo', 'bar']; return { fn }; }`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]}); - function A(){ let [fn, test] = ['foo', 'bar']; return { fn }; }" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ let [fn, test] = ['foo', 'bar']; return { fn }; }"`) expect(result?.deps).toEqual(['vue']) }) @@ -267,10 +231,7 @@ test('rewrite variable in string interpolation in function nested arguments', as const result = await ssrTransformSimple( `import { fn } from 'vue';function A({foo = \`test\${fn}\`} = {}){ return {}; }`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]}); - function A({foo = \`test\${__vite_ssr_import_0__.fn}\`} = {}){ return {}; }" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A({foo = \`test\${__vite_ssr_import_0__.fn}\`} = {}){ return {}; }"`) expect(result?.deps).toEqual(['vue']) }) @@ -279,10 +240,7 @@ test('rewrite variables in default value of destructuring params', async () => { const result = await ssrTransformSimple( `import { fn } from 'vue';function A({foo = fn}){ return {}; }`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]}); - function A({foo = __vite_ssr_import_0__.fn}){ return {}; }" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A({foo = __vite_ssr_import_0__.fn}){ return {}; }"`) expect(result?.deps).toEqual(['vue']) }) @@ -290,10 +248,7 @@ test('do not rewrite when function declaration is in scope', async () => { const result = await ssrTransformSimple( `import { fn } from 'vue';function A(){ function fn() {}; return { fn }; }`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]}); - function A(){ function fn() {}; return { fn }; }" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ function fn() {}; return { fn }; }"`) expect(result?.deps).toEqual(['vue']) }) @@ -302,10 +257,7 @@ test('do not rewrite when function expression is in scope', async () => { const result = await ssrTransformSimple( `import {fn} from './vue';var a = function() { return function fn() { console.log(fn) } }`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["fn"]}); - var a = function() { return function fn() { console.log(fn) } }" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["fn"]});var a = function() { return function fn() { console.log(fn) } }"`) }) // #16452 @@ -313,20 +265,14 @@ test('do not rewrite when function expression is in global scope', async () => { const result = await ssrTransformSimple( `import {fn} from './vue';foo(function fn(a = fn) { console.log(fn) })`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["fn"]}); - foo(function fn(a = fn) { console.log(fn) })" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["fn"]});foo(function fn(a = fn) { console.log(fn) })"`) }) test('do not rewrite when class declaration is in scope', async () => { const result = await ssrTransformSimple( `import { cls } from 'vue';function A(){ class cls {} return { cls }; }`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["cls"]}); - function A(){ class cls {} return { cls }; }" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["cls"]});function A(){ class cls {} return { cls }; }"`) expect(result?.deps).toEqual(['vue']) }) @@ -334,30 +280,21 @@ test('do not rewrite when class expression is in scope', async () => { const result = await ssrTransformSimple( `import { cls } from './vue';var a = function() { return class cls { constructor() { console.log(cls) } } }`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["cls"]}); - var a = function() { return class cls { constructor() { console.log(cls) } } }" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["cls"]});var a = function() { return class cls { constructor() { console.log(cls) } } }"`) }) test('do not rewrite when class expression is in global scope', async () => { const result = await ssrTransformSimple( `import { cls } from './vue';foo(class cls { constructor() { console.log(cls) } })`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["cls"]}); - foo(class cls { constructor() { console.log(cls) } })" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["cls"]});foo(class cls { constructor() { console.log(cls) } })"`) }) test('do not rewrite catch clause', async () => { const result = await ssrTransformSimple( `import {error} from './dependency';try {} catch(error) {}`, ) - expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./dependency", {"importedNames":["error"]}); - try {} catch(error) {}" - `) + expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("./dependency", {"importedNames":["error"]});try {} catch(error) {}"`) expect(result?.deps).toEqual(['./dependency']) }) @@ -368,8 +305,7 @@ test('should declare variable for imported super class', async () => { `import { Foo } from './dependency';` + `class A extends Foo {}`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./dependency", {"importedNames":["Foo"]}); - const Foo = __vite_ssr_import_0__.Foo; + "const __vite_ssr_import_0__ = await __vite_ssr_import__("./dependency", {"importedNames":["Foo"]});const Foo = __vite_ssr_import_0__.Foo; class A extends Foo {}" `) @@ -382,8 +318,7 @@ test('should declare variable for imported super class', async () => { `export class B extends Foo {}`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./dependency", {"importedNames":["Foo"]}); - const Foo = __vite_ssr_import_0__.Foo; + "const __vite_ssr_import_0__ = await __vite_ssr_import__("./dependency", {"importedNames":["Foo"]});const Foo = __vite_ssr_import_0__.Foo; class A extends Foo {}; class B extends Foo {} Object.defineProperty(__vite_ssr_exports__, "B", { enumerable: true, configurable: true, get(){ return B }}); @@ -448,9 +383,7 @@ test('sourcemap is correct for hoisted imports', async () => { const result = (await ssrTransform(code, null, 'input.js', code))! expect(result.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["foo"]}); - const __vite_ssr_import_1__ = await __vite_ssr_import__("vue2", {"importedNames":["bar"]}); - + "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["foo"]});const __vite_ssr_import_1__ = await __vite_ssr_import__("vue2", {"importedNames":["bar"]}); console.log((0,__vite_ssr_import_0__.foo), (0,__vite_ssr_import_1__.bar)); @@ -465,7 +398,7 @@ test('sourcemap is correct for hoisted imports', async () => { column: 0, name: null, }) - expect(originalPositionFor(traceMap, { line: 2, column: 0 })).toStrictEqual({ + expect(originalPositionFor(traceMap, { line: 1, column: 90 })).toStrictEqual({ source: 'input.js', line: 6, column: 0, @@ -532,8 +465,7 @@ test('overwrite bindings', async () => { `function g() { const f = () => { const inject = true }; console.log(inject) }\n`, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["inject"]}); - const a = { inject: __vite_ssr_import_0__.inject }; + "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["inject"]});const a = { inject: __vite_ssr_import_0__.inject }; const b = { test: __vite_ssr_import_0__.inject }; function c() { const { test: inject } = { test: true }; console.log(inject) } const d = __vite_ssr_import_0__.inject; @@ -561,9 +493,8 @@ function c({ _ = bar() + foo() }) {} `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["foo","bar"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["foo","bar"]}); const a = ({ _ = (0,__vite_ssr_import_0__.foo)() }) => {}; function b({ _ = (0,__vite_ssr_import_0__.bar)() }) {} function c({ _ = (0,__vite_ssr_import_0__.bar)() + (0,__vite_ssr_import_0__.foo)() }) {} @@ -583,9 +514,8 @@ const a = () => { `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["n"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["n"]}); const a = () => { const { type: n = 'bar' } = {}; console.log(n) @@ -606,9 +536,8 @@ const foo = {} `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["n","m"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["n","m"]}); const foo = {}; { @@ -649,9 +578,8 @@ objRest() `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["remove","add","get","set","rest","objRest"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["remove","add","get","set","rest","objRest"]}); function a() { const { @@ -699,9 +627,8 @@ const obj = { `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["default"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["default"]}); const bar = 'bar'; @@ -731,9 +658,8 @@ class A { `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["remove","add"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["remove","add"]}); const add = __vite_ssr_import_0__.add; const remove = __vite_ssr_import_0__.remove; @@ -763,9 +689,8 @@ class A { `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["default"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["default"]}); const bar = 'bar'; @@ -809,9 +734,8 @@ bbb() `, ), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["aaa","bbb","ccc","ddd"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["aaa","bbb","ccc","ddd"]}); function foobar() { ddd(); @@ -856,8 +780,6 @@ test('jsx', async () => { .toMatchInlineSnapshot(` "const __vite_ssr_import_0__ = await __vite_ssr_import__("react", {"importedNames":["default"]}); const __vite_ssr_import_1__ = await __vite_ssr_import__("foo", {"importedNames":["Foo","Slot"]}); - - function Bar({ Slot: Slot2 = /* @__PURE__ */ __vite_ssr_import_0__.default.createElement((0,__vite_ssr_import_1__.Foo), null) }) { return /* @__PURE__ */ __vite_ssr_import_0__.default.createElement(__vite_ssr_import_0__.default.Fragment, null, /* @__PURE__ */ __vite_ssr_import_0__.default.createElement(Slot2, null)); } @@ -930,8 +852,7 @@ import foo from "foo"`, ), ).toMatchInlineSnapshot(` "#!/usr/bin/env node - const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["default"]}); - console.log((0,__vite_ssr_import_0__.default)); + const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["default"]});console.log((0,__vite_ssr_import_0__.default)); " `) }) @@ -946,7 +867,6 @@ foo()`, ).toMatchInlineSnapshot(` "#!/usr/bin/env node const __vite_ssr_import_0__ = await __vite_ssr_import__("foo", {"importedNames":["foo"]}); - (0,__vite_ssr_import_0__.foo)()" `) }) @@ -982,7 +902,6 @@ export class Test { expect(await ssrTransformSimpleCode(code)).toMatchInlineSnapshot(` "const __vite_ssr_import_0__ = await __vite_ssr_import__("foobar", {"importedNames":["foo","bar"]}); - if (false) { const foo = 'foo'; console.log(foo) @@ -1023,9 +942,8 @@ function test() { return [foo, bar] }`), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("foobar", {"importedNames":["foo","bar"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("foobar", {"importedNames":["foo","bar"]}); function test() { if (true) { var foo = () => { var why = 'would' }, bar = 'someone' @@ -1050,9 +968,8 @@ function test() { return bar; }`), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("foobar", {"importedNames":["foo","bar","baz"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("foobar", {"importedNames":["foo","bar","baz"]}); function test() { [__vite_ssr_import_0__.foo]; { @@ -1082,9 +999,8 @@ for (const test in tests) { console.log(test) }`), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./test.js", {"importedNames":["test"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("./test.js", {"importedNames":["test"]}); for (const test of tests) { console.log(test) @@ -1114,9 +1030,8 @@ const Baz = class extends Foo {} `, ) expect(result?.code).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./foo", {"importedNames":["default","Bar"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("./foo", {"importedNames":["default","Bar"]}); console.log((0,__vite_ssr_import_0__.default), (0,__vite_ssr_import_0__.Bar)); const obj = { @@ -1135,9 +1050,8 @@ test('import assertion attribute', async () => { import('./bar.json', { with: { type: 'json' } }); `), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./foo.json"); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("./foo.json"); __vite_ssr_dynamic_import__('./bar.json', { with: { type: 'json' } }); " `) @@ -1157,14 +1071,11 @@ console.log(foo + 2) `), ).toMatchInlineSnapshot(` "const __vite_ssr_import_0__ = await __vite_ssr_import__("./foo", {"importedNames":["foo"]}); - console.log(__vite_ssr_import_0__.foo + 1); - const __vite_ssr_import_1__ = await __vite_ssr_import__("./a"); - __vite_ssr_exportAll__(__vite_ssr_import_1__); + const __vite_ssr_import_1__ = await __vite_ssr_import__("./a");__vite_ssr_exportAll__(__vite_ssr_import_1__); ; - const __vite_ssr_import_2__ = await __vite_ssr_import__("./b"); - __vite_ssr_exportAll__(__vite_ssr_import_2__); + const __vite_ssr_import_2__ = await __vite_ssr_import__("./b");__vite_ssr_exportAll__(__vite_ssr_import_2__); ; console.log(__vite_ssr_import_0__.foo + 2) " @@ -1180,12 +1091,10 @@ export * as bar from './bar' console.log(bar) `), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./foo", {"importedNames":["foo"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("./foo", {"importedNames":["foo"]}); __vite_ssr_exports__.default = (0,__vite_ssr_import_0__.foo)(); const __vite_ssr_import_1__ = await __vite_ssr_import__("./bar"); - Object.defineProperty(__vite_ssr_exports__, "bar", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_1__ }});; console.log(bar) " @@ -1256,9 +1165,8 @@ switch (1) { } `), ).toMatchInlineSnapshot(` - "const __vite_ssr_import_0__ = await __vite_ssr_import__("./f", {"importedNames":["f"]}); - - + " + const __vite_ssr_import_0__ = await __vite_ssr_import__("./f", {"importedNames":["f"]}); let x = 0; From 37586e1144e1a3b4cf3b8aceb3059e8469a68e43 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Wed, 18 Dec 2024 13:29:55 -0500 Subject: [PATCH 3/9] chore: format --- .../node/ssr/__tests__/ssrTransform.spec.ts | 72 ++++++++++++++----- 1 file changed, 54 insertions(+), 18 deletions(-) diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index 4b79eca970497d..6b5ad56f907da6 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts @@ -14,7 +14,9 @@ const ssrTransformSimpleCode = async (code: string, url?: string) => test('default import', async () => { expect( await ssrTransformSimpleCode(`import foo from 'vue';console.log(foo.bar)`), - ).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["default"]});console.log(__vite_ssr_import_0__.default.bar)"`) + ).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["default"]});console.log(__vite_ssr_import_0__.default.bar)"`, + ) }) test('named import', async () => { @@ -22,7 +24,9 @@ test('named import', async () => { await ssrTransformSimpleCode( `import { ref } from 'vue';function foo() { return ref(0) }`, ), - ).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["ref"]});function foo() { return (0,__vite_ssr_import_0__.ref)(0) }"`) + ).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["ref"]});function foo() { return (0,__vite_ssr_import_0__.ref)(0) }"`, + ) }) test('named import: arbitrary module namespace specifier', async () => { @@ -30,7 +34,9 @@ test('named import: arbitrary module namespace specifier', async () => { await ssrTransformSimpleCode( `import { "some thing" as ref } from 'vue';function foo() { return ref(0) }`, ), - ).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["some thing"]});function foo() { return (0,__vite_ssr_import_0__["some thing"])(0) }"`) + ).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["some thing"]});function foo() { return (0,__vite_ssr_import_0__["some thing"])(0) }"`, + ) }) test('namespace import', async () => { @@ -38,7 +44,9 @@ test('namespace import', async () => { await ssrTransformSimpleCode( `import * as vue from 'vue';function foo() { return vue.ref(0) }`, ), - ).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue");function foo() { return __vite_ssr_import_0__.ref(0) }"`) + ).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue");function foo() { return __vite_ssr_import_0__.ref(0) }"`, + ) }) test('export function declaration', async () => { @@ -171,7 +179,9 @@ test('hoist import to top', async () => { await ssrTransformSimpleCode( `path.resolve('server.js');import path from 'node:path';`, ), - ).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("node:path", {"importedNames":["default"]});__vite_ssr_import_0__.default.resolve('server.js');"`) + ).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("node:path", {"importedNames":["default"]});__vite_ssr_import_0__.default.resolve('server.js');"`, + ) }) test('import.meta', async () => { @@ -196,7 +206,9 @@ test('do not rewrite method definition', async () => { const result = await ssrTransformSimple( `import { fn } from 'vue';class A { fn() { fn() } }`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});class A { fn() { (0,__vite_ssr_import_0__.fn)() } }"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});class A { fn() { (0,__vite_ssr_import_0__.fn)() } }"`, + ) expect(result?.deps).toEqual(['vue']) }) @@ -204,7 +216,9 @@ test('do not rewrite when variable is in scope', async () => { const result = await ssrTransformSimple( `import { fn } from 'vue';function A(){ const fn = () => {}; return { fn }; }`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ const fn = () => {}; return { fn }; }"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ const fn = () => {}; return { fn }; }"`, + ) expect(result?.deps).toEqual(['vue']) }) @@ -213,7 +227,9 @@ test('do not rewrite when variable is in scope with object destructuring', async const result = await ssrTransformSimple( `import { fn } from 'vue';function A(){ let {fn, test} = {fn: 'foo', test: 'bar'}; return { fn }; }`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ let {fn, test} = {fn: 'foo', test: 'bar'}; return { fn }; }"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ let {fn, test} = {fn: 'foo', test: 'bar'}; return { fn }; }"`, + ) expect(result?.deps).toEqual(['vue']) }) @@ -222,7 +238,9 @@ test('do not rewrite when variable is in scope with array destructuring', async const result = await ssrTransformSimple( `import { fn } from 'vue';function A(){ let [fn, test] = ['foo', 'bar']; return { fn }; }`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ let [fn, test] = ['foo', 'bar']; return { fn }; }"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ let [fn, test] = ['foo', 'bar']; return { fn }; }"`, + ) expect(result?.deps).toEqual(['vue']) }) @@ -231,7 +249,9 @@ test('rewrite variable in string interpolation in function nested arguments', as const result = await ssrTransformSimple( `import { fn } from 'vue';function A({foo = \`test\${fn}\`} = {}){ return {}; }`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A({foo = \`test\${__vite_ssr_import_0__.fn}\`} = {}){ return {}; }"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A({foo = \`test\${__vite_ssr_import_0__.fn}\`} = {}){ return {}; }"`, + ) expect(result?.deps).toEqual(['vue']) }) @@ -240,7 +260,9 @@ test('rewrite variables in default value of destructuring params', async () => { const result = await ssrTransformSimple( `import { fn } from 'vue';function A({foo = fn}){ return {}; }`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A({foo = __vite_ssr_import_0__.fn}){ return {}; }"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A({foo = __vite_ssr_import_0__.fn}){ return {}; }"`, + ) expect(result?.deps).toEqual(['vue']) }) @@ -248,7 +270,9 @@ test('do not rewrite when function declaration is in scope', async () => { const result = await ssrTransformSimple( `import { fn } from 'vue';function A(){ function fn() {}; return { fn }; }`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ function fn() {}; return { fn }; }"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["fn"]});function A(){ function fn() {}; return { fn }; }"`, + ) expect(result?.deps).toEqual(['vue']) }) @@ -257,7 +281,9 @@ test('do not rewrite when function expression is in scope', async () => { const result = await ssrTransformSimple( `import {fn} from './vue';var a = function() { return function fn() { console.log(fn) } }`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["fn"]});var a = function() { return function fn() { console.log(fn) } }"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["fn"]});var a = function() { return function fn() { console.log(fn) } }"`, + ) }) // #16452 @@ -265,14 +291,18 @@ test('do not rewrite when function expression is in global scope', async () => { const result = await ssrTransformSimple( `import {fn} from './vue';foo(function fn(a = fn) { console.log(fn) })`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["fn"]});foo(function fn(a = fn) { console.log(fn) })"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["fn"]});foo(function fn(a = fn) { console.log(fn) })"`, + ) }) test('do not rewrite when class declaration is in scope', async () => { const result = await ssrTransformSimple( `import { cls } from 'vue';function A(){ class cls {} return { cls }; }`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["cls"]});function A(){ class cls {} return { cls }; }"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["cls"]});function A(){ class cls {} return { cls }; }"`, + ) expect(result?.deps).toEqual(['vue']) }) @@ -280,21 +310,27 @@ test('do not rewrite when class expression is in scope', async () => { const result = await ssrTransformSimple( `import { cls } from './vue';var a = function() { return class cls { constructor() { console.log(cls) } } }`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["cls"]});var a = function() { return class cls { constructor() { console.log(cls) } } }"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["cls"]});var a = function() { return class cls { constructor() { console.log(cls) } } }"`, + ) }) test('do not rewrite when class expression is in global scope', async () => { const result = await ssrTransformSimple( `import { cls } from './vue';foo(class cls { constructor() { console.log(cls) } })`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["cls"]});foo(class cls { constructor() { console.log(cls) } })"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("./vue", {"importedNames":["cls"]});foo(class cls { constructor() { console.log(cls) } })"`, + ) }) test('do not rewrite catch clause', async () => { const result = await ssrTransformSimple( `import {error} from './dependency';try {} catch(error) {}`, ) - expect(result?.code).toMatchInlineSnapshot(`"const __vite_ssr_import_0__ = await __vite_ssr_import__("./dependency", {"importedNames":["error"]});try {} catch(error) {}"`) + expect(result?.code).toMatchInlineSnapshot( + `"const __vite_ssr_import_0__ = await __vite_ssr_import__("./dependency", {"importedNames":["error"]});try {} catch(error) {}"`, + ) expect(result?.deps).toEqual(['./dependency']) }) From 811683fd6d0b04605d0fb6697831bd3d6505935e Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Thu, 19 Dec 2024 10:27:02 -0500 Subject: [PATCH 4/9] test: import hoisting and line offset preservation --- .../node/ssr/__tests__/ssrTransform.spec.ts | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index 6b5ad56f907da6..f8acaa32634784 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts @@ -184,6 +184,78 @@ test('hoist import to top', async () => { ) }) +test('preserve line offset when rewriting imports', async () => { + // The line number of each non-import statement must not change. + const inputLines = [ + `debugger;`, + ``, + `import {`, + ` dirname,`, + ` join,`, + `} from 'node:path';`, + ``, + `debugger;`, + ``, + `import fs from 'node:fs';`, + ``, + `debugger;`, + ``, + `import {`, + ` red,`, + ` green,`, + `} from 'kleur/colors';`, + ``, + `debugger;`, + ] + + const output = await ssrTransformSimpleCode(inputLines.join('\n')) + expect(output).toBeDefined() + + const outputLines = output!.split('\n') + expect( + outputLines + .map((line, i) => `${String(i + 1).padStart(2)} | ${line}`.trimEnd()) + .join('\n'), + ).toMatchInlineSnapshot(` + " 1 | const __vite_ssr_import_0__ = await __vite_ssr_import__("node:path", {"importedNames":["dirname","join"]});const __vite_ssr_import_1__ = await __vite_ssr_import__("node:fs", {"importedNames":["default"]});const __vite_ssr_import_2__ = await __vite_ssr_import__("kleur/colors", {"importedNames":["red","green"]});debugger; + 2 | + 3 | + 4 | + 5 | + 6 | + 7 | + 8 | debugger; + 9 | + 10 | + 11 | + 12 | debugger; + 13 | + 14 | + 15 | + 16 | + 17 | + 18 | + 19 | debugger;" + `) + + // Ensure the debugger statements are still on the same lines. + expect(outputLines[0].endsWith(inputLines[0])).toBe(true) + expect(outputLines[7]).toBe(inputLines[7]) + expect(outputLines[11]).toBe(inputLines[11]) + expect(outputLines[18]).toBe(inputLines[18]) +}) + +test.fails('comments between imports do not trigger hoisting', async () => { + expect( + await ssrTransformSimpleCode( + `import { dirname } from 'node:path';// comment\nimport fs from 'node:fs';`, + ), + ).toMatchInlineSnapshot(` + "const __vite_ssr_import_0__ = await __vite_ssr_import__("node:path", {"importedNames":["dirname"]});// comment + const __vite_ssr_import_1__ = await __vite_ssr_import__("node:fs", {"importedNames":["default"]});" + `) +}) + test('import.meta', async () => { expect( await ssrTransformSimpleCode(`console.log(import.meta.url)`), From 2281311221afeaa052c84fab4cf88b412bbb6eed Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Thu, 19 Dec 2024 10:29:21 -0500 Subject: [PATCH 5/9] get the test to pass --- packages/vite/src/node/ssr/ssrTransform.ts | 61 ++++++++++++---------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrTransform.ts b/packages/vite/src/node/ssr/ssrTransform.ts index 077e2c2026ed33..0708cc78b7ff43 100644 --- a/packages/vite/src/node/ssr/ssrTransform.ts +++ b/packages/vite/src/node/ssr/ssrTransform.ts @@ -126,51 +126,54 @@ async function ssrTransformScript( ) { const source = importNode.source.value as string deps.add(source) - const importId = `__vite_ssr_import_${uid++}__` // Reduce metadata to undefined if it's all default values - if ( - metadata && - (metadata.importedNames == null || metadata.importedNames.length === 0) - ) { - metadata = undefined - } - const metadataStr = metadata ? `, ${JSON.stringify(metadata)}` : '' + const metadataArg = + (metadata?.importedNames?.length ?? 0) > 0 + ? `, ${JSON.stringify(metadata)}` + : '' - // Track how many lines the original import statement spans, so we can preserve the line offset. - let linesSpanned = 1 - for (let i = importNode.start; i < importNode.end; i++) { - if (code[i] === '\n') { - linesSpanned++ - } - } + const importId = `__vite_ssr_import_${uid++}__` + const transformedImport = `const ${importId} = await ${ssrImportKey}(${JSON.stringify( + source, + )}${metadataArg});` - s.update( - importNode.start, - importNode.end, - `const ${importId} = await ${ssrImportKey}(${JSON.stringify( - source, - )}${metadataStr});${'\n'.repeat(linesSpanned - 1)}`, - ) + s.update(importNode.start, importNode.end, transformedImport) // Check for non-whitespace characters between the last import and the - // current one. + // current one, to determine if hoisting is needed. + // TODO: Account for comments between imports. const nonWhitespaceRegex = /\S/g nonWhitespaceRegex.lastIndex = index nonWhitespaceRegex.exec(code) - - // TODO: Account for comments between imports. if (importNode.start > nonWhitespaceRegex.lastIndex) { - // By moving the import to the top of the module, we ensure that it's - // imported before it's used. + // Imports are moved to the top of the file (AKA “hoisting”) to ensure any + // non-import statements before them are executed after the import. This + // aligns SSR imports with native ESM import behavior. s.move(importNode.start, importNode.end, index) } else { - // Only update hoistIndex when not hoisting the current import. This + // Only update hoistIndex when *not* hoisting the current import. This // ensures that once any import in this module has been hoisted, all - // remaining imports will also be hoisted. + // remaining imports will also be hoisted. This is inherently true because + // we work from the top of the file downward. hoistIndex = importNode.end } + // Track how many lines the original import statement spans, so we can + // preserve the line offset. + let linesSpanned = 1 + for (let i = importNode.start; i < importNode.end; i++) { + if (code[i] === '\n') { + linesSpanned++ + } + } + if (linesSpanned > 1) { + // This leaves behind any extra newlines that were removed during + // transformation, in the position of the original import statement + // (before any hoisting). + s.prependRight(importNode.end, '\n'.repeat(linesSpanned - 1)) + } + return importId } From dbd8e544c087dd8995d50526d928ce38580f3a2e Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Thu, 19 Dec 2024 10:33:31 -0500 Subject: [PATCH 6/9] one more test --- .../src/node/ssr/__tests__/ssrTransform.spec.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index f8acaa32634784..15cd2f37383207 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts @@ -184,6 +184,19 @@ test('hoist import to top', async () => { ) }) +test('do not hoist import if only whitespace is between them', async () => { + expect( + await ssrTransformSimpleCode( + `import { dirname } from 'node:path';\n\n\nimport fs from 'node:fs';`, + ), + ).toMatchInlineSnapshot(` + "const __vite_ssr_import_0__ = await __vite_ssr_import__("node:path", {"importedNames":["dirname"]}); + + + const __vite_ssr_import_1__ = await __vite_ssr_import__("node:fs", {"importedNames":["default"]});" + `) +}) + test('preserve line offset when rewriting imports', async () => { // The line number of each non-import statement must not change. const inputLines = [ From 0317abdbf83f837831d5d3af81ed0dd47430c3e1 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Thu, 19 Dec 2024 10:35:53 -0500 Subject: [PATCH 7/9] tweak test name --- packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index 15cd2f37383207..6a478d53fcf7ed 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts @@ -184,7 +184,7 @@ test('hoist import to top', async () => { ) }) -test('do not hoist import if only whitespace is between them', async () => { +test('whitespace between imports does not trigger hoisting', async () => { expect( await ssrTransformSimpleCode( `import { dirname } from 'node:path';\n\n\nimport fs from 'node:fs';`, From defd7f41c56945ed56ce6e6f45a8ec3c8f335df1 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Fri, 20 Dec 2024 09:52:25 -0500 Subject: [PATCH 8/9] Update packages/vite/src/node/ssr/ssrTransform.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 翠 / green --- packages/vite/src/node/ssr/ssrTransform.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrTransform.ts b/packages/vite/src/node/ssr/ssrTransform.ts index 0708cc78b7ff43..1294ece439f1c4 100644 --- a/packages/vite/src/node/ssr/ssrTransform.ts +++ b/packages/vite/src/node/ssr/ssrTransform.ts @@ -140,9 +140,10 @@ async function ssrTransformScript( s.update(importNode.start, importNode.end, transformedImport) - // Check for non-whitespace characters between the last import and the - // current one, to determine if hoisting is needed. - // TODO: Account for comments between imports. + // If there's only whitespace characters between the last import and the + // current one, that means there's no statements between them and + // hoisting is not needed. + // FIXME: account for comments between imports const nonWhitespaceRegex = /\S/g nonWhitespaceRegex.lastIndex = index nonWhitespaceRegex.exec(code) From 684893d9f6fe0f714663e0cf559acabcba9422e2 Mon Sep 17 00:00:00 2001 From: Alec Larson <1925840+aleclarson@users.noreply.github.com> Date: Fri, 20 Dec 2024 10:15:05 -0500 Subject: [PATCH 9/9] Update packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 翠 / green --- packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index 6a478d53fcf7ed..d7db971e56218c 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts @@ -258,7 +258,8 @@ test('preserve line offset when rewriting imports', async () => { expect(outputLines[18]).toBe(inputLines[18]) }) -test.fails('comments between imports do not trigger hoisting', async () => { +// not implemented +test.skip('comments between imports do not trigger hoisting', async () => { expect( await ssrTransformSimpleCode( `import { dirname } from 'node:path';// comment\nimport fs from 'node:fs';`,