From e4d3c2c01d12df61e6c28223353b50f6e923945c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eloy=20Dur=C3=A1n?= Date: Fri, 28 Sep 2018 12:33:39 +0200 Subject: [PATCH 1/7] Create 0000-deal-with-peer-dependencies.md --- accepted/0000-deal-with-peer-dependencies.md | 154 +++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 accepted/0000-deal-with-peer-dependencies.md diff --git a/accepted/0000-deal-with-peer-dependencies.md b/accepted/0000-deal-with-peer-dependencies.md new file mode 100644 index 0000000..9606d8e --- /dev/null +++ b/accepted/0000-deal-with-peer-dependencies.md @@ -0,0 +1,154 @@ +- Start Date: 2018-09-28 +- RFC PR: (leave this empty) +- Yarn Issue: (leave this empty) + +# Summary + +Allow a developer to deal with peer dependency warnings, without the need to +install all dependencies, thus reducing warning fatigue and drawing attention to +new warnings. + +# Motivation + +[Peer dependencies][peer-deps] are a useful way of informing the developer that +a dependency relies on the host project to provide a transitive dependency +required by the dependency, without imposing that transitive dependency onto the +project’s dependency graph on its own. + +However, there are legitimate cases when the developer may want to ignore a peer +dependency warning. Such as the host not being compatible with the dependency, +the version constraint being too restrictive, or otherwise not being interested +in the dependency. + +While treating warnings as errors is often useful, warnings are specifically not +errors, because they could possibly be ignored after judgment by the developer. + +Currently, the inability to deal with peer dependency warnings, other than +installing the required package/version, leads to warning fatigue, will be +ignored, and will introduce bugs. + +_Regarding a version constraint being too restrictive, this usually comes up +when the transitive dependency’s version is lower than v1.0.0 and the dependency +has a version requirement on it using a semantic-version operator such as `^`. +E.g. `graphql@0.13.0` will not satisfy a requirement like `graphql@^0.12.0`._ + +# Detailed design + +Consider the following peer dependency warnings shown on `yarn install`: + +``` +warning " > @artsy/palette@2.18.0" has unmet peer dependency "react-native@^0.55.4". +warning "@artsy/palette > styled-reset@1.4.0" has incorrect peer dependency "styled-components@>=4.0.0". +``` + +When the developer has determined that they do not want or cannot install these +right now, they add these to the `ignoredPeerDependencies` list of their +`package.json` manifest. + +For instance, as the project the developer is working on is a web project, they +want to always ignore a warning about the `react-native` peer dependency of +`@artsy/palette`: + +```json +{ + "ignoredPeerDependencies": { + "@artsy/palette": ["react-native@*"] + } +} +``` + +The `styled-components` dependency of the transitive `styled-reset` dependency, +however, is one that the developer cannot update to right now and so, _after_ +having verified that the version that they can use works for their use-case, the +developer adds this warning to the ignore list: + +```json +{ + "ignoredPeerDependencies": { + "styled-reset": ["styled-components@>=4.0.0"] + } +} +``` + +Afterwards, running `yarn install` shows no more warnings, until either: + +* any dependency introduces a _new_ peer dependency + + ``` + warning " > react-tracking@5.3.0" has unmet peer dependency "babel-runtime@^6.20.0". + ``` + +* _another_ dependency relies on `react-native` + + ``` + warning " > @artsy/emission@3.42.0" has unmet peer dependency "react-native@^0.55.4". + ``` + +* the `styled-reset` dependency changes its requirement on `styled-components` + + ``` + warning "@artsy/palette > styled-reset@1.5.0" has an updated ignored peer dependency "styled-components@>=3.9.0". + ``` + +In each of these cases, the developer’s attention should be drawn to the warning +so they can deal with it at the time they have the right context. + +An additional warning could be shown when a listed (transitive) dependency no +longer exists in the dependency graph. + +``` +warning "styled-reset" with ignored peer dependencies is no longer depended on. +``` + +Finally, at any given time all warnings should be visible by passing the +`--verbose` flag to `yarn install`. + +# How We Teach This + +This change is only additive and does not change any existing behavior of Yarn +or how developers are supposed to interact with it. The new terminology clearly +communicates the intent and only introduces a negated version of existing +terminology. + +Its communication should follow that of Yarn’s +[`resolutions` feature][yarn-resolutions], in that it is an extra way that a +developer can gain control over their dependency graph and ensure its stability. + +Links to its documentation could be added to the existing documentation on [peer +dependencies][yarn-peer-deps], but otherwise does not need to be altered. + +# Drawbacks + +It’s one more configuration that needs to be documented and understood. + +An alternative that does not introduce a new `package.json` key could be +preferable, but that does not appear to be feasible while maintaining +compatibility with npm. + +# Alternatives + +Introducing a new version requirement negation operator, for example: + +```json +{ + "peerDependencies": { + "styled-reset": "styled-components@!4.0.0" + } +} +``` + +However, this breaks compatibility with npm and, more importantly, doesn’t allow +for multiple peer dependencies of the (transitive) dependency to be ignored. + +# Unresolved questions + +I don’t feel strongly about the configuration, except that I dislike more +configuration. Any suggestions that could lead to the same outcome with less +new configuration would be greatly preferred. + +Perhaps the alternative design listed above could piggy-back on the +[`resolutions` key][yarn-resolutions]? + +[npm-peer-deps]: https://docs.npmjs.com/files/package.json#peerdependencies +[yarn-peer-deps]: https://yarnpkg.com/lang/en/docs/dependency-types/#toc-peerdependencies +[yarn-resolutions]: https://yarnpkg.com/lang/en/docs/selective-version-resolutions/ \ No newline at end of file From 403bd7fa05475bbee9bd1a958a0be3656a1556ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eloy=20Dur=C3=A1n?= Date: Wed, 31 Oct 2018 12:31:07 +0100 Subject: [PATCH 2/7] Make list of ignored peer dep warnings a yarnrc config. --- accepted/0000-deal-with-peer-dependencies.md | 310 ++++++++++--------- 1 file changed, 156 insertions(+), 154 deletions(-) diff --git a/accepted/0000-deal-with-peer-dependencies.md b/accepted/0000-deal-with-peer-dependencies.md index 9606d8e..8fd90b3 100644 --- a/accepted/0000-deal-with-peer-dependencies.md +++ b/accepted/0000-deal-with-peer-dependencies.md @@ -1,154 +1,156 @@ -- Start Date: 2018-09-28 -- RFC PR: (leave this empty) -- Yarn Issue: (leave this empty) - -# Summary - -Allow a developer to deal with peer dependency warnings, without the need to -install all dependencies, thus reducing warning fatigue and drawing attention to -new warnings. - -# Motivation - -[Peer dependencies][peer-deps] are a useful way of informing the developer that -a dependency relies on the host project to provide a transitive dependency -required by the dependency, without imposing that transitive dependency onto the -project’s dependency graph on its own. - -However, there are legitimate cases when the developer may want to ignore a peer -dependency warning. Such as the host not being compatible with the dependency, -the version constraint being too restrictive, or otherwise not being interested -in the dependency. - -While treating warnings as errors is often useful, warnings are specifically not -errors, because they could possibly be ignored after judgment by the developer. - -Currently, the inability to deal with peer dependency warnings, other than -installing the required package/version, leads to warning fatigue, will be -ignored, and will introduce bugs. - -_Regarding a version constraint being too restrictive, this usually comes up -when the transitive dependency’s version is lower than v1.0.0 and the dependency -has a version requirement on it using a semantic-version operator such as `^`. -E.g. `graphql@0.13.0` will not satisfy a requirement like `graphql@^0.12.0`._ - -# Detailed design - -Consider the following peer dependency warnings shown on `yarn install`: - -``` -warning " > @artsy/palette@2.18.0" has unmet peer dependency "react-native@^0.55.4". -warning "@artsy/palette > styled-reset@1.4.0" has incorrect peer dependency "styled-components@>=4.0.0". -``` - -When the developer has determined that they do not want or cannot install these -right now, they add these to the `ignoredPeerDependencies` list of their -`package.json` manifest. - -For instance, as the project the developer is working on is a web project, they -want to always ignore a warning about the `react-native` peer dependency of -`@artsy/palette`: - -```json -{ - "ignoredPeerDependencies": { - "@artsy/palette": ["react-native@*"] - } -} -``` - -The `styled-components` dependency of the transitive `styled-reset` dependency, -however, is one that the developer cannot update to right now and so, _after_ -having verified that the version that they can use works for their use-case, the -developer adds this warning to the ignore list: - -```json -{ - "ignoredPeerDependencies": { - "styled-reset": ["styled-components@>=4.0.0"] - } -} -``` - -Afterwards, running `yarn install` shows no more warnings, until either: - -* any dependency introduces a _new_ peer dependency - - ``` - warning " > react-tracking@5.3.0" has unmet peer dependency "babel-runtime@^6.20.0". - ``` - -* _another_ dependency relies on `react-native` - - ``` - warning " > @artsy/emission@3.42.0" has unmet peer dependency "react-native@^0.55.4". - ``` - -* the `styled-reset` dependency changes its requirement on `styled-components` - - ``` - warning "@artsy/palette > styled-reset@1.5.0" has an updated ignored peer dependency "styled-components@>=3.9.0". - ``` - -In each of these cases, the developer’s attention should be drawn to the warning -so they can deal with it at the time they have the right context. - -An additional warning could be shown when a listed (transitive) dependency no -longer exists in the dependency graph. - -``` -warning "styled-reset" with ignored peer dependencies is no longer depended on. -``` - -Finally, at any given time all warnings should be visible by passing the -`--verbose` flag to `yarn install`. - -# How We Teach This - -This change is only additive and does not change any existing behavior of Yarn -or how developers are supposed to interact with it. The new terminology clearly -communicates the intent and only introduces a negated version of existing -terminology. - -Its communication should follow that of Yarn’s -[`resolutions` feature][yarn-resolutions], in that it is an extra way that a -developer can gain control over their dependency graph and ensure its stability. - -Links to its documentation could be added to the existing documentation on [peer -dependencies][yarn-peer-deps], but otherwise does not need to be altered. - -# Drawbacks - -It’s one more configuration that needs to be documented and understood. - -An alternative that does not introduce a new `package.json` key could be -preferable, but that does not appear to be feasible while maintaining -compatibility with npm. - -# Alternatives - -Introducing a new version requirement negation operator, for example: - -```json -{ - "peerDependencies": { - "styled-reset": "styled-components@!4.0.0" - } -} -``` - -However, this breaks compatibility with npm and, more importantly, doesn’t allow -for multiple peer dependencies of the (transitive) dependency to be ignored. - -# Unresolved questions - -I don’t feel strongly about the configuration, except that I dislike more -configuration. Any suggestions that could lead to the same outcome with less -new configuration would be greatly preferred. - -Perhaps the alternative design listed above could piggy-back on the -[`resolutions` key][yarn-resolutions]? - -[npm-peer-deps]: https://docs.npmjs.com/files/package.json#peerdependencies -[yarn-peer-deps]: https://yarnpkg.com/lang/en/docs/dependency-types/#toc-peerdependencies -[yarn-resolutions]: https://yarnpkg.com/lang/en/docs/selective-version-resolutions/ \ No newline at end of file +- Start Date: 2018-09-28 +- RFC PR: (leave this empty) +- Yarn Issue: (leave this empty) + +# Summary + +Allow a project author to deal with peer dependency warnings, without the need +to install all dependencies, thus reducing warning fatigue and drawing attention +to new warnings. + +_Throughout the document the term ‘developer’ will be used to indicate a project +author, i.e. the developer on a project that _consumes_ libraries vs a developer +of a library._ + +# Motivation + +[Peer dependencies][peer-deps] are a useful way of informing the developer that +a dependency relies on the host project to provide a transitive dependency +required by the dependency, without imposing that transitive dependency onto the +project’s dependency graph on its own. + +However, there are legitimate cases when the developer may want to ignore a peer +dependency warning. Such as the host not being compatible with the dependency, +the version constraint being too restrictive, or otherwise not being interested +in the dependency. + +While treating warnings as errors is often useful, warnings are specifically not +errors, because they could possibly be ignored after judgment by the developer. + +Currently, the inability to deal with peer dependency warnings, other than +installing the required package/version, leads to warning fatigue, will be +ignored, and will introduce bugs. + +_Regarding a version constraint being too restrictive, this usually comes up +when the transitive dependency’s version is lower than v1.0.0 and the dependency +has a version requirement on it using a semantic-version operator such as `^`. +E.g. `graphql@0.13.0` will not satisfy a requirement like `graphql@^0.12.0`._ + +# Detailed design + +Consider the following peer dependency warnings shown on `yarn install`: + +``` +warning " > @artsy/palette@2.18.0" has unmet peer dependency "react-native@^0.55.4". +warning "@artsy/palette > styled-reset@1.4.0" has incorrect peer dependency "styled-components@>=4.0.0". +``` + +When the developer has determined that they do not want or cannot install these +right now, they add these to the `ignore-warnings > missing-peer-dependency` +list of the _project’s_ `.yarnrc` configuration file. + +For instance, as the project the developer is working on is a web project, they +want to always ignore a warning about the `react-native` peer dependency of +`@artsy/palette`: + +```yaml +ignore-warnings: + missing-peer-dependency: + - name: "react-native" + expected-by: "@artsy/palette" + version: "*" +} +``` + +The `styled-components` dependency of the transitive `styled-reset` dependency, +however, is one that the developer cannot update to right now and so, _after_ +having verified that the version that they can use works for their use-case, the +developer adds this warning to the ignore list: + +```yaml +ignore-warnings: + missing-peer-dependency: + - name: "styled-components" + expected-by: "styled-reset" + version: ">=4.0.0" +} +``` + +Afterwards, running `yarn install` shows no more warnings, until either: + +* any dependency introduces a _new_ peer dependency + + ``` + warning " > react-tracking@5.3.0" has unmet peer dependency "babel-runtime@^6.20.0". + ``` + +* _another_ dependency relies on `react-native` + + ``` + warning " > @artsy/emission@3.42.0" has unmet peer dependency "react-native@^0.55.4". + ``` + +* the `styled-reset` dependency changes its requirement on `styled-components` + + ``` + warning "@artsy/palette > styled-reset@1.5.0" has an updated ignored peer dependency "styled-components@>=3.9.0". + ``` + +In each of these cases, the developer’s attention should be drawn to the warning +so they can deal with it at the time they have the right context. + +An additional warning could be shown when a listed (transitive) dependency no +longer exists in the dependency graph. + +``` +warning "styled-reset" with ignored peer dependencies is no longer depended on. +``` + +Finally, at any given time all warnings should be visible by passing the +`--verbose` flag to `yarn install`. + +# How We Teach This + +This change is only additive and does not change any existing behavior of Yarn +or how developers are supposed to interact with it. The new terminology clearly +communicates the intent and only introduces a negated version of existing +terminology. + +Besides an entry being added to the [the `.yarnrc` documentation][yarnrc-docs], +communication should also inlcude a blog post that explains the feature in a bit +more detail and stresses how this is a way for a developer to gain control over +their project’s dependency graph and ensure its stability. + +Links to its documentation could be added to the existing documentation on [peer +dependencies][yarn-peer-deps], but otherwise does not need to be altered. + +# Drawbacks + +It’s one more configuration that needs to be documented and understood. On the +other hand, it is likely that the `ignore-warnings` project configuration will +be expanded upon in the future, such as ignoring a missing license key in a +dependency’s `package.json` file, and as such will be less of a niche +configuration. + +# Alternatives + +A previous itertation of this RFC suggested the addition of a key to the +`package.json` file instead, but moving this out to the `.yarnrc` file made it +clear that this is a project level concern, not a library one. + +```json +{ + "ignoredPeerDependencies": { + "@artsy/palette": ["react-native@*"] + } +} +``` + +# Unresolved questions + +The suggested format for the `ignore-warnings` list of the `.yarnrc` is YAML, +yet the existing configuration doesn’t appear to be strictly YAML. + +[npm-peer-deps]: https://docs.npmjs.com/files/package.json#peerdependencies +[yarn-peer-deps]: https://yarnpkg.com/lang/en/docs/dependency-types/#toc-peerdependencies +[yarnrc-docs]: https://yarnpkg.com/lang/en/docs/yarnrc/ From 11912266df840aad07a5d2a8fde64360782b9e0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eloy=20Dur=C3=A1n?= Date: Wed, 31 Oct 2018 12:41:33 +0100 Subject: [PATCH 3/7] Add treat-warnings-as-errors configuration. --- accepted/0000-deal-with-peer-dependencies.md | 27 +++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/accepted/0000-deal-with-peer-dependencies.md b/accepted/0000-deal-with-peer-dependencies.md index 8fd90b3..785e73f 100644 --- a/accepted/0000-deal-with-peer-dependencies.md +++ b/accepted/0000-deal-with-peer-dependencies.md @@ -76,6 +76,8 @@ ignore-warnings: } ``` +---- + Afterwards, running `yarn install` shows no more warnings, until either: * any dependency introduces a _new_ peer dependency @@ -109,6 +111,24 @@ warning "styled-reset" with ignored peer dependencies is no longer depended on. Finally, at any given time all warnings should be visible by passing the `--verbose` flag to `yarn install`. +---- + +Additionally, as is custom with build systems that allow control over how to +treat warnings, an option can be introduced to treat warnings as errors. This +means that rather than developers on a project will be unable to accidentally +introduce new peer warnings, as installation will fail and they are forced to +treat the new failure by either satisfying the peer dependency or making the +determination that it should be ignore. + +This should take the form of an additional `.yarnrc` entry to allow persistence +of this project into SCM and distribution across all developers on the project. + +```yaml +treat-warnings-as-errors true +ignore-warnings: + # ... +``` + # How We Teach This This change is only additive and does not change any existing behavior of Yarn @@ -116,7 +136,7 @@ or how developers are supposed to interact with it. The new terminology clearly communicates the intent and only introduces a negated version of existing terminology. -Besides an entry being added to the [the `.yarnrc` documentation][yarnrc-docs], +Besides entries being added to the [the `.yarnrc` documentation][yarnrc-docs], communication should also inlcude a blog post that explains the feature in a bit more detail and stresses how this is a way for a developer to gain control over their project’s dependency graph and ensure its stability. @@ -151,6 +171,11 @@ clear that this is a project level concern, not a library one. The suggested format for the `ignore-warnings` list of the `.yarnrc` is YAML, yet the existing configuration doesn’t appear to be strictly YAML. +Should the `treat-warnings-as-errors` option also be exposed in the CLI? It +seems like when somebody wants to one-off fix warnings they can just look at the +output, whereas the point of this configuration is more to surface new warnings +in a continuous form. + [npm-peer-deps]: https://docs.npmjs.com/files/package.json#peerdependencies [yarn-peer-deps]: https://yarnpkg.com/lang/en/docs/dependency-types/#toc-peerdependencies [yarnrc-docs]: https://yarnpkg.com/lang/en/docs/yarnrc/ From 57c045515d3d64c29505c19d96c2f613855cbb38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eloy=20Dur=C3=A1n?= Date: Wed, 31 Oct 2018 12:46:21 +0100 Subject: [PATCH 4/7] Add unresolved question about CLI addition for adding ignored warning. --- accepted/0000-deal-with-peer-dependencies.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/accepted/0000-deal-with-peer-dependencies.md b/accepted/0000-deal-with-peer-dependencies.md index 785e73f..598b763 100644 --- a/accepted/0000-deal-with-peer-dependencies.md +++ b/accepted/0000-deal-with-peer-dependencies.md @@ -171,6 +171,18 @@ clear that this is a project level concern, not a library one. The suggested format for the `ignore-warnings` list of the `.yarnrc` is YAML, yet the existing configuration doesn’t appear to be strictly YAML. +---- + +As this new configuration deviating from the previous simple format, per the +above, does the ability to add entries to the `ignore-warnings` list need to be +exposed in the CLI, or even an interactive prompt, to ease working with it? + +``` +$ yarn config ignore-warning [--name --expected-by --version ] +``` + +---- + Should the `treat-warnings-as-errors` option also be exposed in the CLI? It seems like when somebody wants to one-off fix warnings they can just look at the output, whereas the point of this configuration is more to surface new warnings From d45ca87c122fab5cf18c33ab52f47c821d0cc7bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eloy=20Dur=C3=A1n?= Date: Wed, 31 Oct 2018 12:48:52 +0100 Subject: [PATCH 5/7] Minor fixes. --- accepted/0000-deal-with-peer-dependencies.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/accepted/0000-deal-with-peer-dependencies.md b/accepted/0000-deal-with-peer-dependencies.md index 598b763..d2daec4 100644 --- a/accepted/0000-deal-with-peer-dependencies.md +++ b/accepted/0000-deal-with-peer-dependencies.md @@ -59,7 +59,6 @@ ignore-warnings: - name: "react-native" expected-by: "@artsy/palette" version: "*" -} ``` The `styled-components` dependency of the transitive `styled-reset` dependency, @@ -73,7 +72,6 @@ ignore-warnings: - name: "styled-components" expected-by: "styled-reset" version: ">=4.0.0" -} ``` ---- @@ -115,10 +113,10 @@ Finally, at any given time all warnings should be visible by passing the Additionally, as is custom with build systems that allow control over how to treat warnings, an option can be introduced to treat warnings as errors. This -means that rather than developers on a project will be unable to accidentally -introduce new peer warnings, as installation will fail and they are forced to -treat the new failure by either satisfying the peer dependency or making the -determination that it should be ignore. +means that developers on a project will be unable to accidentally introduce new +peer warnings, as installation will fail and they are forced to treat the new +failure by either satisfying the peer dependency or making the determination +that it should be ignore. This should take the form of an additional `.yarnrc` entry to allow persistence of this project into SCM and distribution across all developers on the project. From 3573dfec53407ac86c65545af5bf818d01ae2545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eloy=20Dur=C3=A1n?= Date: Wed, 31 Oct 2018 13:07:26 +0100 Subject: [PATCH 6/7] Tiny cleanup. --- accepted/0000-deal-with-peer-dependencies.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accepted/0000-deal-with-peer-dependencies.md b/accepted/0000-deal-with-peer-dependencies.md index d2daec4..6e13e94 100644 --- a/accepted/0000-deal-with-peer-dependencies.md +++ b/accepted/0000-deal-with-peer-dependencies.md @@ -171,7 +171,7 @@ yet the existing configuration doesn’t appear to be strictly YAML. ---- -As this new configuration deviating from the previous simple format, per the +As this new configuration is deviating from the previous simple format, per the above, does the ability to add entries to the `ignore-warnings` list need to be exposed in the CLI, or even an interactive prompt, to ease working with it? From a6dd597aff777fa5a6d7e119718abad083ced366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Duarte=20Cunha=20Le=C3=A3o?= Date: Fri, 21 Dec 2018 20:04:00 +0100 Subject: [PATCH 7/7] Update accepted/0000-deal-with-peer-dependencies.md Co-Authored-By: alloy --- accepted/0000-deal-with-peer-dependencies.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accepted/0000-deal-with-peer-dependencies.md b/accepted/0000-deal-with-peer-dependencies.md index 6e13e94..8692a7d 100644 --- a/accepted/0000-deal-with-peer-dependencies.md +++ b/accepted/0000-deal-with-peer-dependencies.md @@ -152,7 +152,7 @@ configuration. # Alternatives -A previous itertation of this RFC suggested the addition of a key to the +A previous iteration of this RFC suggested the addition of a key to the `package.json` file instead, but moving this out to the `.yarnrc` file made it clear that this is a project level concern, not a library one.