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.

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

chore(deps): Fix transitive dependencies #5796

Merged
merged 38 commits into from
Sep 24, 2024
Merged

chore(deps): Fix transitive dependencies #5796

merged 38 commits into from
Sep 24, 2024

Conversation

esauerbo
Copy link
Contributor

@esauerbo esauerbo commented Sep 12, 2024

Description of changes

  • Add eslint import/no-extraneous-dependencies rule which will error if an import is used that isn't included in the module's package.json
  • Ignore testing files so only dependencies that will be shipped to customer code are checked
  • Update dependencies for packages where transitive dependencies were being included

Issue #, if available

Description of how you validated changes

Manually went through each package and removed a dependency and verified that eslint threw an error

Checklist

  • Have read the Pull Request Guidelines
  • PR description included
  • yarn test passes and tests are updated/added
  • PR title and commit messages follow conventional commit syntax
  • If this change should result in a version bump, changeset added (This can be done after creating the PR.) This does not apply to changes made to docs, e2e, examples, or other private packages.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@esauerbo esauerbo requested a review from a team as a code owner September 12, 2024 00:05
Copy link

changeset-bot bot commented Sep 12, 2024

🦋 Changeset detected

Latest commit: 9dc445b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages
Name Type
@aws-amplify/ui-react-ai Patch
@aws-amplify/ui-react-core-notifications Patch
@aws-amplify/ui-react-core Patch
@aws-amplify/ui-react-geo Patch
@aws-amplify/ui-react-liveness Patch
@aws-amplify/ui-react-native Patch
@aws-amplify/ui-react-notifications Patch
@aws-amplify/ui-react-storage Patch
@aws-amplify/ui-react Patch
@aws-amplify/ui Patch
@aws-amplify/ui-vue Patch
@aws-amplify/ui-angular Patch
@aws-amplify/ui-react-auth Patch
@aws-amplify/ui-react-core-auth Patch
@aws-amplify/ui-react-native-auth Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@calebpollman calebpollman left a comment

Choose a reason for hiding this comment

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

Let's add the linting rule to the base eslint config that the rest of the packages extend from and configure ignore patterns for devDeps in test files there

@esauerbo esauerbo changed the title WIP: Fix transitive dependencies chore(deps): Fix transitive dependencies Sep 12, 2024
packages/react/package.json Outdated Show resolved Hide resolved
@@ -33,10 +33,12 @@
"typecheck": "tsc --noEmit"
},
"dependencies": {
"@aws-amplify/core": "6.4.0",
Copy link
Contributor Author

@esauerbo esauerbo Sep 13, 2024

Choose a reason for hiding this comment

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

Wasn't sure what version to use, went with 6.4.0 because it's what would be brought in by the latest version of the "aws-amplify": "^6.3.2" peer dep

jordanvn
jordanvn previously approved these changes Sep 19, 2024
Copy link
Member

@jordanvn jordanvn left a comment

Choose a reason for hiding this comment

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

This looks good to me. I believe I understand the change, but I would still like to see @calebpollman's remaining question answered

reesscot
reesscot previously approved these changes Sep 20, 2024
@@ -19,6 +20,12 @@ module.exports = {
},
ignorePatterns: ['dist'],
rules: {
'import/no-extraneous-dependencies': [
Copy link
Member

Choose a reason for hiding this comment

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

Nce catch!

calebpollman
calebpollman previously approved these changes Sep 20, 2024
Copy link
Member

@calebpollman calebpollman left a comment

Choose a reason for hiding this comment

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

Smoke tested locally and everything is working as expected, thanks for the hard work here @esauerbo

@esauerbo esauerbo merged commit bf9dbc3 into main Sep 24, 2024
34 checks passed
@esauerbo esauerbo deleted the fix-transitive-deps branch September 24, 2024 16:24
@github-actions github-actions bot mentioned this pull request Sep 24, 2024
esauerbo added a commit that referenced this pull request Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants