fix(deps): fix more implicit deps and add linting #14137
Conversation
…sed-package-dependencies
| "peerDependencies": { | ||
| "react-native": ">=0.70" | ||
| }, |
There was a problem hiding this comment.
it's not strictly necessary because the @aws-amplify/react-native package has a peer dependency on "react-native": ">=0.70" but it is correct in isolation because this package uses react-native
There was a problem hiding this comment.
I don't think we have this in the web browser package either. Hesitant to use peer deps given the inconsistent behavior we sometimes see with different package managers and it gives us more places to update this when none of the rtn- packages really work in isolation anyway. Just a little scared
There was a problem hiding this comment.
For some reason I though rtn-web-browser had a peer dep but it's just a dev dep 🤦🏽 I can create a separate PR for that. If these are truly meant to be used with the @amplify-aws/react-native package then there should be no issues adding these peer deps. I haven't seen any in testing 🤞🏽
| "tslib": "^2.5.0" | ||
| }, | ||
| "peerDependencies": { |
There was a problem hiding this comment.
Could this cause issues with duplicate cores - e.g. during a MV bump this is forgotten?
There was a problem hiding this comment.
we do this in all our packages already. I dont see how this addition would be any different? 👀
There was a problem hiding this comment.
But answer the actual question, yes. If we miss updating anywhere it will cause duplicate dependencies but i think that is caught in our tests
There was a problem hiding this comment.
I'm a little concerned about adding peer deps in general given their behavior at times but it's a little extra daunting when we have two deps on core here and they're already "mismatched". Should be fine, but I'm jut wondering if we're adding some baggage needlessly.
There was a problem hiding this comment.
That's fair but it's useful for modern package managers like pnpm, so there is some benefit. I think it's better to not rely on implicit dependencies which could stop working with future versions of npm
pranavosu
dismissed stale reviews from AllanZhengYP, joon-won, and stocaaro
via
3143d35
Description of changes
Fix implicit dependencies for packages (except adapter-nextjs1, and react-native adjacent2) to better support Yarn Pnp and pnpm.
Issue #, if available
Description of how you validated changes
yarn && yarn lintChecklist
yarn testpassesChecklist for repo maintainers
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.