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(resolve): don't set builtinModules to external by default #18821

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Nov 28, 2024

Description

I didn't intend to include this diff, but got included mistakenly.
d002e7d#diff-11e17761d4ecfee8f8fde15c6d79b7bc0260176396a30dfd8e6f6bbaf5af4745L743-R772
No, it was for d002e7d#diff-9b81bb364c02eab9494a7d27a5effc400cacbffd3b8f349c192f890c37bfc83fL450.
But now that I think we should not set, builtinModules to resolve.external because if a user set external: ['foo'] then, the builtin modules will go away. I think we do need resolve.builtins.

I'll focus on fixing the issue in this PR, let's do the resolve.builtins thing separately.

fixes #18810

@sapphi-red sapphi-red added p3-minor-bug An edge case that only affects very specific usage (priority) feat: ssr regression The issue only appears after a new release labels Nov 28, 2024
@sapphi-red sapphi-red marked this pull request as draft November 28, 2024 10:26
@sapphi-red sapphi-red marked this pull request as ready for review November 29, 2024 01:55
@sapphi-red
Copy link
Member Author

/ecosystem-ci run

Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Didn't occur to me that the builtins are getting overriden too.

@patak-dev patak-dev merged commit 2250ffa into vitejs:main Nov 29, 2024
15 checks passed
@dario-piotrowicz
Copy link
Contributor

let's do the resolve.builtins thing separately.

@sapphi-red 😄 #18584

@sapphi-red sapphi-red deleted the fix/dont-set-builtin-modules-to-external-by-default branch December 2, 2024 06:15
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Dec 3, 2024
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | vite    | 6.0.1 | 6.0.2 |


## [v6.0.2](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small602-2024-12-02-small)

-   chore: run typecheck in unit tests ([#18858](vitejs/vite#18858)) ([49f20bb](vitejs/vite@49f20bb)), closes [#18858](vitejs/vite#18858)
-   chore: update broken links in changelog ([#18802](vitejs/vite#18802)) ([cb754f8](vitejs/vite@cb754f8)), closes [#18802](vitejs/vite#18802)
-   chore: update broken links in changelog ([#18804](vitejs/vite#18804)) ([47ec49f](vitejs/vite@47ec49f)), closes [#18804](vitejs/vite#18804)
-   fix: don't store temporary vite config file in `node_modules` if deno ([#18823](vitejs/vite#18823)) ([a20267b](vitejs/vite@a20267b)), closes [#18823](vitejs/vite#18823)
-   fix(css): referencing aliased svg asset with lightningcss enabled errored ([#18819](vitejs/vite#18819)) ([ae68958](vitejs/vite@ae68958)), closes [#18819](vitejs/vite#18819)
-   fix(manifest): use `style.css` as a key for the style file for `cssCodesplit: false` ([#18820](vitejs/vite#18820)) ([ec51115](vitejs/vite@ec51115)), closes [#18820](vitejs/vite#18820)
-   fix(optimizer): resolve all promises when cancelled ([#18826](vitejs/vite#18826)) ([d6e6194](vitejs/vite@d6e6194)), closes [#18826](vitejs/vite#18826)
-   fix(resolve): don't set builtinModules to `external` by default ([#18821](vitejs/vite#18821)) ([2250ffa](vitejs/vite@2250ffa)), closes [#18821](vitejs/vite#18821)
-   fix(ssr): set `ssr.target: 'webworker'` defaults as fallback ([#18827](vitejs/vite#18827)) ([b39e696](vitejs/vite@b39e696)), closes [#18827](vitejs/vite#18827)
-   feat(css): format lightningcss error ([#18818](vitejs/vite#18818)) ([dac7992](vitejs/vite@dac7992)), closes [#18818](vitejs/vite#18818)
-   refactor: make properties of ResolvedServerOptions and ResolvedPreviewOptions required ([#18796](vitejs/vite#18796)) ([51a5569](vitejs/vite@51a5569)), closes [#18796](vitejs/vite#18796)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: ssr p3-minor-bug An edge case that only affects very specific usage (priority) regression The issue only appears after a new release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vite 6 change in behavior with noExternal & imports that share name with node builtins
4 participants