Resolve and load dependent module paths in react-scripts #530
Conversation
|
Will this PR solve the issue mentioned in this reply? |
|
Probably it will work. You can try it on my forked branch. |
|
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. |
|
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. |
scripts/utils/babelTransform.js
Outdated
| const babelJest = require('babel-jest'); | ||
| const { dependRequire, dependRequireResolve } = require('./dependRequire'); | ||
|
|
||
| const babelJest = dependRequire('babel-jest'); |
There was a problem hiding this comment.
To apply against current master, line 3 needs to be edited to be:
const babelJestMd = dependRequire('babel-jest');
const babelJest = babelJestMd.__esModule ? babelJestMd.default : babelJestMd;This is so that react-app-rewired can use either babel-jest 26 or 27+, since there was a change to using esModules in babel-jest 27.
|
I've tested it with the repository at https://github.com/NiGhTTraX/react-app-rewired-bug-457 and can confirm that it works for the cross-spawn issue. I think it may also work for the babel-jest hoisting issue mentioned in #589 - but the example repo above works in jest with the current master version of babelTransform (i.e. without the dependRequire/dependRequireResolve changes in babelTransform), so I can't be sure on that yet. The conflict that git is reporting is for line 3 in babelTransform.js, was altered to do the esModule vs default export testing in master, so it just needs the patch to be updated to apply against the current master as per the example I provided above. |
|
@dawnmist After fixing the conflicts, I applied my code to https://github.com/NiGhTTraX/react-app-rewired-bug-457 and confirmed that it works. |
|
@Yama-Tomo Thank you for the rapid update! @timarney - I can confirm that the changes work for both a new project with react-scripts 5, and on an older one with react-scripts 3, in addition to an example repo that was having issues finding the |
|
Confirmed that the changes in this pull request also fix finding |
|
Thanks All |
issue #457, #388
Depending on the project, the installation location of the package on which react-scripts depends may change.
This fix resolves and loads the dependency paths of react-scripts.