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.

Sign up for GitHub

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

Jump to bottom

Support webpack config factory (CRA v2.1.2) #346

Merged
merged 4 commits into from
Jan 6, 2019
Merged

Support webpack config factory (CRA v2.1.2) #346

merged 4 commits into from
Jan 6, 2019

Conversation

tiffon
Copy link
Contributor

@tiffon tiffon commented Jan 2, 2019

Fixes #343.

Builds on #344.

Ran into problems with this and had to fix it. #344 was close but didn't quite work because of parens and ternary.

CRA uses node >= 6, so template strings and destructuring should be ok.

There is a lot of overlap between scripts/start and scripts/build. I thought about merging them, but figured best to keep it simple.

@timarney
Copy link
Owner

timarney commented Jan 2, 2019

If anyone wants to try / preview
yarn add react-app-rewired@2.0.2-next.0

tiffon added a commit to jaegertracing/jaeger-ui that referenced this pull request Jan 2, 2019
@tiffon
WIP upgrade to create-react-app v2.1.2
8c30fbc 
Requires changes from PR #346 which are published as
react-app-rewired@2.0.2-next.0

timarney/react-app-rewired#346
@tiffon tiffon mentioned this pull request Jan 2, 2019
Closed
tiffon added a commit to jaegertracing/jaeger-ui that referenced this pull request Jan 2, 2019
@tiffon
WIP upgrade to create-react-app v2.1.2
647c6e7 
Requires changes from PR #346 which are published as
react-app-rewired@2.0.2-next.0

timarney/react-app-rewired#346
Signed-off-by: Joe Farro <joef@uber.com>
@tiffon tiffon mentioned this pull request Jan 2, 2019
Merged
@timarney
Copy link
Owner

timarney commented Jan 3, 2019

Assuming everyone is happy with this...

I think when it gets merged we will pull out "The built-in rewirers" as those are broken and release a major version 2.0.0

@ItsWendell
Copy link

@timarney Would be very valuable for us and many to have this properly working again!

@tiffon
Copy link
Contributor Author

tiffon commented Jan 4, 2019

@timarney Is the jest babelTransform the only build-in rewire being applied?

https://github.com/timarney/react-app-rewired/blob/master/packages/react-app-rewired/scripts/utils/rewireJestConfig.js#L8-L12

@timarney
Copy link
Owner

timarney commented Jan 4, 2019

I believe so ... the decoratorsPluginPath stuff can all go.

@tiffon
Copy link
Contributor Author

tiffon commented Jan 4, 2019

FYI, I have confirmed react-app-rewired@2.0.2-next.0 works in the following two scenarios:

react-scripts@1.0.11 with this config-overrides.js file

react-scripts@2.1.2 with this config-overrides.js file

@tiffon
Copy link
Contributor Author

tiffon commented Jan 4, 2019

@timarney Are you saying the only the decoratorsPluginPath content from the utils/babelTransform.js should be removed? Or, would it make sense to the remove utils/babelTransform.js, entirely?

@timarney
Copy link
Owner

timarney commented Jan 5, 2019

Let's remove it.

@tiffon tiffon mentioned this pull request Jan 6, 2019
Merged
@timarney timarney merged commit db4ed32 into timarney:master Jan 6, 2019
This was referenced Jan 6, 2019
Closed
Closed
tiffon added a commit to jaegertracing/jaeger-ui that referenced this pull request Jan 6, 2019
@tiffon
WIP upgrade to create-react-app v2.1.2
db8f1c7 
Requires changes from PR #346 which are published as
react-app-rewired@2.0.2-next.0

timarney/react-app-rewired#346

Signed-off-by: Joe Farro <joef@uber.com>
@danylomarkov
Copy link
Contributor

@timarney @tiffon Why don't we need this utils/babelTransform.js file? In my particular case I'm applying several babel plugins for jest via .babelrc and now I have only option to add jest override manually like in @tiffon's config-overrides.js.
It seems to be a common issue which folks could face when upgrading to v2.

@timarney
Copy link
Owner

timarney commented Jan 8, 2019

We can add that back.

@alex3683 alex3683 mentioned this pull request Jan 9, 2019
Closed
@danylomarkov
Copy link
Contributor

@timarney So I added a PR which brings it back (except decorators stuff) #350

