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

🪟 🔧 Add additional eslint rules #17348

Merged
merged 6 commits into from
Oct 3, 2022
Merged

🪟 🔧 Add additional eslint rules #17348

merged 6 commits into from
Oct 3, 2022

Conversation

timroes
Copy link
Collaborator

@timroes timroes commented Sep 28, 2022

What

This adds two rules improving on TypeScript code:

How

I needed to move the .eslintrc file to .eslintrc.js since I required access to __dirname for that path to work properly no matter if you run lint via the command line or via VSCode.

Since those new rules require eslint to properly parse our project to have all type information, they can only apply to files in src/ (which are part of out typescript project), thus they went into a separate overwrites.

@timroes timroes requested a review from a team as a code owner September 28, 2022 19:00
@github-actions github-actions bot added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Sep 28, 2022
@teallarson
Copy link
Contributor

The rules look good but looks like there are some build things with the e2e tests failing.

@dizel852
Copy link
Contributor

A great addition to existing rules! 👍
But I notice a few things:

  1. E2E fails because of

image

  1. The terminal is full of "Warning: An update to Formik inside a test was not wrapped in act(...)" warnings after the tests run. But tests are green.

image

@krishnaglick
Copy link
Contributor

The changes from @typescript-eslint/await-thenable may have been partially done in error. It could be functions that were poorly typed, but async, are no longer being awaited.

@timroes
Copy link
Collaborator Author

timroes commented Sep 29, 2022

The changes from @typescript-eslint/await-thenable may have been partially done in error. It could be functions that were poorly typed, but async, are no longer being awaited.

Could you please specify which removed await specifically you referring too, that where done in error?

@krishnaglick
Copy link
Contributor

The changes from @typescript-eslint/await-thenable may have been partially done in error. It could be functions that were poorly typed, but async, are no longer being awaited.

Could you please specify which removed await specifically you referring too, that where done in error?

My statement was based on us having failing tests, so I hedged that was the reason. If tests are passing then it seems like we're good. 👍

@timroes timroes merged commit 2c71086 into master Oct 3, 2022
@timroes timroes deleted the tim/eslint-rules branch October 3, 2022 15:43
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
* Add additional eslint rules

* Add comment

* Restore async await act

* Fix broken gradle reference
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants