Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

fix(ssr): fix semicolon injection by ssr transform #19097

Merged
merged 5 commits into from
Jan 2, 2025
Merged

fix(ssr): fix semicolon injection by ssr transform #19097

merged 5 commits into from
Jan 2, 2025

Conversation

TheAlexLichter
Copy link
Contributor

@TheAlexLichter TheAlexLichter commented Dec 29, 2024 •
edited by hi-ogawa
Loading

Description

This PR is a base for a fix for #19096 containing a failing edge case (not that edgy actually!)
Feel free to use it as base for fixing the problem.
I've tried in several ways but wasn't able to find a way in the time I had (+ keeping the ; + binding strategy)

(cc @hi-ogawa)

@TheAlexLichter TheAlexLichter marked this pull request as draft December 29, 2024 18:15
@hi-ogawa
Copy link
Collaborator

/ecosystem-ci run

@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Dec 30, 2024 •
edited
Loading

Open in Stackblitz

npm i https://pkg.pr.new/vite@19097

commit: c308bb5

@vite-ecosystem-ci
Copy link

📝 Ran ecosystem CI on c308bb5: Open

suite result latest scheduled
histoire ❌ failure ❌ failure
ladle ❌ failure ❌ failure
nuxt ✅ success ❌ failure
redwoodjs ❌ failure ❌ failure
remix ❌ failure ❌ failure
vite-plugin-svelte ❌ failure ❌ failure
vitest ❌ failure ❌ failure

✅ analogjs, astro, laravel, marko, previewjs, quasar, qwik, rakkas, storybook, sveltekit, unocss, vike, vite-environment-examples, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-vue, vite-setup-catalogue, vitepress, vuepress, waku

hi-ogawa
hi-ogawa reviewed Dec 30, 2024
View reviewed changes
Copy link
Collaborator

@hi-ogawa hi-ogawa left a comment

Choose a reason for hiding this comment

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

@TheAlexLichter Thanks for investigating the issue 👍 I pushed a potential fix here.

@hi-ogawa hi-ogawa changed the title test: provide a failing test for ssrTransform error fix(ssr): fix semicolon injection by ssr transform Dec 30, 2024
@hi-ogawa hi-ogawa marked this pull request as ready for review December 30, 2024 07:33
@hi-ogawa hi-ogawa requested a review from sapphi-red December 30, 2024 08:07
@sapphi-red sapphi-red added p4-important Violate documented behavior or significantly improves performance (priority) feat: ssr labels Dec 30, 2024
sapphi-red
sapphi-red approved these changes Dec 30, 2024
View reviewed changes
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

Thanks!

@patak-dev patak-dev merged commit 1c102d5 into vitejs:main Jan 2, 2025
21 checks passed
@TheAlexLichter TheAlexLichter deleted the fix/syntax-error-ssr branch January 3, 2025 09:00
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Jan 3, 2025
@renovate
Update dependency vite to v6.0.7
0f9d57d 
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | vite    | 6.0.6 | 6.0.7 |


## [v6.0.7](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small607-2025-01-02-small)

-   fix: fix `minify` when `builder.sharedPlugins: true` ([#19025](vitejs/vite#19025)) ([f7b1964](vitejs/vite@f7b1964)), closes [#19025](vitejs/vite#19025)
-   fix: skip the plugin if it has been called before with the same id and importer ([#19016](vitejs/vite#19016)) ([b178c90](vitejs/vite@b178c90)), closes [#19016](vitejs/vite#19016)
-   fix(html): error while removing `vite-ignore` attribute for inline script ([#19062](vitejs/vite#19062)) ([a492253](vitejs/vite@a492253)), closes [#19062](vitejs/vite#19062)
-   fix(ssr): fix semicolon injection by ssr transform ([#19097](vitejs/vite#19097)) ([1c102d5](vitejs/vite@1c102d5)), closes [#19097](vitejs/vite#19097)
-   perf: skip globbing for static path in warmup ([#19107](vitejs/vite#19107)) ([677508b](vitejs/vite@677508b)), closes [#19107](vitejs/vite#19107)
-   feat(css): show lightningcss warnings ([#19076](vitejs/vite#19076)) ([b07c036](vitejs/vite@b07c036)), closes [#19076](vitejs/vite#19076)
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Jan 4, 2025
@renovate
Update dependency vite to v6.0.7
a8b1a63 
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | vite    | 6.0.6 | 6.0.7 |


## [v6.0.7](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small607-2025-01-02-small)

-   fix: fix `minify` when `builder.sharedPlugins: true` ([#19025](vitejs/vite#19025)) ([f7b1964](vitejs/vite@f7b1964)), closes [#19025](vitejs/vite#19025)
-   fix: skip the plugin if it has been called before with the same id and importer ([#19016](vitejs/vite#19016)) ([b178c90](vitejs/vite@b178c90)), closes [#19016](vitejs/vite#19016)
-   fix(html): error while removing `vite-ignore` attribute for inline script ([#19062](vitejs/vite#19062)) ([a492253](vitejs/vite@a492253)), closes [#19062](vitejs/vite#19062)
-   fix(ssr): fix semicolon injection by ssr transform ([#19097](vitejs/vite#19097)) ([1c102d5](vitejs/vite@1c102d5)), closes [#19097](vitejs/vite#19097)
-   perf: skip globbing for static path in warmup ([#19107](vitejs/vite#19107)) ([677508b](vitejs/vite@677508b)), closes [#19107](vitejs/vite#19107)
-   feat(css): show lightningcss warnings ([#19076](vitejs/vite#19076)) ([b07c036](vitejs/vite@b07c036)), closes [#19076](vitejs/vite#19076)
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Jan 5, 2025
@renovate
Update dependency vite to v6.0.7
5eb0461 
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | vite    | 6.0.6 | 6.0.7 |


## [v6.0.7](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small607-2025-01-02-small)

-   fix: fix `minify` when `builder.sharedPlugins: true` ([#19025](vitejs/vite#19025)) ([f7b1964](vitejs/vite@f7b1964)), closes [#19025](vitejs/vite#19025)
-   fix: skip the plugin if it has been called before with the same id and importer ([#19016](vitejs/vite#19016)) ([b178c90](vitejs/vite@b178c90)), closes [#19016](vitejs/vite#19016)
-   fix(html): error while removing `vite-ignore` attribute for inline script ([#19062](vitejs/vite#19062)) ([a492253](vitejs/vite@a492253)), closes [#19062](vitejs/vite#19062)
-   fix(ssr): fix semicolon injection by ssr transform ([#19097](vitejs/vite#19097)) ([1c102d5](vitejs/vite@1c102d5)), closes [#19097](vitejs/vite#19097)
-   perf: skip globbing for static path in warmup ([#19107](vitejs/vite#19107)) ([677508b](vitejs/vite@677508b)), closes [#19107](vitejs/vite#19107)
-   feat(css): show lightningcss warnings ([#19076](vitejs/vite#19076)) ([b07c036](vitejs/vite@b07c036)), closes [#19076](vitejs/vite#19076)
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@hi-ogawa hi-ogawa hi-ogawa left review comments

@patak-dev patak-dev patak-dev approved these changes

@vinayakkulkarni vinayakkulkarni vinayakkulkarni approved these changes

@sapphi-red sapphi-red sapphi-red approved these changes

Assignees
No one assigned
Labels
feat: ssr p4-important Violate documented behavior or significantly improves performance (priority) trigger: preview
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ssrTransform can cause syntax error
5 participants
@TheAlexLichter @hi-ogawa @patak-dev @vinayakkulkarni @sapphi-red