[core] Fix some peer dependency warnings #14572

eps1lon · 2019-02-18T12:53:27Z

Reduces noise from peer dependency warnings that were not problematic since the referenced packages were transitive dependencies.

These warnings are removed:

warning " > babel-loader@8.0.5" has unmet peer dependency "webpack@>=2".
warning " > css-loader@2.1.0" has unmet peer dependency "webpack@^4.0.0".
warning " > eslint-import-resolver-webpack@0.11.0" has unmet peer dependency "webpack@>=1.11.0".
warning "karma-webpack > webpack-dev-middleware@2.0.6" has unmet peer dependency "webpack@^2.2.0 || ^3.0.0 || ^4.0.0-alpha || ^4.0.0-beta || ^4.0.0".
warning " > material-ui-pickers@2.2.1" has unmet peer dependency "prop-types@^15.6.0".
warning " > notistack@0.4.2" has unmet peer dependency "prop-types@^15.6.2".
warning " > notistack@0.4.2" has unmet peer dependency "classnames@^2.2.6".
warning " > raw-loader@1.0.0" has unmet peer dependency "webpack@^4.3.0".
warning " > react-final-form@4.0.2" has unmet peer dependency "prop-types@^15.6.0".
warning " > react-frame-component@4.0.2" has unmet peer dependency "prop-types@^15.5.9".
warning " > url-loader@1.1.2" has unmet peer dependency "webpack@^3.0.0 || ^4.0.0".
warning " > webpack-cli@3.2.3" has unmet peer dependency "webpack@4.x.x".
warning " > @material-ui/docs@4.0.0-alpha.0" has incorrect peer dependency "@material-ui/core@^3.0.0".
warning "workspace-aggregator-de8304da-e48d-4e15-b331-ca3b44f22ad6 > @material-ui/benchmark > emotion-server@10.0.7" has unmet peer dependency "emotion@^10.0.0".

eps1lon · 2019-02-18T12:56:01Z

package.json

@@ -150,6 +151,7 @@
    "nyc": "^13.0.0",
    "postcss": "^7.0.0",
    "prettier": "^1.8.2",
+    "prop-types": "^15.6.0",


prop-types shouldn't be a peer dependency as per recommendation anyway: https://github.com/facebook/prop-types#how-to-depend-on-this-package

This is a workaround for libraries not following this recommendation (e.g. material-ui-pickers)
/cc @dmtrKovalenko

Well spotted! We should fix that.

eps1lon · 2019-02-18T12:56:34Z

package.json

@@ -190,6 +192,7 @@
    "url-loader": "^1.0.1",
    "vrtest": "^0.2.0",
    "webfontloader": "^1.6.28",
+    "webpack": "^4.28.4",


Was a transitive dependency of size-limit and rollup-plugin-size-snapshot

It's not true.
https://github.com/TrySound/rollup-plugin-size-snapshot/blob/master/package.json#L72

Right, I mispoke. What I wanted to say was:

Is a direct dependency of [...]

or

Is a transitive dependency due to [...]

eps1lon · 2019-02-18T12:57:25Z

packages/material-ui-benchmark/package.json

@@ -28,7 +28,6 @@
    "@emotion/core": "^10.0.0",
    "@emotion/styled": "^10.0.0",
    "benchmark": "^2.1.4",
-    "emotion-server": "^10.0.6",


This is a migration/compat package only: https://emotion.sh/docs/ssr Not sure if a benchmark for those kind of packages is usefull? @oliviertassinari

Wait, emotion-server package is legacy? How are we supposed to handle interoperability specificity?
Let's say I have a core component X that uses emotion but I also have a component Y that extends the style of X using styled-components. If I understand it correctly, styled-components with inject the CSS in the head when emotion will inject the CSS in the body. The SSR output specificity will be wrong (inverted). cc @tkh44.

Not legacy. Just "compatibility and migration purposes". I don't think that applies to the benchmark in question. It would however apply to the use case you mentioned I think.

I believe the emotion-server package solves these concerns: whatwg/html#1605 (comment)

I don't understand why you derail this to emotion concerns? The package specifically states that this is a package for compatibility concerns. If you think that we need to benchmark this solution then I have no objections. Let's fix the warning and keep the benchmark then.

Criticizing emotion architectural choices here is misplaced. If you have problems with emotions architectural choices you can open issues in their repo.

eps1lon · 2019-02-18T12:59:28Z

package.json

@@ -97,6 +97,7 @@
    "babel-plugin-transform-react-remove-prop-types": "^0.4.21",
    "babel-plugin-unwrap-createStyles": "link:packages/babel-plugin-unwrap-createStyles",
    "chai": "^4.1.2",
+    "classnames": "^2.2.6",


Was transitive dependency due to:

recharts

react-draggable

react-select

react-virtualized

It's a peer dependency of these packages? Why do we need to install it?

No it's a peer dependency of notisstack. notisstack would fail if we would remove the packages I mentioned.

And it is already installed. We're not adding any new packages to the bundle in this PR

I think that the classname package of notistack should make it a dependency cc @iamhosseindhv.

What about waiting for iamhosseindhv/notistack#72 resolution?

We can always remove it once it's resolved. After all it's just an entry in the package.json.

classnames and prop-types should be dependencies of notistack. This will be published in the next version.

oliviertassinari · 2019-02-18T13:41:01Z

I'm fascinated by the number of yarn warnings we have, and yet, they are all false positive (everything works).

eps1lon · 2019-02-18T13:53:30Z

