Skip to content
This repository has been archived by the owner on Nov 19, 2022. It is now read-only.

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

Merged
merged 1 commit into from
May 4, 2020

Conversation

oxc
Copy link
Contributor

@oxc oxc commented May 4, 2020

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.

kotlin-wrappers now exports all its npm dependencies as api.
Export @material-ui dependencies as well, and export all kotlin
dependencies that are part of the libraries' public API.
Remove superfluous dependency declarations where applicable.
@subroh0508
Copy link
Owner

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
Copy link
Owner

subroh0508 commented May 4, 2020

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
Copy link
Contributor Author

oxc commented May 4, 2020

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
Copy link
Contributor Author

oxc commented May 4, 2020

See yarn issue yarnpkg/yarn#4743

@subroh0508
Copy link
Owner

subroh0508 commented May 4, 2020

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
Copy link
Owner

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
Copy link
Contributor Author

oxc commented May 4, 2020

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
Copy link
Owner

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.

@subroh0508 subroh0508 merged commit 57f7c14 into subroh0508:master May 4, 2020
subroh0508 added a commit that referenced this pull request Sep 18, 2021
[update]Update kotlin-wrappers -> pre.104
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants