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.

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

react-scripts bug with peer-deps when locally linked #6579

Closed
transitive-bullshit opened this issue Mar 5, 2019 · 2 comments · Fixed by #6580
Closed

react-scripts bug with peer-deps when locally linked #6579

transitive-bullshit opened this issue Mar 5, 2019 · 2 comments · Fixed by #6580

Comments

@transitive-bullshit
Copy link
Contributor

transitive-bullshit commented Mar 5, 2019

I am the main author behind the popular create-react-library package, and there is currently a bug preventing us from switching to using CRA v2 for the example projects.

This is related to locally linked packages and you may relate it somewhat to monorepo support, but it's such a simple use case, repro scenario, and suggested fix, that I wanted to open an issue for discussion.

Is this a bug report?

Yes.

Did you try recovering your dependencies?

Yes.

node --version v10.3.0
yarn --version 1.13.0

Environment

Environment Info:

  System:
    OS: macOS High Sierra 10.13.6
    CPU: x64 Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
  Binaries:
    Node: 10.3.0 - ~/.nvm/versions/node/v10.3.0/bin/node
    Yarn: 1.13.0 - ~/.nvm/versions/node/v10.3.0/bin/yarn
    npm: 6.8.0 - ~/.nvm/versions/node/v10.3.0/bin/npm
  Browsers:
    Chrome: 72.0.3626.119
    Safari: 11.1.2
  npmPackages:
    react: ^16.8.3 => 16.8.3 
    react-dom: ^16.8.3 => 16.8.3 
    react-scripts: ^2.1.5 => 2.1.5 
  npmGlobalPackages:
    create-react-app: Not Found

Steps to Reproduce

I created a minimal repro case here.

  1. Clone the repro, run yarn at top-level and then also run yarn in the CRA example directory which references the top-level package.
  2. The top-level package imports react which is a peer-dependency. This peer-dep is installed as a normal dependency of the CRA example, so it exists in example/node_modules, but it does not exist in the top-level node_modules.
  3. Run yarn build or yarn start from the example directory.

Expected Behavior

The app should run correctly and display a minimal CRA page with a single component.

Note that this works fine with CRA v1 and appears to be a regression with CRA v2.

Actual Behavior

You get the following error, even though react is installed as a dependency in the CRA package.

Failed to compile.

/Users/tfischer/dev/modules/nala-test/index.js
Cannot find module: 'react'. Make sure this package is installed.

You can install this package by running: yarn add react.

Reproducible Demo

See https://github.com/transitive-bullshit/react-scripts-link-peer-deps-bug-repro and the repro steps above.

Note that this behavior repros even if you don't use the link:.. syntax and instead actually yarn link the dependent package. In both cases, yarn is creating a symlink to the package and webpack only tries to resolve react from the relative node_modules directories, but it does not try to resolve react from the app's root node_modules directory which imho is definitely a bug with react-script's webpack config.

Note that you can fix this issue by running NODE_PATH=node_modules yarn start, which results in this webpack config line containing the full, absolute path to paths.appNodeModules instead of just node_modules as a relative path.

For now, I can work around this by adding NODE_PATH=node_modules to the create-react-library template, but this should really not be necessary and took me awhile to track down. I will be submitting a PR with my suggested fix shortly.

@stale
Copy link

stale bot commented Apr 4, 2019

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Apr 4, 2019
@transitive-bullshit
Copy link
Contributor Author

I know this isn't a huge issue, but the problem is a well-defined regression from CRA v1 and imho the workaround I've proposed in #6580 is pretty simple with minimal risk of causing unintended consequences.

It'd be a shame if this was auto-closed given that so many people depend on create-react-library.

💕

@stale stale bot removed the stale label Apr 5, 2019
@lock lock bot locked and limited conversation to collaborators Apr 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant