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

feat: build.target: ['nodeXX'] ignore replace process.env #8843

Closed
wants to merge 1 commit into from

Conversation

caoxiemeihao
Copy link
Contributor

@caoxiemeihao caoxiemeihao commented Jun 29, 2022

Description

When I use Vite and explicitly specify that the build target is node(Electron App), I hope to bypass the definePlugin

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 Commit 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.

@netlify
Copy link

netlify bot commented Jun 29, 2022

Deploy Preview for vite-docs-main canceled.

Name Link
🔨 Latest commit bcf7967
🔍 Latest deploy log https://app.netlify.com/sites/vite-docs-main/deploys/62c250635c0e2100091bc1d9

@caoxiemeihao caoxiemeihao force-pushed the patch-1 branch 2 times, most recently from 16721e1 to 2b75821 Compare June 29, 2022 02:36
@bluwy
Copy link
Member

bluwy commented Jul 3, 2022

Perhaps we want to make the fix at

const replaceProcessEnv = !ssr || config.ssr?.target === 'webworker'
instead.

I don't think we've considered having special treatment for build.target: ['nodeXX'] yet, and so far support for this has been discussed in #7821 only. I'll put this in the team board and let's see how we'd want to support this. I'm assuming this is commonly needed for electron.

@bluwy bluwy added p2-to-be-discussed Enhancement under consideration (priority) labels Jul 3, 2022
@caoxiemeihao
Copy link
Contributor Author

@bluwy Thanks for your reply, this is indeed an Electron specific scenario. In my project, I had to avoid the vite:define plugin by writing the wizard.

// 🚧 Use ['ENV_NAME'] avoid vite:define plugin
const url = `http://${process.env['VITE_DEV_SERVER_HOST']}:${process.env['VITE_DEV_SERVER_PORT']}`

My project 👉 electron-vite/electron-vite-vue

@caoxiemeihao caoxiemeihao changed the title feat: target='node' ignore replace process.env feat: build.target: ['nodeXX'] ignore replace process.env Jul 4, 2022
@caoxiemeihao caoxiemeihao changed the title feat: build.target: ['nodeXX'] ignore replace process.env feat: build.target: ['nodeXX'] ignore replace process.env Jul 4, 2022
@benmccann
Copy link
Collaborator

Also proposed in #7810. #5577 seems like it might be targeting the same problem as well

@caoxiemeihao
Copy link
Contributor Author

Yeah! It seems that process.env.XXX always works fine so far.

@bluwy bluwy removed the p2-to-be-discussed Enhancement under consideration (priority) label Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants