-
-
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
refactor: remove deprecated api for 3.0 #5868
Conversation
Hey @ydcjeff 👋 could you update this PR? Then we can start to review and merge it. |
I will switch this PR to draft, please switch back to ready for review when finished, then I can start reviewing |
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'm looking into the code and found
packages/vite/src/node/build.ts
line 425 - 432, not sure if we should somehow just ignore this property? Maybe clone the object and remove the property from the clone?
packages/vite/src/node/importGlob.ts
line 31 - 36, 136+
packages/vite/src/node/server/pluginContainer.ts
line 120+ 🤔
packages/vite/types/hot.d.ts
line 11 - 14
packages/vite/types/importMeta.d.ts
line 12 - 17
This is about rollup deprecating their context function so I think we should keep this until rollup removes them?
There is no output property in rollup's OutputOptions, so we can safely remove it? |
Ah yeah, I assume it's also correct here, because we are using |
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.
Starts to look like ready to merge 🙂
there is no longer a mandatory request for a change
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 ran a global search locally and it looks good.
@antfu sorry, I merged the PR before seeing the |
Description
Additional context
logDeprecationWarning
is removed alongside with deprecated options though it is good to have that function as there can be deprecated options in the future. But, build is failing without removing that function.What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).