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: use esbuild platform browser/node instead of neutral #8714

Merged
merged 8 commits into from
Jun 22, 2022

Conversation

patak-dev
Copy link
Member

Description

fixes #8704
fixes #8702
fixes #8712

Alternative to:

We discussed with @bluwy and @sapphi-red that it is better to use esbuild resolution if possible.

Also, looks like we will need to use 'browser'/'node' instead of 'neutral', I'm seeing code that won't run even with the mainFields defined IIUC in esbuild: https://github.com/evanw/esbuild/blob/79975460813c6765de1df858efd671ea5e7e6897/internal/resolver/package_json.go#L349. Later we could ask a feature to disable the automatic replacement of process.env.NODE_ENV, but seems that it is safer to use the right platform


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@netlify
Copy link

netlify bot commented Jun 22, 2022

Deploy Preview for vite-docs-main ready!

Name Link
🔨 Latest commit 32c00bb
🔍 Latest deploy log https://app.netlify.com/sites/vite-docs-main/deploys/62b2f02ea31ee00008daf4d9
😎 Deploy Preview https://deploy-preview-8714--vite-docs-main.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@patak-dev patak-dev changed the title test(optimizer): add failing test for#8704 fix: use esbuild platform browser/node instead of neutral Jun 22, 2022
@patak-dev patak-dev added the p4-important Violate documented behavior or significantly improves performance (priority) label Jun 22, 2022
@patak-dev patak-dev added this to the 3.0 milestone Jun 22, 2022
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.

Great findings!

packages/vite/src/node/optimizer/index.ts Show resolved Hide resolved
packages/vite/src/node/optimizer/index.ts Outdated Show resolved Hide resolved
Co-authored-by: 翠 / green <green@sapphi.red>
@patak-dev patak-dev merged commit a201cd4 into main Jun 22, 2022
@patak-dev patak-dev deleted the fix/esbuild-conditions-and-main-fields branch June 22, 2022 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4-important Violate documented behavior or significantly improves performance (priority)
Projects
None yet
2 participants