-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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: replace import.meta.hot with undefined in the production #11317
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good change because this will prevent people to write import.meta.hot?.accept
that doesn't work. (related: #9235)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this could initially break things, but given import.meta.hot?.
doesn't work, I think this should be safe and consider it as a bug fix.
Let's merge this in 4.1 too 👍🏼 |
Shouldn't we instead replace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rejecting to bring up the discussion.
I'm at a loss 🤔, choose to modify the type, not the value being replaced
|
Rollup wasn't tree-shake optional chaining, I guess that would be the reason why After that, I think we could replace |
import.meta.hot
Maybe we need to update this part to support vite/packages/vite/src/node/plugins/importAnalysis.ts Lines 404 to 427 in 5444781
|
We could have that in another PR, so this is not blocking |
Description
import.meta.hot
is replaced withfalse
when building the production, notundefined
.vite/packages/vite/src/node/plugins/define.ts
Line 54 in 29abeea
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).