Skip to content

chore: split out format, add type checking#436

Draft
wrslatz wants to merge 2 commits intomainfrom
add-type-check-split-lint-format
Draft

chore: split out format, add type checking#436
wrslatz wants to merge 2 commits intomainfrom
add-type-check-split-lint-format

Conversation

@wrslatz
Copy link
Copy Markdown
Contributor

@wrslatz wrslatz commented Apr 15, 2026

Separate format and lint scripts and workflows, and add a type checking script and workflow, as discussed in #435

  • Split format using prettier into format (check) and format:fix (write)

  • Made lint only run ESLint checks, split Prettier out to new format scripts

  • Add new script to run type checking using tsc

  • Updated lint-staged to run format then lint on staged files, and run type-check on JS/TS files; no longer fixing changes during commit, as this can lead to unintended changes

  • Added .github/workflows/style.yml to check formatting

  • Added .github/workflows/type-check.yml to run TypeScript type checks

@wrslatz wrslatz requested a review from riley-kohler April 15, 2026 19:27
@wrslatz wrslatz marked this pull request as ready for review April 15, 2026 19:28
@wrslatz wrslatz requested review from a team as code owners April 15, 2026 19:28
@wrslatz wrslatz self-assigned this Apr 16, 2026
Copy link
Copy Markdown
Contributor

@riley-kohler riley-kohler left a comment

Choose a reason for hiding this comment

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

I'm ok to move forward with this but ideally would prefer type checking as part of the build workflow. In my view, tsc was built to both type check and transpile so we should retain that. While modern node tooling often leaves out typechecking, typescript code isn't correct if it isn't properly typed.

@wrslatz
Copy link
Copy Markdown
Contributor Author

wrslatz commented Apr 16, 2026

I'm ok to move forward with this but ideally would prefer type checking as part of the build workflow. In my view, tsc was built to both type check and transpile so we should retain that. While modern node tooling often leaves out typechecking, typescript code isn't correct if it isn't properly typed.

The only reason I made it a separate workflow is because we are type checking tests, which are not included in the final build. I'd be happy to move that into the build workflow though as a separate job there. It is semantics, really.

@wrslatz wrslatz force-pushed the add-type-check-split-lint-format branch from 1a26d05 to 8b695b6 Compare April 16, 2026 14:27
@wrslatz wrslatz requested a review from riley-kohler April 16, 2026 14:27
@wrslatz wrslatz force-pushed the add-type-check-split-lint-format branch from 8b695b6 to acdde49 Compare April 16, 2026 14:29
@wrslatz wrslatz requested a review from zkoppert April 16, 2026 14:30
@wrslatz
Copy link
Copy Markdown
Contributor Author

wrslatz commented Apr 16, 2026

@zkoppert if these changes make sense to you, could you add the new format and type-check jobs as required checks for PRs to main, like we did in #429?

wrslatz and others added 2 commits April 16, 2026 10:32
- Split format using prettier into format and format:fix
- Made lint only run ESLint checks
- Updated lint-staged to run format then lint on staged files
- Added .github/workflows/style.yml to check formatting
- Added .github/workflows/type-check.yml to run TypeScript type checks

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@wrslatz wrslatz force-pushed the add-type-check-split-lint-format branch from acdde49 to 4144209 Compare April 16, 2026 14:32
@riley-kohler
Copy link
Copy Markdown
Contributor

riley-kohler commented Apr 16, 2026

I'm ok to move forward with this but ideally would prefer type checking as part of the build workflow. In my view, tsc was built to both type check and transpile so we should retain that. While modern node tooling often leaves out typechecking, typescript code isn't correct if it isn't properly typed.

The only reason I made it a separate workflow is because we are type checking tests, which are not included in the final build. I'd be happy to move that into the build workflow though as a separate job there. It is semantics, really.

That's fair. I guess ideally we would type check the src files as part of the build and tests as part of running the tests though we can implement that down the line. I believe vitest has a built in option for type checking and I would like to migrate to it eventually.

@wrslatz
Copy link
Copy Markdown
Contributor Author

wrslatz commented Apr 16, 2026

I guess ideally we would type check the src files as part of the build and tests as part of running the tests though we can implement that down the line. I believe vitest has a built in option for type checking and I would like to migrate to it eventually.

Makes sense to me! I think Next.js already compiles code using tsc under the hood, actually. In which case, type checking tests as part of the test tool probably makes sense. I can try to address that as part of this change.

@wrslatz wrslatz marked this pull request as draft April 16, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants