Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(ssr): Transform named default exports without altering scope #4053

Merged
merged 2 commits into from Jul 2, 2021
Merged

fix(ssr): Transform named default exports without altering scope #4053

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17014d0
fix(ssr): Transform named default exports without altering scope
GrygrFlzr Jun 30, 2021
5f72011
chore: update
antfu Jul 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 49 additions & 1 deletion packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts
View file
Open in desktop
Expand Up @@ -211,8 +211,56 @@ test('should declare variable for imported super class', async () => {
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"./dependency\\")
const Foo = __vite_ssr_import_0__.Foo;
__vite_ssr_exports__.default = class A extends Foo {}
class A extends Foo {}
class B extends Foo {}
Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: A })
Object.defineProperty(__vite_ssr_exports__, \\"B\\", { enumerable: true, configurable: true, get(){ return B }})"
`)
})

// #4049
test('should handle default export variants', async () => {
// default anonymous functions
expect(
(await ssrTransform(`export default function() {}\n`, null, null)).code
).toMatchInlineSnapshot(`
"__vite_ssr_exports__.default = function() {}
"
`)
// default anonymous class
expect((await ssrTransform(`export default class {}\n`, null, null)).code)
.toMatchInlineSnapshot(`
"__vite_ssr_exports__.default = class {}
"
`)
Comment on lines +231 to +235
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why, but stripping out the newline from the source + target caused toMatchInlineSnapshot to insist that the lines were not equivalent. There might be a bug with its handling of single-line code?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could apply .trim() to code before toMatchInlineSnapshot

// default named functions
expect(
(
await ssrTransform(
`export default function foo() {}\n` +
`foo.prototype = Object.prototype;`,
null,
null
)
).code
).toMatchInlineSnapshot(`
"function foo() {}
foo.prototype = Object.prototype;
Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: foo })"
`)
// default named classes
expect(
(
await ssrTransform(
`export default class A {}\n` + `export class B extends A {}`,
null,
null
)
).code
).toMatchInlineSnapshot(`
"class A {}
class B extends A {}
Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: A })
Object.defineProperty(__vite_ssr_exports__, \\"B\\", { enumerable: true, configurable: true, get(){ return B }})"
`)
})
Expand Down
23 changes: 18 additions & 5 deletions packages/vite/src/node/ssr/ssrTransform.ts
View file
Open in desktop
Expand Up @@ -124,11 +124,24 @@ export async function ssrTransform(

// default export
if (node.type === 'ExportDefaultDeclaration') {
s.overwrite(
node.start,
node.start + 14,
`${ssrModuleExportsKey}.default =`
)
if ('id' in node.declaration && node.declaration.id) {
// named hoistable/class exports
// export default function foo() {}
// export default class A {}
const { name } = node.declaration.id
s.remove(node.start, node.start + `export default `.length)
s.append(
`\nObject.defineProperty(${ssrModuleExportsKey}, "default", ` +
`{ enumerable: true, value: ${name} })`
)
} else {
// anonymous default exports
s.overwrite(
node.start,
node.start + `export default`.length,
Copy link
Member Author

@GrygrFlzr GrygrFlzr Jun 30, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The + 14 in the original code seemed like a horrible magic number, which is why I replaced it with a string length operation instead. If this is a cause of concern for performance, we can convert it back, I guess?

`${ssrModuleExportsKey}.default =`
)
}
}

// export * from './foo'
Expand Down