subroh0508 / kotlin-material-ui

[UNMAINTAINED] Kotlin Wrapper Library of Material-UI
MIT License
74 stars 24 forks source link

[update]Update kotlin-wrappers -> pre.104 #23

Closed oxc closed 4 years ago

oxc commented 4 years ago

Update to kotlin-wrappers pre.104. They have changed their setup to export all dependencies as api scope. This definitely makes sense for gradle-dependencies (like kotlin-react-dom -> kotlin-react), for npm it's probably fine as well.

However, it leads to some warnings in the initial kotlinNpmInstall task about missing peer dependencies. However, those dependencies are available in the end.

I have adjusted the build files to follow the kotlin-wrappers change: the @material-ui npm dependencies are now exported as api. I have updated them to the latest version.

All kotlin-wrappers libraries that are part of this library's public API are now correctly marked as api as well.

I have removed all superfluous dependency declarations where applicable.

I'm not totally happy with the warnings. Please let me know what you think about this.

subroh0508 commented 4 years ago

Does the stack trace look like this?

warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-lab > @material-ui/lab@4.0.0-alpha.51" has unmet peer dependency "@material-ui/core@^4.9.10".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-lab > @material-ui/lab@4.0.0-alpha.51" has unmet peer dependency "react@^16.8.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-lab > @material-ui/lab@4.0.0-alpha.51" has unmet peer dependency "react-dom@^16.8.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-react-dom > react-dom@16.13.1" has unmet peer dependency "react@^16.13.1".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-core > @material-ui/core@4.9.12" has unmet peer dependency "react@^16.8.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-core > @material-ui/core@4.9.12" has unmet peer dependency "react-dom@^16.8.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-lab > @material-ui/lab > @material-ui/utils@4.9.12" has unmet peer dependency "react@^16.8.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-lab > @material-ui/lab > @material-ui/utils@4.9.12" has unmet peer dependency "react-dom@^16.8.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-styled > styled-components@4.4.1" has unmet peer dependency "react@>= 16.3.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-styled > styled-components@4.4.1" has unmet peer dependency "react-dom@>= 16.3.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-core > @material-ui/core > @material-ui/styles@4.9.10" has unmet peer dependency "react@^16.8.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-core > @material-ui/core > @material-ui/styles@4.9.10" has unmet peer dependency "react-dom@^16.8.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-core > @material-ui/core > @material-ui/react-transition-group@4.3.0" has unmet peer dependency "react@>=16.6.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-core > @material-ui/core > @material-ui/react-transition-group@4.3.0" has unmet peer dependency "react-dom@>=16.6.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-core > @material-ui/core > @material-ui/system@4.9.10" has unmet peer dependency "react@^16.8.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-core > @material-ui/core > @material-ui/system@4.9.10" has unmet peer dependency "react-dom@^16.8.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-core > @material-ui/core > react-transition-group@4.3.0" has unmet peer dependency "react@>=16.6.0".
warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-material-ui-fork-core > @material-ui/core > react-transition-group@4.3.0" has unmet peer dependency "react-dom@>=16.6.0".
subroh0508 commented 4 years ago

These seem to be occurring because some NPM dependencies require an older version of React.

As you say, this is not the correct state of affairs, but since the latest version of kotlin-react strictly specifies React v16.13.1, I think it is difficult to deal with it perfectly 😭 .

If you simply want to remove the warning, there is a way to add the following to the dependency of Gradle.

api(npm("react", ">=16.x.x"))
api(npm("react-dom", ">=16.x.x"))

In my opinion, I don't think it's a big problem if it's a later version than Hooks support.

So, it would be very helpful if you could add api(npm("react", ">=16.8.0")) to the Gradle.

oxc commented 4 years ago

These seem to be occurring because some NPM dependencies require an older version of React.

I don't think this is the reason. As you can notice, there are also warnings for ^16.13.1 (which at the time of writing is 16.13.1).

warning "workspace-aggregator-09919878-e0a2-4532-813c-00efd3e33c86 > kotlin-react-dom > react-dom@16.13.1" has unmet peer dependency "react@^16.13.1".

Note that there is also a line about material-ui/core, where the same issue applies.

All of those dependency declarations from the warnings should be met by react 16.13.1.

I believe the warnings are actually a bug in yarn with workspaces, because the dependency is correctly defined and pulled in by kotlin-react, and also ends up in the yarn.lock

oxc commented 4 years ago

See yarn issue https://github.com/yarnpkg/yarn/issues/4743

subroh0508 commented 4 years ago

According to the official Node.js documentation, in v3 and later npm, the dependencies specified in peerDependencies will only be warned and will not be installed.

https://docs.npmjs.com/files/package.json#peerdependencies

That is, to resolve these warnings, you need to explicitly specify the version of React in package.json of kotlin-material-ui.

subroh0508 commented 4 years ago

Add api(npm("react", "^16.13.1")) and api(npm("react-dom", "^16.13.1")), the stack trace

warning "workspace-aggregator-ea2d5577-09f1-4180-b51b-06f04fa286a6 > kotlin-material-ui-fork-lab > @material-ui/lab@4.0.0-alpha.51" has unmet peer dependency "@material-ui/core@^4.9.10".
warning "workspace-aggregator-ea2d5577-09f1-4180-b51b-06f04fa286a6 > kotlin-styled > styled-components@4.4.1" has unmet peer dependency "react@>= 16.3.0".
warning "workspace-aggregator-ea2d5577-09f1-4180-b51b-06f04fa286a6 > kotlin-styled > styled-components@4.4.1" has unmet peer dependency "react-dom@>= 16.3.0".
oxc commented 4 years ago

These are not peerDependencies. They are hard dependencies pulled in by kotlin-react and kotlin-material-ui. Check for example build/js/packages_imported/kotlin-react/0.0.0/package.json or packages_imported/kotlin-material-ui-core/0.3.15/package.json

Kotlin-gradle-plugin includes these projects as yarn workspaces (build/js/packages_imported), Yarn hoists them up to the root project and logs false warnings about it.

Adding these dependencies explicitly might prevent the warnings, but is technically not needed, because they are transitively added already.

subroh0508 commented 4 years ago

I understand, this certainly seems to be a bug in yarn workspaces.

It seems to solve the problem if you can specify a commonly used library in the dependencies of build/js/package.json. But I don't know how to specify this used by org.jetbrains.kotlin.js.

As for this PR, I'm going to merge it for now. It would be great if you could create an issue for when someone finds a neat way to get rid of the warning.