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

Inline process.browser for better code elimination #2583

Merged

Conversation

garthenweb
Copy link
Contributor

↪️ Pull Request

Working on a project with server-side rendering I want to enable/ disable some code that should only run in a specific environment (browser or server). As I am using the middleware in development mode it was hard to rely on process.env.BROWSER because this is shared between the NodeJS application and the bundler for the client code (as far as I found out there is no way to pass an private/ non global environment variable at runtime to the bundler).
The process module inserts a process.browser already adds the same information I passed as an environment before, but the dead code elimination was not deleated, therefore the file size grow a lot.
Tersers compress.global_defs option also didn't work for me (not 100% sure why), therefore I checked the implementation for process.env and implemented more or less the same for process.browser.

💻 Examples

// before
const test = process.browser
process.browser = true;
if (process.browser) {
  console.log(process.browser);
}
// after
const test = true;

if (true) {
  console.log(true);
}

🚨 Test instructions

I added some integration tests for this feature you can check out, further the example should give a good idea what should happen. On a real-world project, the code archived the same results as using process.env.BROWSER for me.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)

@DeMoorJasper DeMoorJasper merged commit 4c5993f into parcel-bundler:master Feb 6, 2019
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.

2 participants