chore: split out format, add type checking

[wrslatz]

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

• Updated test to run type-check prior to running tests (alternative to keep jest tests fast without using ts-jest for transformation, can be replaced with vitest type checking support if we swap to vitest in Switch to ESM #331)

[riley-kohler]

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]

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]

@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?

[riley-kohler]

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]

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]

@riley-kohler I removed the type checking workflow and made type checking run as part of the test script. One downside of this is that type checking runs on each Node.js version we test against in the tests workflow. The same would happen if types were checked in the test tool, though. Thoughts?

[riley-kohler]

I'm fine with that. Especially since vitest would do the same thing.

[wrslatz]

@zkoppert could you add the format job to required statuses when you get a chance?