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(repo): migrate to eslint #12620

Closed
wants to merge 25 commits into from
Closed

Conversation

HuiSF
Copy link
Member

@HuiSF HuiSF commented Nov 22, 2023

Description of changes

  1. Use typescript 5.0.2 in the workspace!
  2. Unified tsconfig.json setup and its branching across packages
    • ts will check unit tests too to ensure we are using correct type to ensure unit tests are accurate
    • virtual paths are enabled in each package, no more ../../../module import paths
  3. Migrated to eslint and remove tslint
  4. Ran eslint --fix for each package to auto fix lint issues (approximately 80% of the changes), plus manual fixes (approximately 20% of the changes)

NOTE: This PR upgrades typescript 5.0.2 which breaks the existing jest set up, this PR will depends on #12607 to work (hence marked as Draft PR, and currently __tests__ is ignored by eslint ).

NOTE: Please DO NOT click on update branch button. Thanks

What's new after rebasing off origin/main

  1. chore(repo): update jest setup script to work with eslint and tsconfig
  2. necessary changes made to __tests__ to let the unit tests run (in the commits that are with message: "squash to the previous"...)

Issue #, if available

Description of how you validated changes

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)

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

@HuiSF HuiSF force-pushed the hui/chore/repo/migrate-eslint branch from 7d944fa to 2cf1ba2 Compare November 22, 2023 21:36
Copy link

⚠️ This PR includes manual changes to the "aws-amplify" package.json file, which can have library-wide implications.

Please ensure that this PR:

  • Does not modify "@aws-amplify/*" dependency versions, which may misalign core dependencies across the library.

A repository administrator is required to review & merge this change.

@HuiSF HuiSF force-pushed the hui/chore/repo/migrate-eslint branch from 2cf1ba2 to 3510e6e Compare November 22, 2023 22:05
.prettierrc.js Outdated Show resolved Hide resolved
packages/core/src/utils/WordArray.ts Outdated Show resolved Hide resolved
"editor.defaultFormatter": "esbenp.prettier-vscode",
"editor.detectIndentation": false,
"editor.formatOnSave": true,
"editor.insertSpaces": false,
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to use space in ALL places instead of tabs one day🙏

@HuiSF HuiSF force-pushed the hui/chore/repo/migrate-eslint branch from 3510e6e to 38f7430 Compare November 29, 2023 21:45
@HuiSF HuiSF marked this pull request as ready for review November 29, 2023 22:13
@HuiSF HuiSF requested review from a team as code owners November 29, 2023 22:13
cshfang
cshfang previously approved these changes Nov 29, 2023
package.json Outdated Show resolved Hide resolved
cshfang
cshfang previously approved these changes Nov 29, 2023
@HuiSF HuiSF force-pushed the hui/chore/repo/migrate-eslint branch from cf1496d to 54a4dd4 Compare December 21, 2023 01:52
Copy link
Contributor

@jimblanc jimblanc left a comment

Choose a reason for hiding this comment

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

Thank you for taking this on, looks good to me. Paid special attention to analytics, core, and umbrella package. Have we run the integ test suite against this to double check? Also looks like we have a minor bundle size regression that needs to be adjusted.

@HuiSF HuiSF closed this Jan 29, 2024
@HuiSF HuiSF deleted the hui/chore/repo/migrate-eslint branch May 24, 2024 00:18
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