Skip to content

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

Merged
sapphi-red merged 3 commits intovitejs:mainfrom
NMinhNguyen:patch-1
May 11, 2023
Merged

fix(create-vite): use "target": "ES2020" in React template#13147
sapphi-red merged 3 commits intovitejs:mainfrom
NMinhNguyen:patch-1

Conversation

@NMinhNguyen
Copy link
Copy Markdown
Contributor

@NMinhNguyen NMinhNguyen commented May 11, 2023

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.

@bolt-new-by-stackblitz
Copy link
Copy Markdown

bolt-new-by-stackblitz bot commented May 11, 2023

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

@sapphi-red
Copy link
Copy Markdown
Member

sapphi-red commented May 11, 2023

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
Copy Markdown
Member

ArnaudBarre commented May 11, 2023

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
Copy Markdown
Contributor Author

NMinhNguyen commented May 11, 2023

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

Ah my bad 😅

NMinhNguyen
NMinhNguyen commented May 11, 2023
View reviewed changes
@NMinhNguyen
Copy link
Copy Markdown
Contributor Author

NMinhNguyen commented May 11, 2023

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?

NMinhNguyen
NMinhNguyen commented May 11, 2023
View reviewed changes
ArnaudBarre
ArnaudBarre approved these changes May 11, 2023
View reviewed changes
Copy link
Copy Markdown
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
sapphi-red
sapphi-red approved these changes May 11, 2023
View reviewed changes
Copy link
Copy Markdown
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
@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

Reviewers

@ArnaudBarre ArnaudBarre ArnaudBarre approved these changes

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@NMinhNguyen @sapphi-red @ArnaudBarre