-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[FEATURE] Document the peerDependenciesMeta in the package.json #1247
Comments
Any news? Thanks in advance! |
Formal documentation on this feature would be great. I know npm 7+ is going back to the behavior of installing Edit: I've confirmed locally that |
This isn't _strictly_ necessary, but it is an accurate description of the requirements of the project. We go with the range of "^3.6.3" to match the lowest range that we're currently using in any project (which is found in liferay-js-toolkit), to minimize the risk of introducing duplicates in a consuming project. If you have a ".ts" file in your project, you are going to have `typescript` in your `devDependencies`, and our `overrides` configuration is going to use `@typescript-eslint/parser`, which in turn is going to require `typescript`, transitively, in `@typescript-eslint/typescript-estree` here: https://github.com/typescript-eslint/typescript-eslint/blob/c5ffe8833da7875e82953de117b0abe6fa8a60b0/packages/typescript-estree/src/parser.ts#L2 And all will work, even though `@typescript-eslint/typescript-estree` doesn't explicitly declare its hard dependency on `typescript` in its package.json: https://github.com/typescript-eslint/typescript-eslint/blob/c5ffe8833da7875e82953de117b0abe6fa8a60b0/packages/typescript-estree/package.json#L41-L49 But if you don't have any ".ts", then you shouldn't need to have `typescript` in your dependency graph, and we won't use `@typescript/eslint-parser`. So, it makes sense for us to call this an `peerDependency` and mark it as `optional` as per these posts: - https://medium.com/@noamgjacobsonknzi/what-is-peerdependenciesmeta-acea887c32dd - npm/cli#1247 (Note that the `peerDependenciesMeta` field isn't documented yet.) You could also argue that this should be under `optionalDependencies`: https://docs.npmjs.com/files/package.json#optionaldependencies But I think "optional peer" comes closer to the reality here. `optionalDependencies` is for things that you can add, but the package should work if they're not there. That's not the case here. The package won't work if you have ".ts" files but no `typescript`. Related: #91 (comment)
This isn't _strictly_ necessary, but it is an accurate description of the requirements of the project. We go with the somewhat unsatisfying range of "*" to to minimize the risk of introducing duplicates in a consuming project. At the moment, we're using v4+ in this monorepo, 3.9.7 in liferay-portal, and the lowest range that we're currently using in any other project is "^3.6.3" (in liferay-js-toolkit). In the future, we should be able to converge on v4+. If you have a ".ts" file in your project, you are going to have `typescript` in your `devDependencies`, and our `overrides` configuration is going to use `@typescript-eslint/parser`, which in turn is going to require `typescript`, transitively, in `@typescript-eslint/typescript-estree` here: https://github.com/typescript-eslint/typescript-eslint/blob/c5ffe8833da7875e82953de117b0abe6fa8a60b0/packages/typescript-estree/src/parser.ts#L2 And all will work, even though `@typescript-eslint/typescript-estree` doesn't explicitly declare its hard dependency on `typescript` in its package.json: https://github.com/typescript-eslint/typescript-eslint/blob/c5ffe8833da7875e82953de117b0abe6fa8a60b0/packages/typescript-estree/package.json#L41-L49 But if you don't have any ".ts", then you shouldn't need to have `typescript` in your dependency graph, and we won't use `@typescript/eslint-parser`. So, it makes sense for us to call this an `peerDependency` and mark it as `optional` as per these posts: - https://medium.com/@noamgjacobsonknzi/what-is-peerdependenciesmeta-acea887c32dd - npm/cli#1247 (Note that the `peerDependenciesMeta` field isn't documented yet.) You could also argue that this should be under `optionalDependencies`: https://docs.npmjs.com/files/package.json#optionaldependencies But I think "optional peer" comes closer to the reality here. `optionalDependencies` is for things that you can add, but the package should work if they're not there. That's not the case here. The package won't work if you have ".ts" files but no `typescript`. Related: #91 (comment)
Any news? |
Is |
@wheresrhys they're entirely different, albeit similar. |
Thanks. I had to read that through a few times to grasp it, but I think I've got it now. This distinction really needs documenting properly. I've just had a PR submitted wheresrhys/fetch-mock-jest#22 where the submitter almost certainly wants I'd be happy to help, but maybe best as a non-expert reviewer as I don't think I have the familiarity needed to actually write the docs |
I cant find info about the
peerDependenciesMeta
field in the package.json:https://docs.npmjs.com/files/package.json
References
It was mentioned at the npm blog here:
https://blog.npmjs.org/post/186983646370/npm-cli-roadmap-summer-2019
Also here in the changelog:
https://github.com/npm/cli/releases/tag/v6.11.0
Also the release announcement:
https://npm.community/t/release-6-11-0/9572
The text was updated successfully, but these errors were encountered: