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

Interpolate variables into template before minification #7029

Conversation

cedrics
Copy link

@cedrics cedrics commented May 10, 2019

Fixes #6448

We ran into a problem where a code snippet in our index.html was removed regardless of the NODE_ENV set:

 <script>
 if ("%NODE_ENV%" === "production") {
  // Production only configuration
}
 </script>

The problem was that the interpolation was only registered after the minification already ran. So from the minifier's point of view "%NODE_ENV%" === "production" is always false and the code block can therefore be eliminated.

I tested this out by using a local copy of react-scripts & react-dev-tools and rebuilding our project. I think the only potential bug this change could introduce would be if any steps between the template generation and the emit would actually themselves generate interpolated variables. Those would not be converted by the PR as it is. It may be possible to run the interpolation for both hooks if this turns out to be a problem.

Otherwise code snippets like `if (%NODE_ENV% === production)` will always be removed
as %NODE_ENV% is not yet being replaced by the current environment
@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@GuiHash
Copy link
Contributor

GuiHash commented May 19, 2019

Already fixed in #6449 (but not merge since February) 😅

@stale
Copy link

stale bot commented Jun 18, 2019

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jun 18, 2019
@iansu
Copy link
Contributor

iansu commented Jun 19, 2019

Thanks for your contribution but I'm going to close this in favour of #6449

@iansu iansu closed this Jun 19, 2019
@cedrics cedrics deleted the interpolate-template-before-minification branch June 19, 2019 08:58
@cedrics
Copy link
Author

cedrics commented Jun 19, 2019

Ok, thx. Sorry missed the other PR and the comment linking to it otherwise I would have closed this myself.

@lock lock bot locked and limited conversation to collaborators Jun 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong InterpolateHtmlPlugin hook for env var replacement
4 participants