fix(repo): Fix ESLint errors and use ESLint in lint-staged by LekoArts · Pull Request #1756 · clerk/javascript

LekoArts · 2023-09-21T08:27:48Z

Description

So this is a PR with a lot of changed files but don't worry. The changes are not heavy handed, I reserved the bigger ESLint changes for sometime later 😅

Essentially, this PR fixes the errors that were thrown with the current ESLint setup when run npm run lint at the root. Here and there I fixed some other things that I noticed, too.

Adjust .eslintignore and .gitignore to properly ignore our desired dirs
Add ESLint to our lint-staged
Rename LICENCE (which is wrong) to LICENSE
Add FORCE_COLOR=1 to Turborepo scripts because for whatever reason they don't retain color output?! (see Support colored output from original CLI vercel/turborepo#223)
Change the lint scripts to not lint the whole package but only the src folders. This speeds up the checks and encourages to not put stuff somewhere else
Didn't touch some code paths and instead added a TODO. See the commit that only contains the TODO changes: 0c74bef
Update ESLint and other deps

Important: I purposefully not enabled ESLint in our CI yet as that can be a blocker for PRs. Locally you can get around the lint-staged check with --no-verify. Long-term I want to enable ESLint in CI but I want to wait and see if we run into any problems.

Fixes JS-752

Checklist

npm test runs as expected.
npm run build runs as expected.
(If applicable) JSDoc comments have been added or updated for any package exports
(If applicable) Documentation has been updated

Type of change

Packages affected

changeset-bot · 2023-09-21T08:27:51Z

🦋 Changeset detected

Latest commit: 27a2cd5

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

This PR includes changesets to release 3 packages

Name	Type
@clerk/clerk-js	Patch
@clerk/chrome-extension	Patch
@clerk/clerk-expo	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

LekoArts

Comments to make review easier

LekoArts · 2023-09-21T08:39:25Z

+'@clerk/clerk-js': patch
+---
+
+Some minor TypeScript type fixes to internal components. Also applying some ESLint recommendations.


I've only applied the changeset here because in those files I changed some runtime code (e.g. newSize + 'px' to ${newSize}px). Just to be safe.

In the rest only the TS types changed

LekoArts · 2023-09-21T08:40:18Z

+  "*.{js,jsx,ts,tsx}": ["npx prettier --write", "npx eslint --fix"],
+  "*.{json,md,mdx}": ["npx prettier --write"]


lint-staged runs things concurrently. With this setup prettier and ESLint run after each other for js/jsx/ts/tsx files

LekoArts · 2023-09-21T08:40:44Z

-    "build": "turbo build --concurrency=${TURBO_CONCURRENCY:-2}",
-    "test": "turbo test --concurrency=${TURBO_CONCURRENCY:-2}",
-    "test:ci": "turbo test --concurrency=${TURBO_CONCURRENCY:-2}",
+    "dev": "FORCE_COLOR=1 turbo dev --filter=@clerk/* --filter=!@clerk/expo",


With FORCE_COLOR=1 Turborepo outputs the logs with their colors. Makes it easier to read them

LekoArts · 2023-09-21T08:41:45Z

  backupCodes?: string[];
 } & UserMetadataParams &
-  (UserPasswordHashingParams | {});
+  (UserPasswordHashingParams | object);


Here (and in other instances) {} was changed to object for now since that's what the author intended. Long-term it probably makes sense to convert this to Record<string, unknown> via ESLint rule

nikosdouvlis · 2023-09-22T08:13:40Z

        align='center'
      >
-        {values.map((value, index) => (
+        {values.map((value, index: number) => (


❓ Why do we need the type annotation here?

For some reason ESLint/TypeScript didn't infer it correctly. I don't know 🤷

I get the correct type locally, but eslint complaints with:

No blocker in any way so I'm going to resolve this as well.

Yeah, I got the same. Rather than type casting that instance of index I chose to explicitly type it in the map

nikosdouvlis

Excellent work

nikosdouvlis · 2023-09-22T12:47:25Z

        align='center'
      >
-        {values.map((value, index) => (
+        {values.map((value, index: number) => (


I get the correct type locally, but eslint complaints with:

No blocker in any way so I'm going to resolve this as well.

clerk-cookie · 2024-09-22T00:01:40Z

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

LekoArts added 11 commits September 20, 2023 14:25

chore(repo): Spell license correctly

9a5b94b

chore(repo): ESLint config file changes

d07676b

chore(backend): ESLint fixes

4ce84a0

chore(chrome-extension): ESLint changes

3f18e8a

chore(clerk-js): Userfacing ESLint changes

9b34031

chore(repo): Turbo env var & TS object linting

b90921a

chore(clerk-sdk-node): Move setupEnvVars file

10adce3

chore(eslint-config-custom): ESLint config changes

68577ab

chore(repo): TODO changes

0c74bef

chore(repo): Add ESLint to lint-staged

c9f854b

chore(repo): Adjust lint-staged config

54aa4ef

clerk-cookie added types clerk-js nextjs react remix backend fastify localizations sdk-node expo chrome-extension gatsby themes labels Sep 21, 2023

LekoArts commented Sep 21, 2023

View reviewed changes

LekoArts marked this pull request as ready for review September 21, 2023 08:44

LekoArts requested review from a team and dimkl as code owners September 21, 2023 08:44

Merge branch 'main' into lekoarts/general-cleanup

e9c954a

nikosdouvlis reviewed Sep 22, 2023

View reviewed changes

LekoArts added 2 commits September 22, 2023 13:48

chore(repo): Update lock file

e5348d0

chore(repo): Review comments

5ce68ca

nikosdouvlis approved these changes Sep 22, 2023

View reviewed changes

Merge branch 'main' into lekoarts/general-cleanup

27a2cd5

LekoArts merged commit 3ceb2a7 into main Sep 22, 2023

LekoArts deleted the lekoarts/general-cleanup branch September 22, 2023 12:58

clerk-cookie mentioned this pull request Sep 22, 2023

Version Packages #1768

Merged

octoper pushed a commit that referenced this pull request Sep 25, 2023

fix(repo): Fix ESLint errors and use ESLint in lint-staged (#1756)

69d982a

clerk locked as resolved and limited conversation to collaborators Sep 22, 2024

		"*.{js,jsx,ts,tsx}": ["npx prettier --write", "npx eslint --fix"],
		"*.{json,md,mdx}": ["npx prettier --write"]

Conversation

LekoArts commented Sep 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Type of change

Packages affected

Uh oh!

changeset-bot Bot commented Sep 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

LekoArts left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LekoArts Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nikosdouvlis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clerk-cookie commented Sep 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LekoArts commented Sep 21, 2023 •

edited

Loading

changeset-bot Bot commented Sep 21, 2023 •

edited

Loading

LekoArts Sep 22, 2023 •

edited

Loading