feat!: update esbuild to 0.18.2

[sapphi-red]

Description

This PR updates esbuild to ^0.18.2 from ^0.17.5.
https://github.com/evanw/esbuild/blob/main/CHANGELOG.md

The breaking changes included is:

• Using experimental decorators now requires "experimentalDecorators": true

  • We can add "experimentalDecorators": true on our side to remove this breaking change but I didn't.
    • We will need a breaking change for supporting ES decorators anyway: Feature request: Decorators support evanw/esbuild#104 (comment)
  • 584c9ee

• TypeScript's target no longer affects esbuild's target

  • This is not a breaking change for us. We always set esbuild's target so target in tsconfig was never used.
  • removed the related code: c15c155 (chore(esbuild): add test for configuration overrides #11267's test covers this behavior)

• TypeScript's jsx setting no longer causes esbuild to preserve JSX syntax

  • This is not a breaking change for us. We always use the esbuild's option is both of the options are set. (related: feat(node/plugins): esbuild options #11049)

• useDefineForClassFields behavior has changed

  • This is not a breaking change for us. It works like that already. (related: fix: derive useDefineForClassFields value from tsconfig.compilerOptions.target (fixes #10296) #11301)
  • removed the related code: f4a27dd (fix: derive useDefineForClassFields value from tsconfig.compilerOptions.target (fixes #10296) #11301's test covers this behavior)

• Add support for verbatimModuleSyntax from TypeScript 5.0

  • 50fa325

If a user is using dep optimization with TypeScript files, almost all the breaking changes are related.
https://github.com/evanw/esbuild/blob/main/CHANGELOG.md#0180

fixes #13500
fixes #13391

Additional context

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the PR Title Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[patak-dev]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI: Open

suite	result
analogjs	⏹️ cancelled
astro	❌ failure
histoire	❌ failure
iles	❌ failure
ladle	✅ success
laravel	✅ success
marko	✅ success
nuxt	✅ success
nx	✅ success
previewjs	✅ success
qwik	✅ success
rakkas	✅ success
sveltekit	✅ success
unocss	✅ success
vite-plugin-pwa	✅ success
vite-plugin-ssr	✅ success
vite-plugin-react	✅ success
vite-plugin-react-pages	✅ success
vite-plugin-react-swc	✅ success
vite-plugin-svelte	✅ success
vite-plugin-vue	✅ success
vite-setup-catalogue	✅ success
vitepress	✅ success
vitest	✅ success

[sapphi-red]

analogjs is failing with ERROR: Experimental decorators are not currently enabled error.
https://github.com/vitejs/vite-ecosystem-ci/actions/runs/5276950022/jobs/9544390272#step:8:802

astro is failing with ReferenceError: window is not defined.
https://github.com/vitejs/vite-ecosystem-ci/actions/runs/5276950022/jobs/9544390386#step:8:2634

histoire and iles is failing with main branch, too.

[sapphi-red]

@bluwy I don't understand why this PR fails with astro. Would you take a look?

• The ReferenceError: window is not defined error for "Client only components", "Client only components subpath", "Dynamic components", "Dynamic components subpath"

  • If I remove comment out these files, the error disappears.
    • https://github.com/withastro/astro/blob/3f1cb6b1a001fb03419a313f72c9f4846b890fe0/packages/astro/test/fixtures/astro-client-only/src/components/logResize.js
    • https://github.com/withastro/astro/blob/3f1cb6b1a001fb03419a313f72c9f4846b890fe0/packages/astro/test/fixtures/astro-dynamic/src/components/logResize.js

• The CSS import order errors for "CSS ordering - import order" and "CSS ordering - import order"

  • I wonder if these tests are a bit wrong. For the first test, there's three styles imported. Each for ../styles/base.css and ../components/Client1.jsx and the inlined one.
    https://github.com/withastro/astro/blob/3f1cb6b1a001fb03419a313f72c9f4846b890fe0/packages/astro/test/fixtures/css-order-import/src/pages/index.astro#L2-L12
    But the test expects two style tags.
    https://github.com/withastro/astro/blob/3f1cb6b1a001fb03419a313f72c9f4846b890fe0/packages/astro/test/css-order-import.test.js#L62-L65
    Before this PR, the order of the style tags is base.css -> inline style -> Client1.css.
    After this PR, it's base.css -> Client1.css -> inline style.
    Same for the second test, there's four styles imported but the test expects three.

[patak-dev]

analogjs is failing with ERROR: Experimental decorators are not currently enabled error.
vitejs/vite-ecosystem-ci/actions/runs/5276950022/jobs/9544390272#step:8:802

I wonder if this would affect other Angular projects. Maybe we could add "experimentalDecorators": true on our side and do the breaking change in Vite 5? Is there a benefit to doing it in the 4.4?

[sapphi-red]

I wonder if this would affect other Angular projects.

I guess there won't be many projects. Because if "experimentalDecorators": true is not set, TypeScript lower than v5 will show a warning, Typescript 5+ will show a type error.

Is there a benefit to doing it in the 4.4?

This breaking chage is required for esbuild's ES decorators support: evanw/esbuild#104 (comment)
The benefit is that we might get support for that in 4.4. Though, I don't know how much ES decorators is used.
One more benefit is the consistency with esbuild. It might be confusing that Vite only supports experimental decorators while esbuild supports ES decorators.

That said, I think it's also fine to delay this breaking change.

[patak-dev]

I guess there won't be many projects. Because if "experimentalDecorators": true is not set, TypeScript lower than v5 will show a warning, Typescript 5+ will show a type error.

I think this is enough, let's align with esbuild as you proposed. cc @brandonroberts

[brandonroberts]

I guess there won't be many projects. Because if "experimentalDecorators": true is not set, TypeScript lower than v5 will show a warning, Typescript 5+ will show a type error.

I think this is enough, let's align with esbuild as you proposed. cc @brandonroberts

Thanks. There isn't a good reason experimentalDecorators: true wouldn't be set in an Angular project.

angular/angular#48096

[bluwy]

@bluwy I don't understand why this PR fails with astro. Would you take a look?

@sapphi-red The problem seems to be that esbuild and this PR now picks up verbatimModuleSyntax, derived from this tsconfig during the tests. And this value affects this transformWithEsbuild usage.

Note that there's a comment that it relied on importsNotUsedAsValues: 'remove' for unused import elision, but now the tsconfig has verbatimModuleSyntax: true that overrides that, preventing the elision. You can check why the elision is needed in this playground, where we want to remove the client:only import side-effects.

In Astro if I add verbatimModuleSyntax: false to have a higher override, it fixes the issue, which we can do if we go forward with this PR.

[sapphi-red]

Ah, I see. Thank you for the explanation!

[patak-dev]

We think we should try this update in 4.4, given that Vite 5 is still 3 months away. We can give a longer testing window, and release 4.4 in the first days of July