Update Webpack config location for Dev and Prod#62
Update Webpack config location for Dev and Prod#62patricklafrance merged 1 commit intodilanx:masterfrom
Conversation
|
|
||
| function getWebpackDevConfigPath() { | ||
| return resolveConfigFilePath("webpack.config.dev"); | ||
| return resolveConfigFilePath("webpack.config"); |
There was a problem hiding this comment.
Do you think it's better to check react-scripts version first?
If it's <= v2.1.1, import as is, if not, import using the new approach!
There was a problem hiding this comment.
definitely check the version of react-scripts or try/catch with require.resolve
There was a problem hiding this comment.
May this PR for react-app-rewired helps:
https://github.com/timarney/react-app-rewired/pull/344/files
There was a problem hiding this comment.
Nice feedback, @AndyOGo @marceloadsj
I think we should check the version because each version has a specific change on webpack.config.
There was a problem hiding this comment.
I wonder if craco could just have a version compatibility table, instead of trying to support every version of react-scripts.
E.g. The next version of craco could only support react-scripts >= 2.1.2, and you have to use an earlier version of craco for older versions of react-scripts.
Usually it's good to provide backwards compatibility, but I think this case is different. Everyone should be trying to use the latest version of react-scripts, and if you don't update react-scripts, then there's no reason to update craco. Also you don't need to change your webpack config very often.
This would make it much easier for plugin maintainers as well. This is the approach I'll be using for craco-less and craco-antd. I'm just not going to support older versions of craco and react-scripts. If people need that, then they can just use an older version of craco-less (which works perfectly fine.)
So I think it's fine to have a breaking change here. Instead of working on backwards compatibility, we should be encouraging people to upgrade react-scripts.
There was a problem hiding this comment.
I agree with @ndbroadbent.
I tryed to make it backward compatible for a few minor changes but it get quite hard to maintain.
I'll be glad to accept a PR if someone want to submit a first draft of a version compatibility table.
|
Any idea when this may land and be merged? |
|
It seems that react-app-rewired is already resolved. |
|
Yep it was resolved over there. And it seems rescripts works too: |
|
@patricklafrance React-Scripts released a security fix in version 2.1.3 for this: https://www.npmjs.com/advisories/725 I would assume that 2.1.2 --> 2.1.3 does not revert the changes that are causing the issue solved by this pull request. As such, we are not able to update for the security fix. Can you please merge this, or at least review it? |
|
Any traction on this. We were happy utilizing Craco but now are having to discuss our plans moving forward. If this gets merged it answers that question. Thanks! |
|
Perhaps it is a wise moment to nominate a contributor with merge permissions to help the maintainer with those tasks. |
|
Hi everyone, Sorry for the delay. I had an accident and couldn't use a computer for a few weeks. I will make sure this is merge soon by tomorrow night. And yes @kopax since the project is getting popular I will try to find a maintainer to help. Someone already mention he would be glad to help. |
|
@patricklafrance Hi, I am sorry about your accident and I am glad you are back. Hope you are recovered. |
|
@kopax yes! I still have to take it easy for a few weeks but it's getting better :) |
|
Thank you @hckhanh ! |
|
@patricklafrance you're welcome. I hope you will get well soon 💊 |
10 participants
I make a small changes to deal with the react-scripts
2.1.2to fix #61.This bug come from a MR of facebook/create-react-app#5722
Please take a look and let me know.