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.

Sign up for GitHub

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

Jump to bottom

fix(allow-scripts): update types #649

Merged
merged 4 commits into from
Oct 5, 2023
Merged

fix(allow-scripts): update types #649

merged 4 commits into from
Oct 5, 2023

Conversation

boneskull
Copy link
Contributor

This adds @typescript-eslint/parser & its ilk; typescript files (just .d.ts for now) are now linted.

@boneskull
Copy link
Contributor Author

boneskull commented Aug 11, 2023 •
edited

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@boneskull boneskull added the chore overhead, tests, dev env, etc. label Aug 11, 2023
@boneskull boneskull self-assigned this Aug 11, 2023
@boneskull
Copy link
Contributor Author

Ah, urgh. needs node 16

@boneskull
Copy link
Contributor Author

This now splits the CI jobs between test and lint; test runs against all supported Node.js versions and lint only runs on the latest.

@boneskull boneskull force-pushed the boneskull/ts branch 2 times, most recently from 10ddb3a to ac33ab9 Compare August 22, 2023 21:48
leotm
leotm requested changes Sep 22, 2023
View reviewed changes
Copy link
Contributor

@leotm leotm left a comment •
edited

Choose a reason for hiding this comment

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

spotted an eslint warning, so could downgrade to TS 5.2.0
or better bump @typescript-eslint/typescript-parser
to fix the @typescript-eslint/typescript-estree compat warning
then bump @typescript-eslint/eslint-plugin alongside
edit: trying to figure why this isn't happening in CI :suspect:
edit: unsure why i'm on npm@10 🤦‍♂️ but same results on npm@9

Screenshot 2023-09-22 at 5 00 41 pm

leotm
leotm reviewed Sep 22, 2023
View reviewed changes
.eslintrc.js Outdated Show resolved Hide resolved
leotm
leotm reviewed Sep 22, 2023
View reviewed changes
Copy link
Contributor

@leotm leotm left a comment •
edited

Choose a reason for hiding this comment

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

can confirm on Apple silicon need npm i -D @nx/nx-darwin-arm64 for npm run lint

so our solution could be to add @nx/... devDeps for platforms we support (cc @legobeat)

LavaMoat/package-lock.json

Lines 14306 to 14317 in 80a2272

"optionalDependencies": {
"@nx/nx-darwin-arm64": "16.7.4",
"@nx/nx-darwin-x64": "16.7.4",
"@nx/nx-freebsd-x64": "16.7.4",
"@nx/nx-linux-arm-gnueabihf": "16.7.4",
"@nx/nx-linux-arm64-gnu": "16.7.4",
"@nx/nx-linux-arm64-musl": "16.7.4",
"@nx/nx-linux-x64-gnu": "16.7.4",
"@nx/nx-linux-x64-musl": "16.7.4",
"@nx/nx-win32-arm64-msvc": "16.7.4",
"@nx/nx-win32-x64-msvc": "16.7.4"
},

@boneskull
Copy link
Contributor Author

@leotm The nx thing should become moot b/c #727

@boneskull boneskull force-pushed the boneskull/ts-eslint branch 2 times, most recently from 2b82514 to 467dc31 Compare September 25, 2023 20:43
@boneskull
Copy link
Contributor Author

This is going to need to bake a little while longer, as it seems typescript-eslint's type-checking behavior is very fussy.

@boneskull boneskull marked this pull request as draft September 25, 2023 21:28
@leotm leotm self-requested a review September 26, 2023 10:26
@boneskull boneskull marked this pull request as ready for review September 26, 2023 20:38
Base automatically changed from boneskull/ts to main October 5, 2023 17:58
@boneskull
chore(ci): only run lint under Node.js v18.x
c7406c9
@boneskull
chore: lint typescript files
19b0ccf 
This adds `@typescript-eslint/parser` & its ilk; typescript files (just `.d.ts` for now) are now linted.
@boneskull
chore(deps,allow-scripts,yarn-plugin-allow-scripts): lint sources
c3f8d71 
Lints all .ts files and adds `type-fest` as a dev dep cuz it's handy.
@boneskull
fix(allow-scripts): update some types
ef6bc89
@boneskull boneskull changed the title chore: lint typescript files fix(allow-scripts): update types Oct 5, 2023
@boneskull boneskull merged commit f10ac68 into main Oct 5, 2023
11 checks passed
@boneskull boneskull deleted the boneskull/ts-eslint branch October 5, 2023 20:19
@github-actions github-actions bot mentioned this pull request Oct 5, 2023
Merged
boneskull added a commit that referenced this pull request Oct 16, 2023
@boneskull
feat(core): add some types (squash merge)
3bad797 
commit ba28275
Author: Christopher Hiller <boneskull@boneskull.com>
Date:   Thu Oct 12 16:34:33 2023 -0700

    chore(deps): upgrade type-fest globally

commit 328dc8e
Author: Christopher Hiller <boneskull@boneskull.com>
Date:   Tue Oct 3 18:04:37 2023 -0700

    feat(core): ship some types

    - Adds `type-fest` to core
    - Adds `@types/json-stable-stringify` as dev dep
    - Reorganize typescript config

commit f10ac68
Author: Christopher Hiller <boneskull@boneskull.com>
Date:   Thu Oct 5 13:19:41 2023 -0700

    fix(allow-scripts): update types (#649)

    chore(ci): only run lint under Node.js v18.x

    chore: lint typescript files

    This adds `@typescript-eslint/parser` & its ilk; typescript files (just `.d.ts` for now) are now linted.

    chore(deps,allow-scripts,yarn-plugin-allow-scripts): lint sources
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kumavis kumavis kumavis approved these changes

@leotm leotm leotm approved these changes

@legobeat legobeat legobeat approved these changes

Assignees

@boneskull boneskull

Labels
chore overhead, tests, dev env, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@boneskull @kumavis @leotm @legobeat