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

fix(create-vite): use "target": "ES2020" in React template #13147

Merged
merged 3 commits into from
May 11, 2023

Conversation

NMinhNguyen
Copy link
Contributor

Description

Unless I am mistaken, I think this flag was unintentionally dropped in #12604 (cc @ArnaudBarre). This PR restores it for consistency with the other templates.

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
Copy link

stackblitz bot commented May 11, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@sapphi-red
Copy link
Member

template-react-ts has "target": "ESNext", so useDefineForClassFields is enabled by default.

@ArnaudBarre I found other templates uses "target": "ES2020". Are there any reasons to use ESNext just for react? or is this just a mistake?

@ArnaudBarre
Copy link
Member

This is clearly is mistake, it should be ES2020 (for lib, the others are mostly decorative given that this is configured in Vite) + useDefineForClassFields

@NMinhNguyen
Copy link
Contributor Author

template-react-ts has "target": "ESNext", so useDefineForClassFields is enabled by default.

Ah my bad 😅

@NMinhNguyen
Copy link
Contributor Author

This is clearly is mistake, it should be ES2020 (for lib, the others are mostly decorative given that this is configured in Vite) + useDefineForClassFields

Wait, so target should remain as ESNext?

Copy link
Member

@ArnaudBarre ArnaudBarre left a comment

Choose a reason for hiding this comment

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

Yeah "module" doesn't bring much and Vite will jump on the new features when they arrive for that so I think this good like this

@sapphi-red sapphi-red changed the title fix(create-vite): re-enable useDefineForClassFields in React template fix(create-vite): use "target": "ES2020" in React template May 11, 2023
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 for updating!

@sapphi-red sapphi-red merged commit 23096b1 into vitejs:main May 11, 2023
14 checks passed
@NMinhNguyen NMinhNguyen deleted the patch-1 branch May 11, 2023 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants