replace ESLint 8 + Prettier with Biome, and gate lint/typecheck in CI - #11
Merged
Conversation
…k in CI The repo already had a linter; what it lacked was enforcement. ESLint and Prettier were configured and wired into the codegen script, but there was no `lint` script, `.husky/pre-commit` called `npx lint-staged` which was never installed, and CI ran only build and test. Nothing kept the tree clean. Standing put was not an option either: ESLint 8.56 has been EOL since October 2024 and typescript-eslint 7 is a major behind, so either path was a migration. That is what makes Biome the better trade here rather than merely the trendier one: - There was almost no config to lose. eslint-plugin-react was installed in a Node HTTP client with no React, eslint-plugin-jest was never extended, and eslint-plugin-import was pulled in only for `plugin:import/typescript`, which contributes resolver settings and zero rules. - Speed lands where it hurts. A full ESLint pass took 4.4s, 4.0s of which was the two generated files -- and `npm run generate` paid that twice, once for Prettier and once for ESLint. Biome checks all 23 files in ~85ms. - Nine devDependencies collapse to one. The accepted cost: noFloatingPromises and noMisusedPromises are the highest-value rules for a library where every public method returns a Promise, and in Biome 2.5 both are still nursery. They are enabled explicitly, and a floating promise was confirmed to actually trip the rule rather than trusting the config. files.maxSize is raised to 4 MiB. src/types.generated.ts is ~1 MiB and silently exceeds Biome's 1 MiB default, which skipped the largest file in the repo without reporting a failure. Also adds `npm run typecheck`, closing a separate hole: nothing typechecked test/**. tsconfig-build.json compiles only src/, and @swc/jest strips types without checking them, so a type error in a test was invisible. Verified by introducing one -- tsc fails, jest still passes. Removes `importHelpers: true` from tsconfig.json. It required tslib, which was only ever present as a hoisted transitive dep of the ESLint stack, so removing those packages surfaced it. tsconfig-build.json already sets it to false, so published output was never affected and no dependency needs adding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mechanical, behavior-preserving fixes from the Biome migration: - node: protocol on builtin imports (fs, path, events, url) - `import type` for type-only imports, plus sorted import order - drop dead imports and unused locals, including the unused URLSearchParams import in test/zoomApi.test.ts that ESLint had been reporting to nobody - `let throwError` -> `let throwError: unknown` (noImplicitAnyLet) - remove ZoomS2SO's constructor, which only forwarded to super(client). ZoomOauth declares the identical signature, so `new ZoomS2SO(client)` still typechecks for consumers through inheritance and dist/zoomS2SO.d.ts still exposes the class unchanged apart from the now-inherited constructor. src/types.generated.ts is untouched: Biome's formatting of all 19k lines matches Prettier's byte for byte. src/zoomApi.generated.ts changes only in import order, and `npm run generate` is idempotent against the result. One finding left in place rather than silently "fixed": openapi-parser.ts keeps its non-null assertion behind a biome-ignore. The map key is guaranteed by the block directly above it, and Biome's suggested `?.` rewrite would silently drop endpoints instead of failing loudly. Note that `--unsafe` applies that rewrite even with a suppression present if the suppression's reason wraps onto a second line -- biome-ignore has to be a single line to bind to the next statement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each was an early-generation helper that a later implementation replaced without
the original being removed:
pathToResourceName -> logic now inlined in openapi-parser.ts
operationIdToMethodName -> determineMethodName in api-generator.ts
operationIdToTypeName -> operationIdToBaseTypeName
extractPathParams -> getFirstPathParamName
getResourceGroup -> groupEndpointsByResource
getResourceGroup is the one worth calling out: it still carried the naive
`endsWith('s') ? slice(0, -1)` singularization that 627f09a fixed in
api-generator.ts, so it was a live trap for anyone who wired it up.
No cascade -- snakeToCamel and snakeToPascal remain in use from api-generator.ts
and type-generator.ts.
Scope: scripts/ only. Confirmed by counting references minus declarations for
every function in non-generated code (13 exported and 18 module-local in
scripts/, 7 local in src/), then closing the gaps that census misses: all 13
exported interfaces in openapi-parser.ts (every one reachable from OpenApiSpec),
arrow-function consts (none exist), and every private/protected method in src/
(all used). These five were the only dead ones.
Nothing removed from src/. Its exports are published API, so an export with no
internal caller is still a consumer's entry point, not dead code -- and the 250+
methods in zoomApi.generated.ts are the library's product. `npm run generate`
produces byte-identical output before and after these deletions, which is the
strongest available evidence that nothing reachable was removed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The repo already had a linter; what it lacked was enforcement. ESLint and
Prettier were configured and wired into the codegen script, but there was no
lintscript,.husky/pre-commitcallednpx lint-stagedwhich was neverinstalled, and CI ran only build and test. Nothing kept the tree clean.
Standing put was not an option either: ESLint 8.56 has been EOL since October
2024 and typescript-eslint 7 is a major behind, so either path was a migration.
That is what makes Biome the better trade here rather than merely the trendier
one:
Node HTTP client with no React, eslint-plugin-jest was never extended, and
eslint-plugin-import was pulled in only for
plugin:import/typescript, whichcontributes resolver settings and zero rules.
the two generated files -- and
npm run generatepaid that twice, once forPrettier and once for ESLint. Biome checks all 23 files in ~85ms.
The accepted cost: noFloatingPromises and noMisusedPromises are the
highest-value rules for a library where every public method returns a Promise,
and in Biome 2.5 both are still nursery. They are enabled explicitly, and a
floating promise was confirmed to actually trip the rule rather than trusting
the config.
files.maxSize is raised to 4 MiB. src/types.generated.ts is ~1 MiB and silently
exceeds Biome's 1 MiB default, which skipped the largest file in the repo
without reporting a failure.
Also adds
npm run typecheck, closing a separate hole: nothing typecheckedtest/**. tsconfig-build.json compiles only src/, and @swc/jest strips types
without checking them, so a type error in a test was invisible. Verified by
introducing one -- tsc fails, jest still passes.
Removes
importHelpers: truefrom tsconfig.json. It required tslib, which wasonly ever present as a hoisted transitive dep of the ESLint stack, so removing
those packages surfaced it. tsconfig-build.json already sets it to false, so
published output was never affected and no dependency needs adding.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com