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

Installing Typescript definitions for react and react-addons-test-utils is inconsistent with npm #4226

Open
StanleyGoldman opened this issue Aug 22, 2017 · 13 comments

Comments

@StanleyGoldman
Copy link

StanleyGoldman commented Aug 22, 2017

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
Performing a yarn add of @types/react@^15.6.1 and @types/react-addons-test-utils creates output that is inconsistent with npm install of the same packages.

The package.json of @types/react-addons-test-utils lists react@* for it's dependency. I'm assuming this is the root of the issue.

If the current behavior is a bug, please provide the steps to reproduce.
yarn add @types/react@^15.6.1 @types/react-addons-test-utils --save-dev

Creates the following..

node_modules/@types/react (containing react 15.6 Typescript definitions)
node_modules/@types/react-addons-test-utils
node_modules/@types/react-addons-test-utils/node_modules/@types/react  (containing react 16.0 Typescript definitions)

What is the expected behavior?
yarn add @types/react@^15.6.1 @types/react-addons-test-utils --save-dev

as compared to..

npm install @types/react@^15.6.1 @types/react-addons-test-utils --save-dev

Creates the following..

node_modules/@types/react (containing react 15.6 Typescript definitions)
node_modules/@types/react-addons-test-utils

Please mention your node.js, yarn and operating system version.

  • Windows 10
  • yarn 0.27.5
  • node v6.11.2
@Johoni
Copy link

Johoni commented Aug 22, 2017

It also happens on other packages like react-bootstrap,
where the package.json in the typings defines:

"dependencies": {
        "@types/react": "*"
    },

@ryanewtaylor
Copy link

ryanewtaylor commented Sep 14, 2017

I have encountered this same issue with

  • node v8.5.0
  • npm 5.3.0
  • yarn 1.0.2
  • Windows 10

@petemahoney
Copy link

Have had similar problems with yarn install with package.json that calls for @types\react-select and @types\react-dom - similar in that after yarn install a @types\react-select\node_modules@types\react folder is created where as in npm install this folder is not created. tsc fails with a bunch of TS1005 errors (expected ",", expected ">" etc) on the yarn install - specifically on the @types\react-select\node_modules@types\react\react.d.ts file. Removing the offending folder (@types/react-select/node_modules) will fix the tsc problem. We've implemented a .yarnclean file to do this on yarn install but would like a solution that avoids the creation of those folders in the first place.

FWIW found similar question on stackoverflow - https://stackoverflow.com/questions/45715891/why-does-yarn-install-node-modules-folder-for-types-angular-dependencies

@StanleyGoldman
Copy link
Author

/me googles ".yarnclean"

@BYK
Copy link
Member

BYK commented Sep 16, 2017

I think the core issue is listing react: "*" as a dependency whereas is should be listed as a peer dependency, right?

@StanleyGoldman
Copy link
Author

I'm not sure what is technically correct. I thought that yarn sought to emulate npm. If that assumption is correct then yarn has a bug.

@Jontem
Copy link

Jontem commented Sep 18, 2017

With the new selective version resolution feature #4105 in 1.0.0 you can work around this by specifying the resolutions property in package.json

  "resolutions": {
    "**/@types/react": "15.0.26"
  }

Also mentioned in the announcement of 1.0

@BYK
Copy link
Member

BYK commented Sep 18, 2017

I'm not sure what is technically correct. I thought that yarn sought to emulate npm. If that assumption is correct then yarn has a bug.

That assumption is not correct. We are aiming to be compatible when it makes sense. If npm's resolution is technically incorrect, Yarn is not going to try emulate that. Even if it is correct, Yarn doesn't strive to get you the same directory structure with npm.

@frankwallis
Copy link

frankwallis commented Sep 18, 2017

@BYK I think the issue is to do with how yarn is handling a dependency of @types/react: "*" because at the moment it seems to install @latest (16.x.x) which causes an extra copy to get installed when another package has specified 15.x.x.

According to semver: * := >=0.0.0 (Any version satisfies) so it should just resolve to 15.x.x and not install another version

@BYK
Copy link
Member

BYK commented Sep 18, 2017

According to semver: * := >=0.0.0 (Any version satisfies) so it should just resolve to 15.x.x and not install another version

It really depends on the order you resolve them. In this case, Yarn encounters * first, happily resolves it to the latest version because we should prioritize latest versions and then encounters one looking for 15.x.x which won't be satisfied with the already resolved version and adds a new copy. It would be better to use peerDependencies for this is two separate packages wants to lock into the same version while still using ranges.

@StanleyGoldman
Copy link
Author

We are aiming to be compatible when it makes sense. If npm's resolution is technically incorrect, Yarn is not going to try emulate that. Even if it is correct, Yarn doesn't strive to get you the same directory structure with npm.

@BYK thanks for the clarification. I can use the workaround and that fixes my problems. Should we file issues with the repositories that cause a need for the workaround and cite this issue? You said they should list react as peer dependency instead?

@frankwallis
Copy link

frankwallis commented Sep 18, 2017

@BYK is that correct though - are we saying that dependency: '*' only relates to packages underneath that package and not to packages above? It seems to me that yarn should do the full resolution before working out which version to use for '*'? I'm not familiar with the yarn resolution process so I don't understand the impact of this but that seems to be what npm is doing.

@3af
Copy link

3af commented Nov 15, 2017

.yarnclean that solved the problem for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants