search

timarney / react-app-rewired

Override create-react-app webpack configs without ejecting
MIT License
9.77k stars 425 forks source link

fix: Require cross-spawn directly instead of from react-dev-utils #559

Closed NiGhTTraX closed 2 years ago

NiGhTTraX commented 2 years ago

This fixes the issue where a package manager doesn't hoist react-dev-utils so we can require it directly. The particular path we're requiring just re-exports cross-spawn so I've added it as a direct dependency.

Fixes #457 and is an alternative to #530.

timarney commented 2 years ago

If @dawnmist or others can confirm this is needed --- happy to merge.

I haven't used Create React App or react-app-rewired for a number of years now so relying on others to let me know if this breaks or maintains things.

dawnmist commented 2 years ago

My only concern with this is that we're now tying react-scripts to a particular version of cross-spawn separately to whatever version is in use by the particular version of react-scripts that the user is using. In that respect #530 may be a more flexible/future-proof solution.

Does anyone have an example mini-repo that triggers the issue that I can use for testing the fix against? I can create a "normal" example, but not having used yarn workspaces it'll take me longer to try to put something together to test that the bug is actually fixed.

NiGhTTraX commented 2 years ago

Hey @dawnmist, check out https://github.com/NiGhTTraX/react-app-rewired-bug-457 for reproduction. I tried both this PR and #530 and they both fix the issue.

timarney commented 2 years ago

Closing this now that #530 is merged.