chore: lint/format/types cleanup — enforce the whole tree in CI#73
Merged
bigmistqke merged 9 commits intoJun 8, 2026
Merged
Conversation
The lint:code glob 'src/**/*.{ts,tsx}' was unquoted, so under sh (no globstar) it only matched one directory level and silently skipped every top-level src/*.ts(x) file. Quoting it lets eslint do its own globbing and lint the whole tree.
That surfaced 23 pre-existing warnings, resolved as:
- simplify (genuinely redundant): dead conditionals/optional-chains, no-op `as` assertions, unused type imports, a stray non-null assertion, and an unused eslint-disable
- fix the lying cast (restore the guard): re-add `| undefined` dropped by casts on LOADER_CACHE.get and props.gl, so the cache-miss and `?? {}` fallbacks mean something again
- document with a targeted disable (real guard the type can't express): applyProp's nested-recursion `!source`, the disable-caching `!useLoader.cache`, the augmentation-bridging gl() cast, and create-xr's `!gl.xr` runtime check
- rewrite isWebGLShadowMap's `in` guard to narrow without a cast (the cast was load-bearing for the `in` operator, so eslint's "unnecessary" was wrong)
No behavior change; lint (eslint/tsc/dpdm) green, 215 tests pass.
The eslint config already declares files: ["**/*.{ts,tsx}"], but lint:code only ever pointed it at src/ — so tests/ went unlinted and the no-only-tests rule could never fire. Point lint:code at tests/ too and resolve the 54 findings it surfaces.
- simplify (redundant): no-op `as` assertions, redundant optional chains and non-null assertions, unused imports/vars; rename type-only `createT` probe vars to `_T`/`_TP`
- honest types over guards: type the fake-XR `listeners` map as `... | undefined` so its defensive `?.` is justified rather than flagged
- refs use a definite assignment assertion (`let ref!: T`) — Solid compiles `ref={ref}` into an assignment, so these must stay `let`; the `!`-with-no-initializer form is the idiomatic pattern and sidesteps prefer-const without a disable
- async-utils: `while (true)` -> `for (;;)`, drop unused catch binding
- setup.ts: targeted no-console disable for the deliberate console.trace diagnostic
- import/extensions: add ignorePackages so it enforces extensions on relative imports only (it was erroring on the `solid-js/web` package subpath, which can't take a `.js`)
No behavior change; lint (eslint over src + tests, tsc, dpdm) green, 215 tests pass.
The pkg-pr-new workflow type-checked and tested every push/PR but never ran lint:code or lint:circular, so eslint and dependency-cycle regressions could land unnoticed. Replace the Typecheck step with `pnpm lint` (lint:circular && lint:code && lint:types). It stays after Build so lint:types can still resolve site/'s linked types.
commit: |
Two eslint-disables were really signals of imprecise types. Fix the types instead: - useLoader.cache: widen to `LoaderRegistry | undefined`. Its JSDoc already documents disabling the cache by setting it to `undefined`, so the type was simply wrong; widening it makes the `!useLoader.cache` guard type-justified. Adds a test for the cache-disabled path (resolves without caching). - createXR: replace the `as unknown as XRRenderer` casts and the disabled `!gl.xr` check with an `isXRRenderer` type-guard. Widen `RendererLike.init` to `Promise<void | this>` so a WebGPURenderer is assignable to `RendererLike` — letting the effect type `gl` as `RendererLike`, which the guard narrows to the flat, callable `XRRenderer` slice (the open `Renderer` union's `xr.addEventListener` overloads don't combine into a callable signature). lint (eslint + tsc + dpdm) green, 215 tests pass.
A `.prettierrc` existed but Prettier was never actually enforced — not a dependency, no script, and absent from `lint`/CI — so files had drifted from it. Same "configured but unapplied" gap as the unquoted lint:code glob. - add `prettier@3.8.3` as a devDependency - add `format` (write) and `lint:format` (check) scripts, and chain `lint:format` into `lint` — so the CI `pnpm lint` step added earlier now enforces formatting too - run the formatter across src + tests; folds in the import-ordering tidy-ups already present in the tree Formatting only, no behavior change; lint (circular + code + format + types) green, 216 tests pass.
bigmistqke
changed the title
bigmistqke
changed the title
`Renderer` shadowed three's own `Renderer` class (the WebGPU base that `WebGPURenderer` extends) — confusing, since the union literally contains a `three.Renderer`. Rename it to `SupportedRenderer`. It stays publicly exported: it's the type you widen `Register` to when a project mixes renderers across components (the docs site does exactly this). `RendererLike` (custom-renderer contract) and `ResolvedRenderer` (augmented concrete type) are unchanged. BREAKING CHANGE: the `Renderer` type export is renamed to `SupportedRenderer`. Update imports and `Register` augmentations accordingly.
bigmistqke
force-pushed
the
chore/lint-glob-and-warnings
branch
from
a5704d4 to
c7dedc9
Compare
None of these are referenced in code, scripts, or config: - `jest` — the project tests with vitest; its `jest` config block pointed at a `scripts/jest/node` preset that doesn't exist. Removed the block too. - `solid-app-router` — superseded by `@solidjs/router` (used in the site workspace, which has its own dep). - `tsm`, `esbuild-register`, `jiti` — TS loaders nothing invokes; `jiti`/`esbuild-register` remain available transitively for the tools that actually use them. Build, lint, and 216 tests still pass.
`format`/`lint:format` only covered `src` + `tests`, leaving `demo/` and the root config files unformatted and already drifting. Widen the globs to include `demo/**/*.{ts,tsx}` and root `*.{ts,js,mjs,cjs}`, and format the three files that had drifted (`demo/src/App.tsx`, `eslint.config.js`, `vitest.config.ts`).
(eslint coverage of `demo/` is left out — it's excluded from the root tsconfig, so type-aware linting there needs separate tsconfig wiring.)
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.
Summary
A general lint/format/types/deps cleanup pass. The project had several quality gates that were configured but not actually enforced — an eslint glob that silently skipped files, a
.prettierrcwith no Prettier wired up, and a CI step that only type-checked. This makes them real, fixes everything they surface, tidies the renderer type names, and prunes dead dependencies.What changed
eslint coverage
lint:code's glob was unquoted (src/**/*.{ts,tsx}), so undershit only matched one directory level — every top-levelsrc/*.ts(x)file went unlinted. Quote it, and extend it totests/(matching the eslint config's ownfiles: ["**/*.{ts,tsx}"], and makingno-only-testsactually apply).src/, 54 intests/): redundant casts/conditions/optional-chains, unused vars/imports, lying casts re-typed honestly, genuine runtime guards kept.type-level fixes over disables
useLoader.cachewidened toLoaderRegistry | undefined(its JSDoc documents disabling viaundefined), so!useLoader.cacheis type-justified. Adds a cache-disabled test.createXRdrops theas unknown as XRRenderercasts and disabled!gl.xrcheck for anisXRRenderertype-guard;RendererLike.initwidened toPromise<void | this>soWebGPURenderersatisfiesRendererLike, letting the guard narrow to the flat, callable XR slice.Prettier
.prettierrcexisted but Prettier wasn't a dep, a script, or in CI. Addprettier@3.8.3, aformat/lint:formatscript, chainlint:formatintolint, and formatsrc+tests+demo/+ root config files.renderer type names
Rendererunion shadowed three's ownRendererclass (the WebGPU baseWebGPURendererextends). Rename it toSupportedRenderer. It stays publicly exported (the type you widenRegisterto for mixed-renderer projects — the docs site does exactly this).RendererLikeandResolvedRendererare unchanged.Renderertype export is renamed toSupportedRenderer. Update imports andRegisteraugmentations.dependencies
jest(project uses vitest; its dead config block referenced a missing preset),solid-app-router(superseded by@solidjs/router), and the unused TS loaderstsm/esbuild-register/jiti.CI
pnpm lint(circular && code && format && types) on every push/PR, instead oflint:typesalone.Verification
pnpm build && pnpm lint && pnpm testgreen; all 216 tests pass. The only API change is theRenderer→SupportedRenderertype rename.