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.

Sign up for GitHub

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

[core] Fix some peer dependency warnings #14572

Merged
merged 1 commit into from
Feb 18, 2019
Merged

[core] Fix some peer dependency warnings #14572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Member Author

@eps1lon eps1lon Feb 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Was transitive dependency due to:

  • recharts
  • react-draggable
  • react-select
  • react-virtualized

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

@oliviertassinari oliviertassinari Feb 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

What about waiting for iamhosseindhv/notistack#72 resolution?

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

"clean-css": "^4.1.11",
"clipboard-copy": "^2.0.0",
"cross-env": "^5.1.1",
Expand Down Expand Up @@ -150,6 +151,7 @@
"nyc": "^13.0.0",
"postcss": "^7.0.0",
"prettier": "^1.8.2",
"prop-types": "^15.6.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

Well spotted! We should fix that.

"puppeteer": "^1.5.0",
"raw-loader": "^1.0.0",
"react": "^16.8.0",
Expand Down Expand Up @@ -190,6 +192,7 @@
"url-loader": "^1.0.1",
"vrtest": "^0.2.0",
"webfontloader": "^1.6.28",
"webpack": "^4.28.4",
Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@eps1lon eps1lon Feb 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

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

Is a direct dependency of [...]

or

Is a transitive dependency due to [...]

"webpack-bundle-analyzer": "^3.0.0",
"webpack-cli": "^3.2.3"
},
Expand Down
1 change: 1 addition & 0 deletions packages/material-ui-benchmark/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"@emotion/core": "^10.0.0",
"@emotion/styled": "^10.0.0",
"benchmark": "^2.1.4",
"emotion": "^10.0.7",
Copy link
Member Author

@eps1lon eps1lon Feb 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@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.

Copy link
Member

Choose a reason for hiding this comment

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

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

"emotion-server": "^10.0.6",
"nodemod": "^1.5.19",
"styled-system": "^3.1.11"
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui-docs/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"test": "exit 0"
},
"peerDependencies": {
"@material-ui/core": "^3.0.0",
"@material-ui/core": "^3.0.0 || ^4.0.0-alpha",
"react": "^16.8.0",
"react-dom": "^16.8.0"
},
Expand Down
20 changes: 19 additions & 1 deletion yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3544,7 +3544,7 @@ class-utils@^0.3.5:
isobject "^3.0.0"
static-extend "^0.1.1"

classnames@^2.2.3, classnames@^2.2.5, classnames@~2.2.5:
classnames@^2.2.3, classnames@^2.2.5, classnames@^2.2.6, classnames@~2.2.5:
version "2.2.6"
resolved "https://registry.yarnpkg.com/classnames/-/classnames-2.2.6.tgz#43935bffdd291f326dad0a205309b38d00f650ce"
integrity sha512-JR/iSQOSt+LQIWwrwEzJ9uk0xfN3mTVYMwt1Ir5mUcSN6pU+V4zQFFaJsclJbPuAUQH+yfWef6tm7l1quW3C8Q==
Expand Down Expand Up @@ -4050,6 +4050,16 @@ [email protected]:
multipipe "^1.0.2"
through "^2.3.8"

create-emotion@^10.0.7:
version "10.0.7"
resolved "https://registry.yarnpkg.com/create-emotion/-/create-emotion-10.0.7.tgz#608d69c550e2f57827383762d416a9ddf75d8bed"
integrity sha512-2T6vvvh7XN/MvI3far2SXeQ5s7wti/dae6jKuHxkK4IA1IKdYocKTujZ+r56azZ8fguq3Qj4ua1AJ2vHCq7VTg==
dependencies:
"@emotion/cache" "^10.0.7"
"@emotion/serialize" "^0.11.4"
"@emotion/sheet" "0.9.2"
"@emotion/utils" "0.11.1"

create-emotion@^9.2.12:
version "9.2.12"
resolved "https://registry.yarnpkg.com/create-emotion/-/create-emotion-9.2.12.tgz#0fc8e7f92c4f8bb924b0fef6781f66b1d07cb26f"
Expand Down Expand Up @@ -5033,6 +5043,14 @@ emotion-theming@^10.0.5:
hoist-non-react-statics "^2.3.1"
object-assign "^4.1.1"

emotion@^10.0.7:
version "10.0.7"
resolved "https://registry.yarnpkg.com/emotion/-/emotion-10.0.7.tgz#74ea432c7004c2bd5452d017d52e307a7a31a835"
integrity sha512-k1gGBoel9rlHvHIUVHyk4iJPsRaDsrpr7vKivOmdJAH2Va+smxIYvsjjzXnxTeqJt5IwcVBareuoAJMxeShG/w==
dependencies:
babel-plugin-emotion "^10.0.7"
create-emotion "^10.0.7"

emotion@^9.1.2:
version "9.2.12"
resolved "https://registry.yarnpkg.com/emotion/-/emotion-9.2.12.tgz#53925aaa005614e65c6e43db8243c843574d1ea9"
Expand Down