tiffon added a commit to jaegertracing/jaeger-ui that referenced this pull request Jan 9, 2019
@tiffon
Update create-react-app to 2.1.2 (#302)
7a6190e 
* WIP upgrade to create-react-app v2.1.2

Requires changes from PR #346 which are published as
react-app-rewired@2.0.2-next.0

timarney/react-app-rewired#346

Signed-off-by: Joe Farro <joef@uber.com>

* Use node 8, less liberal browser support, fix test

Signed-off-by: Joe Farro <joef@uber.com>

* Use eslintrc, fix CI build, fix pre-commit hook

Make sure the ./packages/jaeger-ui uses the ./.eslintrc.

Make sure all tests are run in pre-commit hook.

CRA is now failing builds in CI if there are any webpack build warnings:

https://github.com/facebook/create-react-app/blob/73e3d0ebf1f2834e1c8c41d3a25ae5e0e99e6f14/packages/react-scripts/scripts/build.js#L171-L184
Signed-off-by: Joe Farro <joef@uber.com>

* Skip react-vis.css format check, fail-fast in CI

Signed-off-by: Joe Farro <joef@uber.com>

* Don't collect coverage from dev proxy setup

Signed-off-by: Joe Farro <joef@uber.com>

* Upgrade react-app-rewired to 2.0.1

Avoid issue yarnpkg/yarn#6300

Signed-off-by: Joe Farro <joef@uber.com>

* Cleanup npm packages in packages/jaeger-ui

Signed-off-by: Joe Farro <joef@uber.com>
everett980 pushed a commit to everett980/jaeger-ui that referenced this pull request Jan 16, 2019
@tiffon @everett980
Update create-react-app to 2.1.2 (jaegertracing#302)
6ea7d6a 
* WIP upgrade to create-react-app v2.1.2

Requires changes from PR jaegertracing#346 which are published as
react-app-rewired@2.0.2-next.0

timarney/react-app-rewired#346

Signed-off-by: Joe Farro <joef@uber.com>

* Use node 8, less liberal browser support, fix test

Signed-off-by: Joe Farro <joef@uber.com>

* Use eslintrc, fix CI build, fix pre-commit hook

Make sure the ./packages/jaeger-ui uses the ./.eslintrc.

Make sure all tests are run in pre-commit hook.

CRA is now failing builds in CI if there are any webpack build warnings:

https://github.com/facebook/create-react-app/blob/73e3d0ebf1f2834e1c8c41d3a25ae5e0e99e6f14/packages/react-scripts/scripts/build.js#L171-L184
Signed-off-by: Joe Farro <joef@uber.com>

* Skip react-vis.css format check, fail-fast in CI

Signed-off-by: Joe Farro <joef@uber.com>

* Don't collect coverage from dev proxy setup

Signed-off-by: Joe Farro <joef@uber.com>

* Upgrade react-app-rewired to 2.0.1

Avoid issue yarnpkg/yarn#6300

Signed-off-by: Joe Farro <joef@uber.com>

* Cleanup npm packages in packages/jaeger-ui

Signed-off-by: Joe Farro <joef@uber.com>
Signed-off-by: Everett Ross <reverett@uber.com>
vvvprabhakar pushed a commit to vvvprabhakar/jaeger-ui that referenced this pull request Jul 5, 2021
@tiffon
Update create-react-app to 2.1.2 (jaegertracing#302)
98323de 
* WIP upgrade to create-react-app v2.1.2

Requires changes from PR jaegertracing#346 which are published as
react-app-rewired@2.0.2-next.0

timarney/react-app-rewired#346

Signed-off-by: Joe Farro <joef@uber.com>

* Use node 8, less liberal browser support, fix test

Signed-off-by: Joe Farro <joef@uber.com>

* Use eslintrc, fix CI build, fix pre-commit hook

Make sure the ./packages/jaeger-ui uses the ./.eslintrc.

Make sure all tests are run in pre-commit hook.

CRA is now failing builds in CI if there are any webpack build warnings:

https://github.com/facebook/create-react-app/blob/73e3d0ebf1f2834e1c8c41d3a25ae5e0e99e6f14/packages/react-scripts/scripts/build.js#L171-L184
Signed-off-by: Joe Farro <joef@uber.com>

* Skip react-vis.css format check, fail-fast in CI

Signed-off-by: Joe Farro <joef@uber.com>

* Don't collect coverage from dev proxy setup

Signed-off-by: Joe Farro <joef@uber.com>

* Upgrade react-app-rewired to 2.0.1

Avoid issue yarnpkg/yarn#6300

Signed-off-by: Joe Farro <joef@uber.com>

* Cleanup npm packages in packages/jaeger-ui

Signed-off-by: Joe Farro <joef@uber.com>

Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
vvvprabhakar pushed a commit to vvvprabhakar/jaeger-ui that referenced this pull request Jul 5, 2021
@tiffon @everett980
Update create-react-app to 2.1.2 (jaegertracing#302)
b17ff16 
* WIP upgrade to create-react-app v2.1.2

Requires changes from PR jaegertracing#346 which are published as
react-app-rewired@2.0.2-next.0

timarney/react-app-rewired#346

Signed-off-by: Joe Farro <joef@uber.com>

* Use node 8, less liberal browser support, fix test

Signed-off-by: Joe Farro <joef@uber.com>

* Use eslintrc, fix CI build, fix pre-commit hook

Make sure the ./packages/jaeger-ui uses the ./.eslintrc.

Make sure all tests are run in pre-commit hook.

CRA is now failing builds in CI if there are any webpack build warnings:

https://github.com/facebook/create-react-app/blob/73e3d0ebf1f2834e1c8c41d3a25ae5e0e99e6f14/packages/react-scripts/scripts/build.js#L171-L184
Signed-off-by: Joe Farro <joef@uber.com>

* Skip react-vis.css format check, fail-fast in CI

Signed-off-by: Joe Farro <joef@uber.com>

* Don't collect coverage from dev proxy setup

Signed-off-by: Joe Farro <joef@uber.com>

* Upgrade react-app-rewired to 2.0.1

Avoid issue yarnpkg/yarn#6300

Signed-off-by: Joe Farro <joef@uber.com>

* Cleanup npm packages in packages/jaeger-ui

Signed-off-by: Joe Farro <joef@uber.com>
Signed-off-by: Everett Ross <reverett@uber.com>

Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

react-app-rewired breaks w/ react-scripts 2.1.2
5 participants
@tiffon @timarney @ItsWendell @danylomarkov @vdfor