Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add check-installed-peers rfc #386

Closed
wants to merge 1 commit into from
Closed

Add check-installed-peers rfc #386

wants to merge 1 commit into from

Conversation

Roaders
Copy link

@Roaders Roaders commented May 27, 2021

Discusses the ability to be able to mark a peer dependency as optional so that it does not have to be installed by a consumer but if it is installed by a consumer then the version should be checked against the version specified by the installed dependency.

References


## Summary

Currently `peerDependenciesMeta` (as far as I know) only adds the ability to basically turn off peer dependency warnings for your peer. I would like to add the ability to mark a dependency that may or not be installed (`optional` but that terminology is already used) but who's version should be checked if it is installed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to me like it should be the default behavior. An optional peer dep will likely be try/catch required. If the wrong version exists, the program will happily use it, and perhaps break in terrifyingly silent ways.

I can't think of any use case where the package exists, the version is incompatible, and that's desirable.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to me like it should be the default behavior. An optional peer dep will likely be try/catch required. If the wrong version exists, the program will happily use it, and perhaps break in terrifyingly silent ways.

I can't think of any use case where the package exists, the version is incompatible, and that's desirable.

Yes I completely agree. It would be much easier and better to modify existing behaviour but I was trying to avoid breaking what has already been implemented.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw what this RFC describes is how Yarn implements it. That npm has a slightly different behavior wasn't intended and mostly just an oversight when I ported it. Seems a good idea to align.

@darcyclarke darcyclarke added the Agenda will be discussed at the Open RFC call label Jun 9, 2021
@Roaders
Copy link
Author

Roaders commented Jun 10, 2021

On the open RFC Meeting last night we discussed this and it was confirmed that this is how it should be implemented in npm 7. I have tested this though and it does not seem to work as expected.

@Roaders
Copy link
Author

Roaders commented Jun 16, 2021

Ok, after saying it didn't work as expected. It does IRL (in the real world). It seems where I work we have some specific issues to us.
I will close this PR.
Thanks for the help.

@Roaders Roaders closed this Jun 16, 2021
@isaacs isaacs removed the Agenda will be discussed at the Open RFC call label Jun 23, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants