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

feat: eslint and related upgrades #60

Merged
merged 7 commits into from
May 20, 2024
Merged

Conversation

ashwinibm
Copy link
Contributor

@ashwinibm ashwinibm commented May 10, 2024

Summary

We had to upgrade a couple of packages (for this) on the web frontend.
This resulted in some linting issues that are a result of de-sync on the eslint-plugin-instawork repo on the eslint versioning mismatches. The eslint-plugin-instawork packages have been out of date for a while.
Here, we've upgraded a bunch of packages in a way that is compatible for both web and mobile repos.

Notes:

  • Also had to patch eslint-plugin-flowtype. See this issue.
  • Need to have Node > 18.18.0

Screenshots

  1. $ yarn test on eslint-plugin-instawork repo
Screenshot 2024-05-10 at 2 43 38 PM
  1. $ yarn on instawork/ali/microfe/testing-enhancements-v2 branch
Screenshot 2024-05-10 at 3 01 02 PM Screenshot 2024-05-10 at 3 03 24 PM
  1. $ yarn test:lint on applicant-app
Screenshot 2024-05-10 at 3 23 24 PM
  • Had to turn off instawork/component-logic-loc which we had added for web since the rule is crashing due to some issue in the flow syntax.
  • There are some lint errors with the new upgrades. I'll fix those separately and go through the App release cycle. A lot of them are class-methods-use-this errors that we can probably turn off for the time being.

@ashwinibm ashwinibm marked this pull request as ready for review May 10, 2024 10:22
steps:
- checkout
- run: yarn --prod # install only prod dependencies first to trigger postinstall patch-package script
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe the reason you have to resort to this hack is because of this:
ds300/patch-package#84 (comment)

We should make sure we undo this as we work on 2.x branch.

.eslintrc Outdated
Comment on lines 8 to 14
// "parser": "@babel/eslint-parser",
// "parserOptions": {
// "requireConfigFile": true, // This can be true if you have a Babel configuration file
// "babelOptions": {
// "presets": ["@babel/preset-env", "jest"] // Adapt or add other Babel presets as needed
// }
// },
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be removed?

@ashwinibm ashwinibm merged commit 5c96435 into master May 20, 2024
1 check passed
@ashwinibm ashwinibm deleted the ashwinibm/feat-upgrades branch May 20, 2024 08:22
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.

3 participants