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.

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

Unable to use standard decorators in Vanilla template, using TypeScript #18693

Closed
7 tasks done
trusktr opened this issue Nov 18, 2024 · 6 comments
Closed
7 tasks done

Comments

@trusktr
Copy link

trusktr commented Nov 18, 2024

Describe the bug

TypeScript 5 has supported decorators out of the box since version 5, but Vite's default TypeScript setup has syntax errors with decorators.

Reproduction

https://github.com/trusktr/vite-issue-18693

Steps to reproduce

git clone [email protected]:trusktr/vite-issue-18693.git
cd vite-issue-18693
git checkout 2586f5f17023716cbd0167d26aa9b16050ec53fd
npm clean-install
npm run build # error

System Info

System:
    OS: macOS 15.0.1
    CPU: (8) arm64 Apple M2
    Memory: 139.98 MB / 8.00 GB
    Shell: 5.9 - /opt/homebrew/bin/zsh
  Binaries:
    Node: 20.6.1 - ~/.n-node-versions/bin/node
    Yarn: 1.22.19 - ~/.n-node-versions/bin/yarn
    npm: 10.5.2 - ~/.npm-packages/bin/npm
    pnpm: 9.0.5 - ~/.npm-packages/bin/pnpm
    Watchman: 2023.11.27.00 - /opt/homebrew/bin/watchman
  Browsers:
    Brave Browser: 110.1.48.158
    Chrome: 130.0.6723.117
    Chrome Canary: 133.0.6836.0
    Safari: 18.0.1
    Safari Technology Preview: 18.0
  npmPackages:
    vite: ^5.4.10 => 5.4.11

Used Package Manager

npm

Logs

Click to expand!
❯ npm run build

> [email protected] build
> tsc && vite build

vite v5.4.11 building for production...
✓ 3 modules transformed.
x Build failed in 68ms
error during build:
src/main.ts (34:0): Using the export keyword between a decorator and a class is not allowed. Please use `export @dec class` instead. (Note that you need plugins to import files that are not JavaScript)
file: /Users/trusktr/src/vite-decorator-test/src/main.ts:34:0

32: 
33: export
34: @element('kitchen-sink')
    ^
35: class KitchenSink /*extends HTMLElement*/ {}

    at getRollupError (file:///Users/trusktr/src/vite-decorator-test/node_modules/rollup/dist/es/shared/parseAst.js:396:41)
    at ParseError.initialise (file:///Users/trusktr/src/vite-decorator-test/node_modules/rollup/dist/es/shared/node-entry.js:13645:28)
    at convertNode (file:///Users/trusktr/src/vite-decorator-test/node_modules/rollup/dist/es/shared/node-entry.js:15384:10)
    at convertProgram (file:///Users/trusktr/src/vite-decorator-test/node_modules/rollup/dist/es/shared/node-entry.js:14627:12)
    at Module.setSource (file:///Users/trusktr/src/vite-decorator-test/node_modules/rollup/dist/es/shared/node-entry.js:16368:24)
    at async ModuleLoader.addModuleSource (file:///Users/trusktr/src/vite-decorator-test/node_modules/rollup/dist/es/shared/node-entry.js:20266:13)

Validations

@trusktr
Copy link
Author

trusktr commented Nov 18, 2024

If we modify this code,

export
@element('kitchen-sink')
class KitchenSink /*extends HTMLElement*/ {}

to this,

export {KitchenSink}; // totally separate export statement

@element('kitchen-sink')
class KitchenSink /*extends HTMLElement*/ {}

then npm run build succeeds. However npm run dev still fails with Uncaught SyntaxError: Invalid or unexpected token.

If we modify that code to

@element('kitchen-sink')
export class KitchenSink /*extends HTMLElement*/ {}

then we get the same error message.

Looks like prod vs dev are using different tools that can and cannot handle the syntax.

@trusktr
Copy link
Author

trusktr commented Nov 18, 2024

I tried setting build.target to es2022, which the docs state configures esbuild, but that still results in the build error.

Furthermore, with build.target set to es2022 and the export workaround from above, then dev mode still includes the decorator and causes a client error:

Screenshot 2024-11-17 at 6 49 12 PM

@trusktr
Copy link
Author

trusktr commented Nov 18, 2024

The Vite config is confusing. I was able to fix the issue by not using build.target, and instead using esbuild.target, which is not mentioned in Vite's Build Options docs, and in fact the Build Options docs explains that build.target is used for esbuild, so it isn't clear at all why I would want to use esbuild.target instead.

So instead of this which is not working,

/** @import {UserConfig} from 'vite' */

/** @type {UserConfig} */
export default {
  build: {
    target: 'es2022'
  }
}

this is working,

/** @import {UserConfig} from 'vite' */

/** @type {UserConfig} */
export default {
  esbuild: {
    target: 'es2022'
  }
}

but from the docs it is not clear why there are two target options for seemingly the same purpose, so to avoid any issues, I'm doing the following which based on the docs I've no idea if necessary:

/** @import {UserConfig} from 'vite' */

/** @type {UserConfig} */
export default {
  // Configure both `target`s just in case
  build: {
    target: 'es2022'
  },
  esbuild: {
    target: 'es2022'
  }
}

@hi-ogawa
Copy link
Collaborator

For syntax lowering during dev, esbuild.target or esbuild.supported needs to be used https://main.vite.dev/config/shared-options.html#esbuild. For example #13756 (comment) It's also tracking a documentation issue, so we can close this in favor of that.

@trusktr
Copy link
Author

trusktr commented Nov 18, 2024

Hmm, maybe the thing vite could do that would be more intuitive is to accept a single build.target as the source of truth for all build varieties, otherwise it is confusing. Similar to this comment: #13756 (comment).

I think it is a good idea. If someone wants to test dev mode without the same transform as prod, which is unlikely, there could be an option to override it.

@trusktr
Copy link
Author

trusktr commented Nov 18, 2024

Looking at the type definition compared to docs, its a little confusing. To be safe, the following is what I am doing. Is this how we are supposed to configure the target?

/** @import {UserConfig} from 'vite' */

// Make sure to put the target in all three locations below.
const target = 'es2022'

/** @type {UserConfig} */
export default {
  build: {
    target,
  },
  esbuild: {
    target,
  },
  optimizeDeps: {
    esbuildOptions: {
      target,
    }
  }
}

Here's a working example in the reproduction repo at this commit: https://github.com/trusktr/vite-issue-18693/tree/72aaaff170d5ec5d325fa8a498420862540af757

It seems like a nice idea to default esbuild.target and optimizeDeps.esbuildOptions.target both to build.target (and then override if needed).

@github-actions github-actions bot locked and limited conversation to collaborators Dec 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants