Skip to content

Edit InterpolateHtmlPlugin hook (#6448)#6449

Merged
iansu merged 1 commit into
react:masterfrom
GuiHash:master
Oct 23, 2019
Merged

Edit InterpolateHtmlPlugin hook (#6448)#6449
iansu merged 1 commit into
react:masterfrom
GuiHash:master

Conversation

@GuiHash

@GuiHash GuiHash commented Feb 17, 2019

Copy link
Copy Markdown
Contributor

Fixes #6448

@olessavluk

olessavluk commented Mar 20, 2019

Copy link
Copy Markdown

Hi @GuiHash
I have tried to use your patch but result is the same. Could you please create example repo and steps on how to test your change?

What I have done:

  1. yarn add file:/Users/olessavluk/tmp/create-react-app/packages/react-dev-utils/ - install patched package
  2. yarn why react-dev-utils - to verify that I have only one package installed
  3. open file less ./node_modules/react-dev-utils/InterpolateHtmlPlugin.js and verify that .afterTemplateExecution.tap( is present in this file
  4. run yarn buid
  5. less ./build/index.html and still see old value <script>var isProduction=!1</script>

@GuiHash

GuiHash commented Mar 22, 2019

Copy link
Copy Markdown
Contributor Author

Hi @olessavluk,

No need to create an example repo.
Here are the steps to test the changes :

  1. clone repository : git clone https://github.com/GuiHash/create-react-app
  2. go to directory cd create-react-app/packages/react-dev-utils
  3. create link for react-dev-utils : yarn link
  4. go to your bugged repository : cd your-repo
  5. link react-dev-utils : yarn link "react-dev-utils"
  6. run yarn build
  7. 🚀

@olessavluk

olessavluk commented Mar 24, 2019 •
edited
Loading

Copy link
Copy Markdown

Hi @GuiHash

Thanks for providing steps to test this package.
I have created test repo and tried to this steps, but got an error - https://asciinema.org/a/fCpTGKHHOb6amRLDTUmrOySe7

Error: Cannot find module 'react-dev-utils/ForkTsCheckerWebpackPlugin' 
~/t/cra-env-interpolation (master|✔) $ yarn build
yarn run v1.13.0
$ react-scripts build
internal/modules/cjs/loader.js:584
    throw err;
    ^

Error: Cannot find module 'react-dev-utils/ForkTsCheckerWebpackPlugin'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:582:15)
    at Function.Module._load (internal/modules/cjs/loader.js:508:25)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (/Users/olessavluk/tmp/cra-env-interpolation/node_modules/react-scripts/config/webpack.config.js:32:36)
    at Module._compile (internal/modules/cjs/loader.js:701:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:712:10)
    at Module.load (internal/modules/cjs/loader.js:600:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:539:12)
    at Function.Module._load (internal/modules/cjs/loader.js:531:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (/Users/olessavluk/tmp/cra-env-interpolation/node_modules/react-scripts/scripts/build.js:39:23)
    at Module._compile (internal/modules/cjs/loader.js:701:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:712:10)
    at Module.load (internal/modules/cjs/loader.js:600:32)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Also when I tried to use your changes via yarn add file://... result have not changed, variables still injected after optimizations.

@GuiHash

GuiHash commented Mar 24, 2019

Copy link
Copy Markdown
Contributor Author

@olessavluk,

your react-scripts dependency must be in version 2.1.5 to work !

@olessavluk

olessavluk commented Mar 24, 2019

Copy link
Copy Markdown

@GuiHash yes, now it works, index.html is correctly optimized. Thanks for the explanation!

@GuiHash GuiHash mentioned this pull request May 19, 2019
Closed
@iansu iansu added this to the 3.1 milestone Jun 19, 2019
@heyimalex heyimalex mentioned this pull request Jul 29, 2019
Open
@iansu iansu merged commit 42140e5 into react:master Oct 23, 2019
@iansu

iansu commented Oct 23, 2019

Copy link
Copy Markdown
Contributor

Thanks!

@lock lock Bot locked and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@iansu iansu iansu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CLA Signed tag: bug fix

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

Wrong InterpolateHtmlPlugin hook for env var replacement

5 participants

@GuiHash @olessavluk @iansu @facebook-github-bot @bugzpodder