I'm fascinated by the number of yarn warnings we have, and yet, they are all false positive.

You can look at them like a built-in import/no-extraneous-dependencies lint rule. All of the warnings I fixed in this PR are indeed defects. They're only not actual bugs because of transitive dependencies. As soon as we would remove unrelated packages that create these transitive dependencies we would face actual bugs in our code. Therefore they're not false positives unless you think that import/no-extraneous-dependencies is not a useful rule.

Some are also just underspecified peer dependencies. The jss plugins for instance all work with the alpha for our usage. But maybe the full test suite of these plugins is not passing for the jss alpha which is why the peer is pinned to a stable version.

oliviertassinari · 2019-02-18T13:57:15Z

@eps1lon You are right, it's hiding core problems. Now, let's imagine people who are pressured by a deadline, and don't have the time to look closer what they must think about yarn warnings: it's just noise. I believe many people have learned to ignore them. We shouldn't encourage this!

eps1lon · 2019-02-18T14:11:28Z

My main issue is that you don't have a place were you can ignore those warnings (and preferably include an explanation why).

This was pretty annoying during the hooks alpha where basically every package triggered peer dependency warnings. Spotting an actual warning in 30+ warnings that do not cause harm is very hard. I would've liked to have some sort of warning whitelist where I can tell that this is fine because this alpha is stable enough to not cause any harm (or ran integration tests etc). All of this is responsibility of the dev in control of the package.json

Yarn maintainers are at least aware of this issue: yarnpkg/yarn#4064 (comment)

peer dependencies are not in a perfect spot but it's all we have so we should make the best of it. Since we have things like tree-shaking I rather have an unused package in my package.json and no warnings than a warning that I just ignore.

eps1lon · 2019-02-18T14:13:05Z

packages/material-ui-benchmark/package.json

@@ -28,6 +28,7 @@
    "@emotion/core": "^10.0.0",
    "@emotion/styled": "^10.0.0",
    "benchmark": "^2.1.4",
+    "emotion": "^10.0.7",


@oliviertassinari I think this is now a more realistic benchmark. emotion-server@10 should use emotion@10 but it was using emotion@9 before this change. emotion@9 is a direct dependency of react-select and was therefore available for emotion-server.

We should be using emotion@10 exclusively. I can fix that later on.

eps1lon added the core label Feb 18, 2019

eps1lon commented Feb 18, 2019

View reviewed changes

oliviertassinari closed this Feb 18, 2019

oliviertassinari reopened this Feb 18, 2019

[core] Fix some peer dependency warnings

819955b

eps1lon force-pushed the core/reduce-install-wrnings branch from 379709a to 819955b Compare February 18, 2019 14:00

eps1lon commented Feb 18, 2019

View reviewed changes

oliviertassinari approved these changes Feb 18, 2019

View reviewed changes

oliviertassinari added the PR: good for merge label Feb 18, 2019

oliviertassinari merged commit d2ff187 into mui:next Feb 18, 2019

eps1lon deleted the core/reduce-install-wrnings branch February 18, 2019 16:58

eps1lon mentioned this pull request Jul 18, 2019

[docs] Move dependencies/scripts from root into workspace #16640

Merged

15 tasks

zannager added the core Infrastructure work going on behind the scenes label Mar 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Fix some peer dependency warnings #14572

[core] Fix some peer dependency warnings #14572

eps1lon commented Feb 18, 2019

eps1lon Feb 18, 2019

oliviertassinari Feb 18, 2019

eps1lon Feb 18, 2019

TrySound Feb 18, 2019

eps1lon Feb 18, 2019 •

edited

Loading

eps1lon Feb 18, 2019

oliviertassinari Feb 18, 2019 •

edited

Loading

eps1lon Feb 18, 2019

oliviertassinari Feb 18, 2019 •

edited

Loading

eps1lon Feb 18, 2019

eps1lon Feb 18, 2019 •

edited

Loading

oliviertassinari Feb 18, 2019

eps1lon Feb 18, 2019

eps1lon Feb 18, 2019

oliviertassinari Feb 18, 2019 •

edited

Loading

oliviertassinari Feb 18, 2019

eps1lon Feb 18, 2019

iamhosseindhv Feb 18, 2019

oliviertassinari commented Feb 18, 2019 •

edited

Loading

eps1lon commented Feb 18, 2019

oliviertassinari commented Feb 18, 2019 •

edited

Loading

eps1lon commented Feb 18, 2019

eps1lon Feb 18, 2019 •

edited

Loading

oliviertassinari Feb 18, 2019

[core] Fix some peer dependency warnings #14572

[core] Fix some peer dependency warnings #14572

Conversation

eps1lon commented Feb 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eps1lon Feb 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Feb 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Feb 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eps1lon Feb 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Feb 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari commented Feb 18, 2019 • edited Loading

eps1lon commented Feb 18, 2019

oliviertassinari commented Feb 18, 2019 • edited Loading

eps1lon commented Feb 18, 2019

eps1lon Feb 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eps1lon Feb 18, 2019 •

edited

Loading

oliviertassinari Feb 18, 2019 •

edited

Loading

oliviertassinari Feb 18, 2019 •

edited

Loading

eps1lon Feb 18, 2019 •

edited

Loading

oliviertassinari Feb 18, 2019 •

edited

Loading

oliviertassinari commented Feb 18, 2019 •

edited

Loading

oliviertassinari commented Feb 18, 2019 •

edited

Loading

eps1lon Feb 18, 2019 •

edited

Loading