chore: migrate from eslint+prettier to Vite+#8
Conversation
Replace eslint and prettier with Vite+ unified toolchain (oxlint for linting, oxfmt for formatting). Standardize all package lint scripts to use `vp lint` / `vp lint --fix`. Add per-package oxlint/oxfmt configs for VS Code multi-root workspace support. Remove orphaned ecs-benchmark CI workflow. Delete stale docs (benchmark.md, hierarchy-ideas.md, performance-plan.md, tasks.md) — content moved to GitHub issues #6 and #7. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ Deploy Preview for objecs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughReplaces the ESLint/Prettier/Vitest toolchain with a vite-plus (VP) centered toolchain: adds root VP/vite-plus config, OxLint/OxFmt configs, per-package ox configs, updates scripts/dependencies to VP/vite-plus, normalizes formatting and lint directives, and updates documentation and workspace/editor settings. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (editor/terminal)
participant Git as Git (pre-commit)
participant VP as vp CLI (vite-plus)
participant OxLint as OxLint
participant Editor as Editor/VSCode (oxc.vscode)
Dev->>Git: git commit
Git->>VP: run "vp staged"
VP->>OxLint: lint staged files (type-aware)
OxLint-->>VP: lint results & fixes
VP->>Git: allow/deny commit
Dev->>Editor: save file
Editor->>VP: source.fixAll.oxc / format on save
VP->>OxLint: apply fixes & format
OxLint-->>Editor: updated file
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move build config into vite.config.ts pack section, update package.json exports to match tsdown output extensions (.mjs/.cjs/.d.mts/.d.cts), and remove tsup dependency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/examples/src/lib/event-emitter.ts (1)
28-35:⚠️ Potential issue | 🔴 CriticalSpread
argswhen invoking listeners.At line 34,
listener(args)passes the tuple as a single argument instead of spreading it, causing listeners to receive the wrong payload shape. Single-arg events will receive an array instead of the value; multi-arg events will fail entirely.Proposed fix
public emit<K extends keyof T>(event: K, ...args: Parameters<T[K]>) { if (!this.has(event)) { return; } for (const listener of this.get(event) ?? []) { - listener(args); + listener(...args); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/src/lib/event-emitter.ts` around lines 28 - 35, The emit method is calling listener(args) which passes the entire args tuple as one parameter; change the invocation in EventEmitter.emit to spread the tuple so listeners receive the actual parameters by calling listener(...args) (locate the emit method and the loop over this.get(event) listeners where listener(args) is used).CLAUDE.md (1)
20-57:⚠️ Potential issue | 🟡 MinorUpdate "Common Commands" section to use
vpworkflow instead ofpnpmdirectly.Lines 20–57 teach developers to use
pnpm install,pnpm lint, andpnpm --filter ...commands, but line 228 explicitly states "Do not use pnpm, npm, or Yarn directly." Replace the Common Commands section withvpequivalents (vp install,vp lint,vp run, etc.) to align with the Vite+ guidance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 20 - 57, Update the "Common Commands" examples under the "Root-level", "Core Library (packages/objecs)", "Examples (packages/examples)", "Benchmarks", and "Game Benchmarks" sections to use the project's vp workflow instead of pnpm: replace occurrences like `pnpm install`, `pnpm lint`, `pnpm lint:fix`, `pnpm --filter <pkg> build/test/run/lint`, and `pnpm --filter <pkg> start` with the corresponding `vp` commands (`vp install`, `vp lint`, `vp lint:fix`, `vp --filter <pkg> run build` / `vp --filter <pkg> run test` / `vp --filter <pkg> run` / `vp --filter <pkg> run lint`, or `vp --filter <pkg> run start`), and adjust single-command examples (e.g., `pnpm --filter examples dev` → `vp --filter examples run dev`) so the documentation aligns with the "Do not use pnpm, npm, or Yarn directly" guidance.
🧹 Nitpick comments (8)
packages/game-benchmark/src/games/mutation/miniplex/index.ts (1)
124-133: Type assertion is safe but could be cleaner.The type assertion
(entity as Entity & { position: { x: number } }).position.xworks at runtime because all entities are created withposition(lines 71-80). However, iterating over queries that don't requireposition(e.g.,shielded,poisonedEntities) with this assumption couples the iteration logic to the entity creation logic.For a benchmark file this is acceptable, but consider documenting this invariant with a comment if it's intentional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/game-benchmark/src/games/mutation/miniplex/index.ts` around lines 124 - 133, The iteration uses a type assertion on entity to read position.x inside iterateSystem (sum += (entity as Entity & { position: { x: number } }).position.x) which relies on all entities being created with position (lines ~71-80); make this invariant explicit by adding a short comment above the iterateSystem block (or immediately before the assertion) stating that every entity produced by the benchmark always has a position property so the cast is intentional and safe, mentioning the related queries/collections (queries, shielded, poisonedEntities) to avoid surprising future readers.packages/examples/src/demos/shmup/systems/sprite-animation-system.ts (1)
30-35: Guard these frame lookups instead of suppressing them.Both indexed reads can still come back
undefinedat runtime, and the next assignment would propagate that bad state into the sprite data. A small guard/invariant here is safer than carrying the!s forward.Suggested change
- const frameIndex = - // oxlint-disable-next-line `@typescript-eslint/no-non-null-assertion` - spriteAnimation.frameSequence[spriteAnimation.currentFrame]!; - - // oxlint-disable-next-line `@typescript-eslint/no-non-null-assertion` - const frame = spriteAnimation.frames[frameIndex]!; + const frameIndex = + spriteAnimation.frameSequence[spriteAnimation.currentFrame]; + const frame = + frameIndex === undefined + ? undefined + : spriteAnimation.frames[frameIndex]; + if (frame === undefined) { + world.removeEntityComponents(entity, "spriteAnimation"); + continue; + }As per coding guidelines,
Remove components via world.removeEntityComponents(entity, 'component') which uses delete; never set a component value to undefined.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/src/demos/shmup/systems/sprite-animation-system.ts` around lines 30 - 35, The indexed reads using spriteAnimation.frameSequence[spriteAnimation.currentFrame] and spriteAnimation.frames[frameIndex] must be guarded instead of using non-null assertions: check that frameIndex is not undefined and that frame exists before assigning into sprite data; if either lookup fails, do not set spriteAnimation.currentFrame/frame or sprite data—instead remove the sprite component using world.removeEntityComponents(entity, 'spriteAnimation') (or the appropriate component name) to avoid storing undefined. Update the logic in sprite-animation-system.ts around the frameIndex/frame resolution (symbols: spriteAnimation, frameSequence, currentFrame, frames, frameIndex, frame) to perform these runtime checks and call world.removeEntityComponents when the invariants are violated.packages/perf-proofs/src/array/for-of-vs-for-index-vs-foreach.ts (1)
12-15: Model the benchmark entity fromEntityBase.This benchmark is meant to approximate ECS-style entity iteration, but the local
Entityshape drops the base entity contract entirely. ExtendingEntityBasehere keeps the measured object layout closer to the entities used elsewhere in the repo.Suggested change
+import type { EntityBase } from "objecs"; import { run, bench, summary } from "mitata"; -type Entity = { +type Entity = EntityBase & { position: { x: number; y: number }; velocity: { x: number; y: number }; };As per coding guidelines,
Entities must extend EntityBase and components must be JSON-compatible values (no functions/symbols).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/perf-proofs/src/array/for-of-vs-for-index-vs-foreach.ts` around lines 12 - 15, Replace the local Entity type with one that extends the repo's EntityBase so the benchmark uses the same entity layout as other code: import EntityBase if necessary and declare type Entity = EntityBase & { position: { x: number; y: number }; velocity: { x: number; y: number } }; ensure both position and velocity remain plain JSON-compatible objects (no functions/symbols) so the Entity shape used in for-of-vs-for-index-vs-foreach.ts matches the ECS-style entities elsewhere.packages/examples/src/demos/shmup/systems/text-blink-animation-system.ts (1)
27-32: Avoid writing an unresolved color back into the component.These
!-suppressed lookups can still yieldundefinedfrom a bad sequence index or missing color key. Guard the lookup and keep the previous color when the animation data is inconsistent.Suggested change
- const colorSequence = - // oxlint-disable-next-line `@typescript-eslint/no-non-null-assertion` - textBlinkAnimation.colorSequence[textBlinkAnimation.currentColorIndex]!; - - // oxlint-disable-next-line `@typescript-eslint/no-non-null-assertion` - textBlinkAnimation.color = textBlinkAnimation.colors[colorSequence]!; + const colorSequence = + textBlinkAnimation.colorSequence[textBlinkAnimation.currentColorIndex]; + const color = + colorSequence === undefined + ? undefined + : textBlinkAnimation.colors[colorSequence]; + if (color === undefined) { + continue; + } + textBlinkAnimation.color = color;As per coding guidelines,
Remove components via world.removeEntityComponents(entity, 'component') which uses delete; never set a component value to undefined.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/src/demos/shmup/systems/text-blink-animation-system.ts` around lines 27 - 32, Guard the two non-null assertions when resolving the next blink color: read const seqKey = textBlinkAnimation.colorSequence[textBlinkAnimation.currentColorIndex] and const resolved = textBlinkAnimation.colors[seqKey], then only assign textBlinkAnimation.color = resolved if resolved !== undefined; otherwise leave the existing textBlinkAnimation.color unchanged (do not set it to undefined or delete the property). This uses the existing identifiers textBlinkAnimation.colorSequence, textBlinkAnimation.currentColorIndex, textBlinkAnimation.colors and textBlinkAnimation.color to locate and fix the code.pnpm-workspace.yaml (1)
11-14: Pin the shared Vite+ catalog entries instead oflatest.Using mutable tags here means a clean install or lockfile refresh can move the lint/build toolchain for the whole workspace without any manifest change. For a repo-wide tooling migration, it’s safer to pin the tested versions and bump them intentionally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pnpm-workspace.yaml` around lines 11 - 14, The catalog entries "vite", "vitest", and "vite-plus" currently point to mutable tags (npm:`@voidzero-dev/vite-plus-core`@latest, npm:`@voidzero-dev/vite-plus-test`@latest, vite-plus: latest); change each to a pinned version string (e.g. replace "@latest" and "latest" with exact semver versions like "1.2.3" or the commit SHA you verified) so the workspace toolchain is deterministic, then run pnpm install to update the lockfile and commit the updated pnpm-lock.yaml alongside the manifest change..oxlintrc.json (1)
177-177: Move@typescript-eslint/prefer-for-ofexception to package-level override in.oxlintrc.json.The global
"error"setting at line 177 forces 8 inlineoxlint-disable-next-linesuppressions acrosspackages/perf-proofs/**. These files benchmark different iteration strategies, so the rule enforces noise rather than guidance. A package-level override eliminates the scattered disables:Suggested override
"overrides": [ + { + "files": ["packages/perf-proofs/**/*.{ts,tsx,mts,cts}"], + "rules": { + "@typescript-eslint/prefer-for-of": "off" + } + }, { "files": ["**/*.{js,mjs,cjs,ts,mts,cts}"], "rules": { "depend/ban-dependencies": "error"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.oxlintrc.json at line 177, The project-wide rule "@typescript-eslint/prefer-for-of" is set to "error" in .oxlintrc.json causing multiple inline oxlint-disable-next-line comments in packages/perf-proofs/**; remove or relax that global rule and add a package-level override for the perf-proofs package that disables or changes "@typescript-eslint/prefer-for-of" for "packages/perf-proofs/**" so benchmarks can use alternative iteration without inline suppressions; update the "overrides" array in .oxlintrc.json to include a new object targeting "packages/perf-proofs/**" with the rule adjusted and remove the need for the eight inline disables.packages/game-benchmark/src/games/mutation/objecs/index.ts (1)
92-95: Tighten the factory return type instead of asserting non-null here.
Entity[OptionalComponent]is optional, so the!suppresses a real type mismatch. Use a mapped type to ensure factories return non-null values, aligning with the guideline to never set component values toundefined.♻️ Suggested type tightening
- const componentFactories: Record< - OptionalComponent, - () => Entity[OptionalComponent] - > = { + const componentFactories: { + [K in OptionalComponent]: () => NonNullable<Entity[K]>; + } = { shield: () => ({ strength: 10 + Math.random() * 90 }), poisoned: () => ({ tickDamage: 1 + Math.random() * 5 }), stunned: () => ({ duration: 0.5 + Math.random() * 2 }), buff: () => ({ multiplier: 1.1 + Math.random() * 0.9 }), }; @@ } else { world.addEntityComponents( entity, comp, - // oxlint-disable-next-line `@typescript-eslint/no-non-null-assertion` - componentFactories[comp]()!, + componentFactories[comp](), ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/game-benchmark/src/games/mutation/objecs/index.ts` around lines 92 - 95, The factory map componentFactories currently types return values as Entity[OptionalComponent] (which is optional) and relies on non-null assertions; change the return type to guarantee non-null by using a mapped/indexed type such as mapping over OptionalComponent (e.g. [K in OptionalComponent]: () => NonNullable<Entity[K]>) so each factory returns a concrete component value (not undefined), and remove any trailing `!` assertions in the factory implementations to satisfy the "never undefined" guideline; keep references to componentFactories, OptionalComponent and Entity when updating the type.packages/objecs/vite.config.ts (1)
2-5: Consider adding a brief explanation for the lint suppression.The
oxlint-disable-next-linecomment suppressestypescript/no-unsafe-assignment. While this may be necessary due todefineConfig's return type inference, a brief inline comment explaining why it's needed would help future maintainers understand the suppression isn't masking a real issue.📝 Suggested clarification
-// oxlint-disable-next-line typescript/no-unsafe-assignment +// oxlint-disable-next-line typescript/no-unsafe-assignment -- defineConfig returns untyped config object const config = defineConfig({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/objecs/vite.config.ts` around lines 2 - 5, Add a short inline rationale immediately after the oxlint-disable-next-line typescript/no-unsafe-assignment comment explaining why the suppression is necessary (e.g., defineConfig's generic/third-party return type causes a false positive and the assignment is safe), so future maintainers know this isn't hiding a real type bug; reference the suppression and the defineConfig usage in your note and keep it concise (one sentence) describing the specific reason and that it was reviewed/intentional.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vscode/settings.json:
- Line 2: The settings entry "js/ts.preferences.importModuleSpecifierEnding" is
not recognized by VS Code; replace it with the documented keys
"javascript.preferences.importModuleSpecifierEnding" and
"typescript.preferences.importModuleSpecifierEnding" and set both to the same
value ("js") so the preference is applied for both JS and TS editors; update the
.vscode/settings.json entry that currently contains
"js/ts.preferences.importModuleSpecifierEnding" accordingly.
In `@CLAUDE.md`:
- Around line 228-234: The guidance contradicts Oxc docs: update the bullet
about "Type-Aware Linting" to state definitively whether oxlint-tsgolint must be
present or is auto-managed by Vite+; specifically either (A) change the sentence
to "Vite+ automatically installs and manages oxlint-tsgolint for type-aware
linting, so you do not need to add it manually" or (B) change it to "Install
oxlint-tsgolint as a devDependency (e.g., oxlint-tsgolint@0.16.0) to enable vp
lint --type-aware", and ensure the term "oxlint-tsgolint" and the command "vp
lint --type-aware" are mentioned exactly so readers know the dependency and the
command to run.
In `@packages/examples/src/demos/basic/index.html`:
- Around line 19-20: The CSS block setting "width: 640;" and "height: 480;" is
invalid and ignored by browsers; update the CSS for the canvas (the rules
containing width: 640 and height: 480) to use explicit units (e.g. "width:
640px;" and "height: 480px;") or remove those declarations entirely since the
<canvas> element already has width="640" and height="480" HTML attributes that
determine its drawing size.
In `@packages/examples/src/demos/shmup/components/text-blink-animation.ts`:
- Around line 14-16: The code currently uses non-null assertions on
colorSequence/currentColorIndex and durationMs which can yield undefined color
or Infinity frameRate; update the logic in the text-blink-animation function to
defensively validate inputs: ensure colorSequence is non-empty (fallback to a
safe default sequence), clamp or validate currentColorIndex against
colorSequence.length so colors[colorSequence[currentColorIndex]] cannot be
undefined, and ensure durationMs and computed divisor (colorSequence.length) are
> 0 (fallback to a sensible positive value) before computing frameRate; replace
the "!" assertions with safe access and explicit defaults so the returned
component never contains undefined values.
In `@packages/objecs/src/archetype.ts`:
- Around line 146-152: The chained .without() currently overwrites prior
exclusions by passing only the new components array (the line constructing the
new archetype uses "without: components as any"); modify the creation so it
preserves and merges prior exclusions with the newly-specified ones (e.g.,
combine this.#without with components into a deduplicated list/set) before
assigning to the new archetype's without field so successive
.without("b").without("c") retains both exclusions; update the code around the
Archetype cloning/creation (the method named without / the constructor call that
sets world, components, without) to perform this merge and dedupe.
In `@packages/perf-proofs/src/map-set/map-vs-object-lookup.ts`:
- Line 16: The object lookup in the "known-hit" loop should be a pure lookup to
match the Map side: replace the nullish coalescing use (obj[k] ?? 0) with a
non-null assertion (obj[k]!) so the benchmark measures the same pure read as
map.get(k)!; update the loop that compares obj and map lookups to use obj[k]!
(keeping the existing obj: Record<string, number | undefined> declaration).
In `@vite.config.ts`:
- Around line 191-201: The current override uses jsPlugins:
["eslint-plugin-depend"] which the Oxlint language server ignores, so the
depend/ban-dependencies rule only appears in CLI; to fix, move the plugin into
ESLint's standard plugin configuration (e.g., add "plugins": ["depend"] inside
the same override or at top-level) and remove jsPlugins, ensure the rule
"depend/ban-dependencies" remains in the override's rules, and confirm the
plugin is installed (package.json) so VS Code LSP will load and surface the rule
diagnostics.
---
Outside diff comments:
In `@CLAUDE.md`:
- Around line 20-57: Update the "Common Commands" examples under the
"Root-level", "Core Library (packages/objecs)", "Examples (packages/examples)",
"Benchmarks", and "Game Benchmarks" sections to use the project's vp workflow
instead of pnpm: replace occurrences like `pnpm install`, `pnpm lint`, `pnpm
lint:fix`, `pnpm --filter <pkg> build/test/run/lint`, and `pnpm --filter <pkg>
start` with the corresponding `vp` commands (`vp install`, `vp lint`, `vp
lint:fix`, `vp --filter <pkg> run build` / `vp --filter <pkg> run test` / `vp
--filter <pkg> run` / `vp --filter <pkg> run lint`, or `vp --filter <pkg> run
start`), and adjust single-command examples (e.g., `pnpm --filter examples dev`
→ `vp --filter examples run dev`) so the documentation aligns with the "Do not
use pnpm, npm, or Yarn directly" guidance.
In `@packages/examples/src/lib/event-emitter.ts`:
- Around line 28-35: The emit method is calling listener(args) which passes the
entire args tuple as one parameter; change the invocation in EventEmitter.emit
to spread the tuple so listeners receive the actual parameters by calling
listener(...args) (locate the emit method and the loop over this.get(event)
listeners where listener(args) is used).
---
Nitpick comments:
In @.oxlintrc.json:
- Line 177: The project-wide rule "@typescript-eslint/prefer-for-of" is set to
"error" in .oxlintrc.json causing multiple inline oxlint-disable-next-line
comments in packages/perf-proofs/**; remove or relax that global rule and add a
package-level override for the perf-proofs package that disables or changes
"@typescript-eslint/prefer-for-of" for "packages/perf-proofs/**" so benchmarks
can use alternative iteration without inline suppressions; update the
"overrides" array in .oxlintrc.json to include a new object targeting
"packages/perf-proofs/**" with the rule adjusted and remove the need for the
eight inline disables.
In `@packages/examples/src/demos/shmup/systems/sprite-animation-system.ts`:
- Around line 30-35: The indexed reads using
spriteAnimation.frameSequence[spriteAnimation.currentFrame] and
spriteAnimation.frames[frameIndex] must be guarded instead of using non-null
assertions: check that frameIndex is not undefined and that frame exists before
assigning into sprite data; if either lookup fails, do not set
spriteAnimation.currentFrame/frame or sprite data—instead remove the sprite
component using world.removeEntityComponents(entity, 'spriteAnimation') (or the
appropriate component name) to avoid storing undefined. Update the logic in
sprite-animation-system.ts around the frameIndex/frame resolution (symbols:
spriteAnimation, frameSequence, currentFrame, frames, frameIndex, frame) to
perform these runtime checks and call world.removeEntityComponents when the
invariants are violated.
In `@packages/examples/src/demos/shmup/systems/text-blink-animation-system.ts`:
- Around line 27-32: Guard the two non-null assertions when resolving the next
blink color: read const seqKey =
textBlinkAnimation.colorSequence[textBlinkAnimation.currentColorIndex] and const
resolved = textBlinkAnimation.colors[seqKey], then only assign
textBlinkAnimation.color = resolved if resolved !== undefined; otherwise leave
the existing textBlinkAnimation.color unchanged (do not set it to undefined or
delete the property). This uses the existing identifiers
textBlinkAnimation.colorSequence, textBlinkAnimation.currentColorIndex,
textBlinkAnimation.colors and textBlinkAnimation.color to locate and fix the
code.
In `@packages/game-benchmark/src/games/mutation/miniplex/index.ts`:
- Around line 124-133: The iteration uses a type assertion on entity to read
position.x inside iterateSystem (sum += (entity as Entity & { position: { x:
number } }).position.x) which relies on all entities being created with position
(lines ~71-80); make this invariant explicit by adding a short comment above the
iterateSystem block (or immediately before the assertion) stating that every
entity produced by the benchmark always has a position property so the cast is
intentional and safe, mentioning the related queries/collections (queries,
shielded, poisonedEntities) to avoid surprising future readers.
In `@packages/game-benchmark/src/games/mutation/objecs/index.ts`:
- Around line 92-95: The factory map componentFactories currently types return
values as Entity[OptionalComponent] (which is optional) and relies on non-null
assertions; change the return type to guarantee non-null by using a
mapped/indexed type such as mapping over OptionalComponent (e.g. [K in
OptionalComponent]: () => NonNullable<Entity[K]>) so each factory returns a
concrete component value (not undefined), and remove any trailing `!` assertions
in the factory implementations to satisfy the "never undefined" guideline; keep
references to componentFactories, OptionalComponent and Entity when updating the
type.
In `@packages/objecs/vite.config.ts`:
- Around line 2-5: Add a short inline rationale immediately after the
oxlint-disable-next-line typescript/no-unsafe-assignment comment explaining why
the suppression is necessary (e.g., defineConfig's generic/third-party return
type causes a false positive and the assignment is safe), so future maintainers
know this isn't hiding a real type bug; reference the suppression and the
defineConfig usage in your note and keep it concise (one sentence) describing
the specific reason and that it was reviewed/intentional.
In `@packages/perf-proofs/src/array/for-of-vs-for-index-vs-foreach.ts`:
- Around line 12-15: Replace the local Entity type with one that extends the
repo's EntityBase so the benchmark uses the same entity layout as other code:
import EntityBase if necessary and declare type Entity = EntityBase & {
position: { x: number; y: number }; velocity: { x: number; y: number } }; ensure
both position and velocity remain plain JSON-compatible objects (no
functions/symbols) so the Entity shape used in for-of-vs-for-index-vs-foreach.ts
matches the ECS-style entities elsewhere.
In `@pnpm-workspace.yaml`:
- Around line 11-14: The catalog entries "vite", "vitest", and "vite-plus"
currently point to mutable tags (npm:`@voidzero-dev/vite-plus-core`@latest,
npm:`@voidzero-dev/vite-plus-test`@latest, vite-plus: latest); change each to a
pinned version string (e.g. replace "@latest" and "latest" with exact semver
versions like "1.2.3" or the commit SHA you verified) so the workspace toolchain
is deterministic, then run pnpm install to update the lockfile and commit the
updated pnpm-lock.yaml alongside the manifest change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8dd8e256-4465-4636-9d83-ff726809817a
⛔ Files ignored due to path filters (2)
packages/examples/public/js/feather.min.jsis excluded by!**/*.min.jspnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (101)
.claude/skills/vite-plus.oxfmtrc.json.oxlintrc.json.prettierrc.vite-hooks/pre-commit.vscode/settings.jsonAGENTS.mdCLAUDE.mdbenchmark.mdeslint.config.mjshierarchy-ideas.mdjekyll.config.ymlnetlify.tomlpackage.jsonpackages/benchmark/.oxfmtrc.jsonpackages/benchmark/.oxlintrc.jsonpackages/benchmark/.prettierrcpackages/benchmark/.vscode/settings.jsonpackages/benchmark/package.jsonpackages/benchmark/renovate.jsonpackages/ecs-benchmark/.github/workflows/main.ymlpackages/ecs-benchmark/.oxfmtrc.jsonpackages/ecs-benchmark/.oxlintrc.jsonpackages/ecs-benchmark/.prettierrcpackages/ecs-benchmark/package.jsonpackages/examples/.oxfmtrc.jsonpackages/examples/.oxlintrc.jsonpackages/examples/.prettierrcpackages/examples/index.htmlpackages/examples/package.jsonpackages/examples/public/css/demo.csspackages/examples/public/js/feather-init.jspackages/examples/src/demos/basic/index.htmlpackages/examples/src/demos/bouncy-rectangles/index.htmlpackages/examples/src/demos/debug-rendering/index.htmlpackages/examples/src/demos/debug-rendering/systems/sprite-animation-system.tspackages/examples/src/demos/falling-sand/index.htmlpackages/examples/src/demos/falling-sand/main.tspackages/examples/src/demos/pixel-text/index.htmlpackages/examples/src/demos/shmup/components/sprite-outline-animation.tspackages/examples/src/demos/shmup/components/text-blink-animation.tspackages/examples/src/demos/shmup/content.tspackages/examples/src/demos/shmup/enemy/switch-enemy-to-attach-mode.tspackages/examples/src/demos/shmup/index.htmlpackages/examples/src/demos/shmup/scene.tspackages/examples/src/demos/shmup/systems/destroy-boss-event-system.tspackages/examples/src/demos/shmup/systems/sprite-animation-system.tspackages/examples/src/demos/shmup/systems/sprite-outline-animation-system.tspackages/examples/src/demos/shmup/systems/sprite-outline-rendering-system.tspackages/examples/src/demos/shmup/systems/text-blink-animation-system.tspackages/examples/src/demos/sprite-animation/index.htmlpackages/examples/src/demos/sprite-tweening/index.htmlpackages/examples/src/demos/sprite-tweening/main.tspackages/examples/src/lib/canvas.tspackages/examples/src/lib/event-emitter.tspackages/examples/src/lib/pixel-text/renderer.tspackages/examples/src/style.csspackages/examples/src/vite-env.d.tspackages/examples/tsconfig.jsonpackages/examples/vite.config.tspackages/game-benchmark/.oxfmtrc.jsonpackages/game-benchmark/.oxlintrc.jsonpackages/game-benchmark/README.mdpackages/game-benchmark/package.jsonpackages/game-benchmark/src/games/ant-simulation/miniplex/index.tspackages/game-benchmark/src/games/ant-simulation/objecs/index.tspackages/game-benchmark/src/games/ant-simulation/systems/render-system.tspackages/game-benchmark/src/games/ant-simulation/systems/steering-system.tspackages/game-benchmark/src/games/boids/miniplex/index.tspackages/game-benchmark/src/games/boids/objecs/index.tspackages/game-benchmark/src/games/boids/systems/explosion-system.tspackages/game-benchmark/src/games/mutation/miniplex/index.tspackages/game-benchmark/src/games/mutation/objecs/index.tspackages/game-benchmark/src/index.tspackages/game-benchmark/src/profiler.tspackages/objecs/.oxfmtrc.jsonpackages/objecs/.oxlintrc.jsonpackages/objecs/.prettierrcpackages/objecs/README.mdpackages/objecs/package.jsonpackages/objecs/src/archetype.test.tspackages/objecs/src/archetype.tspackages/objecs/src/world.test.tspackages/objecs/src/world.tspackages/objecs/vite.config.tspackages/perf-proofs/.oxfmtrc.jsonpackages/perf-proofs/.oxlintrc.jsonpackages/perf-proofs/package.jsonpackages/perf-proofs/src/array/every-vs-for-loop.tspackages/perf-proofs/src/array/for-of-vs-for-index-vs-foreach.tspackages/perf-proofs/src/array/fused-every-some.tspackages/perf-proofs/src/iteration/iterator-protocol-overhead.tspackages/perf-proofs/src/map-set/map-vs-object-lookup.tspackages/perf-proofs/src/set/set-alloc-vs-alternatives.tspackages/perf-proofs/src/set/set-vs-array-iteration.tspackages/perf-proofs/src/set/small-set-vs-small-array.tsperformance-plan.mdpnpm-workspace.yamltasks.mdvite.config.tsworkspace.code-workspace
💤 Files with no reviewable changes (11)
- benchmark.md
- eslint.config.mjs
- packages/ecs-benchmark/.github/workflows/main.yml
- .prettierrc
- packages/examples/.prettierrc
- packages/benchmark/.prettierrc
- packages/objecs/.prettierrc
- packages/ecs-benchmark/.prettierrc
- performance-plan.md
- hierarchy-ideas.md
- tasks.md
| @@ -1,3 +1,21 @@ | |||
| { | |||
| "typescript.preferences.importModuleSpecifierEnding": "js" | |||
| "js/ts.preferences.importModuleSpecifierEnding": "js", | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
VS Code javascript.preferences.importModuleSpecifierEnding typescript.preferences.importModuleSpecifierEnding documentation
💡 Result:
These VS Code settings control how auto-imports (IntelliSense / Quick Fix import) choose the ending of generated module specifiers (e.g. whether to drop index and/or include the .js extension):
-
javascript.preferences.importModuleSpecifierEnding(JavaScript) — valid values:"auto"(default; uses projectjsconfig),"minimal"(drop/index.js→/),"index"(keep/index),"js"(include the.jsextension). [1]
-
typescript.preferences.importModuleSpecifierEnding(TypeScript) — same intent; docs describe:"auto","minimal"(e.g../component/index.js→./component),"index"(→./component/index),"js"(don’t shorten; include the.jsor.tsextension). [2]
Example workspace/user settings:
{
"javascript.preferences.importModuleSpecifierEnding": "js",
"typescript.preferences.importModuleSpecifierEnding": "js"
}VS Code also lists both settings among JS/TS preferences that can be configured per-language. [3]
Sources: [1] (code.visualstudio.com) [2] (github.com) [3] (code.visualstudio.com)
Citations:
- 1: https://code.visualstudio.com/updates/v1_44
- 2: https://github.com/microsoft/vscode/blob/master/extensions/typescript-language-features/package.nls.json
- 3: https://code.visualstudio.com/updates/v1_65%5C/
🏁 Script executed:
# Check if there's any VS Code settings reference or documentation in the repository
find . -type f -name "*.json" -o -name "*.md" | xargs grep -l "importModuleSpecifierEnding" 2>/dev/null | head -20Repository: jakeklassen/objecs
Length of output: 85
🏁 Script executed:
cat .vscode/settings.jsonRepository: jakeklassen/objecs
Length of output: 587
Use the documented JS and TS setting keys here.
VS Code documents separate javascript.preferences.importModuleSpecifierEnding and typescript.preferences.importModuleSpecifierEnding keys for this preference. The combined js/ts.preferences.importModuleSpecifierEnding entry is not recognized and will be ignored.
Suggested fix
- "js/ts.preferences.importModuleSpecifierEnding": "js",
+ "javascript.preferences.importModuleSpecifierEnding": "js",
+ "typescript.preferences.importModuleSpecifierEnding": "js",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "js/ts.preferences.importModuleSpecifierEnding": "js", | |
| "javascript.preferences.importModuleSpecifierEnding": "js", | |
| "typescript.preferences.importModuleSpecifierEnding": "js", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vscode/settings.json at line 2, The settings entry
"js/ts.preferences.importModuleSpecifierEnding" is not recognized by VS Code;
replace it with the documented keys
"javascript.preferences.importModuleSpecifierEnding" and
"typescript.preferences.importModuleSpecifierEnding" and set both to the same
value ("js") so the preference is applied for both JS and TS editors; update the
.vscode/settings.json entry that currently contains
"js/ts.preferences.importModuleSpecifierEnding" accordingly.
| - **Using the package manager directly:** Do not use pnpm, npm, or Yarn directly. Vite+ can handle all package manager operations. | ||
| - **Always use Vite commands to run tools:** Don't attempt to run `vp vitest` or `vp oxlint`. They do not exist. Use `vp test` and `vp lint` instead. | ||
| - **Running scripts:** Vite+ commands take precedence over `package.json` scripts. If there is a `test` script defined in `scripts` that conflicts with the built-in `vp test` command, run it using `vp run test`. | ||
| - **Do not install Vitest, Oxlint, Oxfmt, or tsdown directly:** Vite+ wraps these tools. They must not be installed directly. You cannot upgrade these tools by installing their latest versions. Always use Vite+ commands. | ||
| - **Use Vite+ wrappers for one-off binaries:** Use `vp dlx` instead of package-manager-specific `dlx`/`npx` commands. | ||
| - **Import JavaScript modules from `vite-plus`:** Instead of importing from `vite` or `vitest`, all modules should be imported from the project's `vite-plus` dependency. For example, `import { defineConfig } from 'vite-plus';` or `import { expect, test, vi } from 'vite-plus/test';`. You must not install `vitest` to import test utilities. | ||
| - **Type-Aware Linting:** There is no need to install `oxlint-tsgolint`, `vp lint --type-aware` works out of the box. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let me check the exact content of CLAUDE.md around the mentioned lines
head -n 250 CLAUDE.md | tail -n +220Repository: jakeklassen/objecs
Length of output: 1953
🌐 Web query:
oxlint-tsgolint type-aware linting requirement
💡 Result:
Oxlint’s type-aware linting is provided by the separate oxlint-tsgolint backend (tsgolint). To use it you must:
- Install the extra dependency:
oxlint-tsgolint@latest(dev dependency). [1] - Enable type-aware mode: run
oxlint --type-aware(or setoptions.typeAware: truein the root config). [1] - Have a TypeScript project it can build type info for (it constructs TS “programs” via
typescript-go). In practice that means it will use yourtsconfig.json(defaults to./tsconfig.json; you can pass a path with--tsconfig). [1], [2] - Meet the TypeScript compatibility requirement: the type-aware engine is powered by
typescript-goand requires TypeScript 7.0+; some legacytsconfigoptions may be unsupported. [1]
If you also want TypeScript compiler diagnostics during linting, add --type-check (or options.typeCheck: true). [1]
Sources: [1] Oxlint “Type-Aware Linting” docs, [2] Oxlint CLI docs.
🏁 Script executed:
# Check for oxlint-tsgolint in package.json
grep -A 5 -B 5 "oxlint-tsgolint" package.json 2>/dev/null || echo "Not found in package.json root"Repository: jakeklassen/objecs
Length of output: 308
🏁 Script executed:
# Check if there are multiple package.json files
find . -name "package.json" -type f | head -20Repository: jakeklassen/objecs
Length of output: 287
The type-aware linting guidance contradicts the actual setup and Oxc documentation.
The statement "There is no need to install oxlint-tsgolint" is inaccurate. Oxc's official documentation explicitly requires installing oxlint-tsgolint as a dev dependency for type-aware linting, and the repository already includes it (oxlint-tsgolint@0.16.0 in devDependencies). Either clarify that Vite+ automatically installs and manages oxlint-tsgolint as part of its type-aware setup, or revise the guidance to reflect the actual requirement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 228 - 234, The guidance contradicts Oxc docs: update
the bullet about "Type-Aware Linting" to state definitively whether
oxlint-tsgolint must be present or is auto-managed by Vite+; specifically either
(A) change the sentence to "Vite+ automatically installs and manages
oxlint-tsgolint for type-aware linting, so you do not need to add it manually"
or (B) change it to "Install oxlint-tsgolint as a devDependency (e.g.,
oxlint-tsgolint@0.16.0) to enable vp lint --type-aware", and ensure the term
"oxlint-tsgolint" and the command "vp lint --type-aware" are mentioned exactly
so readers know the dependency and the command to run.
| width: 640; | ||
| height: 480; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/examples/src/demos/basic/index.html | head -30Repository: jakeklassen/objecs
Length of output: 1040
🏁 Script executed:
# Also verify CSS spec on unitless values
web_searchRepository: jakeklassen/objecs
Length of output: 112
🌐 Web query:
CSS unitless width height values invalid specification
💡 Result:
In CSS, width/height do not accept unitless numbers. Their value syntax is auto | <length-percentage> | …, so values like width: 100; or height: 20; are invalid and get ignored by the browser (the declaration is dropped). [1]
The main exception people remember is unitless zero: 0 may be written without a unit (so width: 0; is valid). [2]
One subtlety: even though width: 0; is valid, 0 inside calc() is still a <number>, not a <length>, so width: calc(0 + 5px) is invalid (mixing <number> and <length>). [3]
Sources
[1] CSS Sizing Module Level 3 – width/height value definition. (w3.org)
[2] MDN <length> – unit optional for 0. (developer.mozilla.org)
[3] CSS Values & Units (example showing calc(0 + 5px) invalid). (w3.org)
Citations:
- 1: https://www.w3.org/TR/css-sizing-3/?utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/length?utm_source=openai
- 3: https://www.w3.org/TR/2019/WD-css-values-4-20190131/?utm_source=openai
🏁 Script executed:
grep -n "canvas" packages/examples/src/demos/basic/index.htmlRepository: jakeklassen/objecs
Length of output: 138
Add units to the canvas size CSS or remove these declarations.
width: 640 and height: 480 are invalid CSS and are ignored by the browser. The <canvas> element already has width="640" and height="480" HTML attributes that set the size correctly, so either delete these CSS rules or add units: 640px and 480px.
💡 Suggested fix
- width: 640;
- height: 480;
+ width: 640px;
+ height: 480px;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| width: 640; | |
| height: 480; | |
| width: 640px; | |
| height: 480px; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/examples/src/demos/basic/index.html` around lines 19 - 20, The CSS
block setting "width: 640;" and "height: 480;" is invalid and ignored by
browsers; update the CSS for the canvas (the rules containing width: 640 and
height: 480) to use explicit units (e.g. "width: 640px;" and "height: 480px;")
or remove those declarations entirely since the <canvas> element already has
width="640" and height="480" HTML attributes that determine its drawing size.
| // oxlint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
| const color = colors[colorSequence[currentColorIndex]!]!; | ||
| const frameRate = durationMs / 1_000 / colorSequence.length; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/examples/src/demos/shmup/components/text-blink-animation.tsRepository: jakeklassen/objecs
Length of output: 852
🏁 Script executed:
cd packages/examples && find . -type f -name "*.ts" -o -name "*.tsx" | head -20Repository: jakeklassen/objecs
Length of output: 1002
🏁 Script executed:
rg "textBlinkAnimationFactory" -A 5 -B 5Repository: jakeklassen/objecs
Length of output: 20132
Add defensive guards to prevent undefined component values.
Lines 14-16 suppress non-null assertions on array access, relying on implicit assumptions that colorSequence is non-empty and indices are in bounds. While all current callers satisfy these invariants, the function should guard its inputs to ensure the returned component never contains undefined values, per the guideline that components must be JSON-compatible and set explicitly (never undefined). If colorSequence is empty or contains out-of-range indices, color becomes undefined despite the NonNullable return type, and frameRate becomes Infinity.
💡 Proposed fix
export function textBlinkAnimationFactory({
colors,
colorSequence,
durationMs,
}: {
colors: HexColor[];
colorSequence: number[];
durationMs: number;
}): NonNullable<Entity["textBlinkAnimation"]> {
const delta = 0;
const currentColorIndex = 0;
- // oxlint-disable-next-line `@typescript-eslint/no-non-null-assertion`
- const color = colors[colorSequence[currentColorIndex]!]!;
+ const initialColorIndex = colorSequence[currentColorIndex];
+ if (initialColorIndex === undefined) {
+ throw new Error("textBlinkAnimationFactory requires a non-empty colorSequence");
+ }
+ const color = colors[initialColorIndex];
+ if (color === undefined) {
+ throw new Error(`textBlinkAnimationFactory received an invalid color index: ${initialColorIndex}`);
+ }
const frameRate = durationMs / 1_000 / colorSequence.length;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // oxlint-disable-next-line @typescript-eslint/no-non-null-assertion | |
| const color = colors[colorSequence[currentColorIndex]!]!; | |
| const frameRate = durationMs / 1_000 / colorSequence.length; | |
| const initialColorIndex = colorSequence[currentColorIndex]; | |
| if (initialColorIndex === undefined) { | |
| throw new Error("textBlinkAnimationFactory requires a non-empty colorSequence"); | |
| } | |
| const color = colors[initialColorIndex]; | |
| if (color === undefined) { | |
| throw new Error(`textBlinkAnimationFactory received an invalid color index: ${initialColorIndex}`); | |
| } | |
| const frameRate = durationMs / 1_000 / colorSequence.length; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/examples/src/demos/shmup/components/text-blink-animation.ts` around
lines 14 - 16, The code currently uses non-null assertions on
colorSequence/currentColorIndex and durationMs which can yield undefined color
or Infinity frameRate; update the logic in the text-blink-animation function to
defensively validate inputs: ensure colorSequence is non-empty (fallback to a
safe default sequence), clamp or validate currentColorIndex against
colorSequence.length so colors[colorSequence[currentColorIndex]] cannot be
undefined, and ensure durationMs and computed divisor (colorSequence.length) are
> 0 (fallback to a sensible positive value) before computing frameRate; replace
the "!" assertions with safe access and explicit defaults so the returned
component never contains undefined values.
| entities, | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
| // oxlint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-explicit-any | ||
| world: this.#world as any, | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
| // oxlint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-explicit-any | ||
| components: this.#components as any, | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
| // oxlint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-explicit-any | ||
| without: components as any, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/objecs/src/archetype.ts | head -200Repository: jakeklassen/objecs
Length of output: 5341
Preserve prior exclusions when chaining .without().
The without() method passes only the newly specified components to exclude, discarding prior exclusions. When chaining .without("b").without("c"), the resulting archetype remembers only "c" in its exclusion list, allowing entities with "b" to match. This breaks the expected semantic of successive filters.
Minimal fix
+ const excluding = [...(this.#excluding ?? []), ...components];
+
const archetype = new Archetype<
SafeEntity<
Omit<Entity, (typeof components)[number]>,
Exclude<Components[number], (typeof components)[number]>
>,
Array<Exclude<Components[number], (typeof components)[number]>>
>({
entities,
world: this.#world as any,
components: this.#components as any,
- without: components as any,
+ without: excluding as any,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| entities, | |
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | |
| // oxlint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-explicit-any | |
| world: this.#world as any, | |
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | |
| // oxlint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-explicit-any | |
| components: this.#components as any, | |
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | |
| // oxlint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-explicit-any | |
| without: components as any, | |
| const excluding = [...(this.#excluding ?? []), ...components]; | |
| const archetype = new Archetype< | |
| SafeEntity< | |
| Omit<Entity, (typeof components)[number]>, | |
| Exclude<Components[number], (typeof components)[number]> | |
| >, | |
| Array<Exclude<Components[number], (typeof components)[number]>> | |
| >({ | |
| entities, | |
| // oxlint-disable-next-line `@typescript-eslint/no-unsafe-assignment`, `@typescript-eslint/no-explicit-any` | |
| world: this.#world as any, | |
| // oxlint-disable-next-line `@typescript-eslint/no-unsafe-assignment`, `@typescript-eslint/no-explicit-any` | |
| components: this.#components as any, | |
| // oxlint-disable-next-line `@typescript-eslint/no-unsafe-assignment`, `@typescript-eslint/no-explicit-any` | |
| without: excluding as any, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/objecs/src/archetype.ts` around lines 146 - 152, The chained
.without() currently overwrites prior exclusions by passing only the new
components array (the line constructing the new archetype uses "without:
components as any"); modify the creation so it preserves and merges prior
exclusions with the newly-specified ones (e.g., combine this.#without with
components into a deduplicated list/set) before assigning to the new archetype's
without field so successive .without("b").without("c") retains both exclusions;
update the code around the Archetype cloning/creation (the method named without
/ the constructor call that sets world, components, without) to perform this
merge and dedupe.
|
|
||
| const map = new Map<string, number>(); | ||
| const obj: Record<string, number> = {}; | ||
| const obj: Record<string, number | undefined> = {}; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/perf-proofs/src/map-set/map-vs-object-lookup.tsRepository: jakeklassen/objecs
Length of output: 2594
Keep the known-hit object loop as a pure lookup.
Line 16's wider type is fine for the miss case, but line 58's obj[k] ?? 0 adds an extra nullish coalescing operation only on the object side. This measures a different operation than map.get(k)!, biasing the benchmark. Use obj[k]! instead to keep both sides as pure lookups, matching the pattern used correctly in the "set + get cycle" benchmark (line 85).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/perf-proofs/src/map-set/map-vs-object-lookup.ts` at line 16, The
object lookup in the "known-hit" loop should be a pure lookup to match the Map
side: replace the nullish coalescing use (obj[k] ?? 0) with a non-null assertion
(obj[k]!) so the benchmark measures the same pure read as map.get(k)!; update
the loop that compares obj and map lookups to use obj[k]! (keeping the existing
obj: Record<string, number | undefined> declaration).
| overrides: [ | ||
| { | ||
| files: ["**/*.{js,mjs,cjs,ts,mts,cts}"], | ||
| rules: { | ||
| "depend/ban-dependencies": "error", | ||
| }, | ||
| jsPlugins: ["eslint-plugin-depend"], | ||
| env: { | ||
| browser: true, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Oxlint jsPlugins language server LSP support documentation
💡 Result:
- JS Plugins (Oxlint
jsPlugins) docs: Oxlint can load ESLint-compatible JS plugins by listing them in.oxlintrc.jsonunderjsPlugins, then enabling their rules underrules(supports specifiers like./plugin.js,eslint-plugin-foo,@scope/eslint-plugin, plus optional aliases). [1] - LSP (Oxlint language server) usage: Editor integrations run Oxlint’s language server via
oxlint --lsp(sooxlintmust be installed in the project). [2] - LSP capabilities + configuration knobs (including config path): The
oxc_language_serverREADME documents supported LSP features (diagnostics, code actions likesource.fixAll.oxc, formatting foroxfmt) and workspace options such asconfigPath(to point at a specific oxlint config / control nested config behavior),typeAware, and more. [3]
Sources
[1] (oxc.rs)
[2] (oxc.rs)
[3] (github.com)
Citations:
- 1: https://oxc.rs/docs/guide/usage/linter/js-plugins
- 2: https://oxc.rs/docs/guide/usage/linter/editors
- 3: https://github.com/oxc-project/oxc/blob/main/crates/oxc_language_server/README.md
🌐 Web query:
Oxc configuration jsPlugins linter LSP editor integration
💡 Result:
Oxc (Oxlint) config: plugins vs jsPlugins
- Native (Rust) plugins: configure with
pluginsin.oxlintrc.json. Note that settingpluginsoverwrites the default plugin set (so include everything you want enabled). [2] - JavaScript plugins (
jsPlugins, experimental): add them underjsPluginsin.oxlintrc.json(string paths or objects with aliases). These are meant for ESLint-plugin compatibility / advanced integrations. [2], [3] - Important limitation:
jsPluginsare currently not supported in the language server (LSP). If you rely onjsPlugins, they may work via CLI linting but won’t be reflected through editor LSP diagnostics. [2]
Also: Oxlint config files are JSON-only (but support comments, i.e., JSONC-style). [4]
LSP + editor integration (diagnostics / fix-on-save)
- Editor extensions generally run
oxlint --lspfrom your project, sooxlintmust be installed locally in the repo (not just globally). [1] - VS Code / Cursor: install the official extension
oxc.oxc-vscode, and (optionally) enable fix-on-save via:.vscode/extensions.jsonrecommendations.vscode/settings.jsoneditor.codeActionsOnSave: { "source.fixAll.oxc": "always" }[1]
Practical takeaway
- If you need the best editor/LSP experience today: prefer native
pluginsfor rules you want surfaced in-editor. [2] - Use
jsPluginsonly when you specifically need JS/ESLint plugin compatibility—and expect that LSP/editor diagnostics won’t include those rules yet. [2], [3]
Sources:
[1] Oxc docs – Setup editors (Oxlint LSP + VS Code settings) (oxc.rs)
[2] Oxc docs – Oxlint configuration (plugins, jsPlugins, and LSP limitation) (oxc.rs)
[3] Oxc docs – JS Plugins guide (oxc.rs)
[4] Oxc docs – Config format notes (JSON w/ comments) (oxc.rs)
Citations:
- 1: https://oxc.rs/docs/guide/usage/linter/editors?utm_source=openai
- 2: https://oxc.rs/docs/guide/usage/linter/config?utm_source=openai
- 3: https://oxc.rs/docs/guide/usage/linter/js-plugins?utm_source=openai
- 4: https://oxc.rs/docs/guide/usage/linter/config.html?utm_source=openai
🌐 Web query:
Oxlint VS Code extension --lsp flag behavior
💡 Result:
The Oxc (Oxlint) VS Code extension starts Oxlint in Language Server (LSP) mode by always appending --lsp to whatever “oxlint binary” it resolved for your workspace (so Oxlint must be installed locally in the project for the extension to work as intended). [1]
Concretely, the extension launches one of these command shapes: [2]
- If the resolved “binary” looks like a Node script (e.g. ends in
.js/.cjs/.mjs, or is thenode_modules/<tool>/bin/<tool>style path), it runs:node <binaryPath> --lsp
- Otherwise it runs the binary directly:
<binaryPath> --lsp- On Windows it may run via a shell and quotes the path:
"${binaryPath}" --lsp
So: in the VS Code extension, --lsp is not a “lint option”; it’s the switch that makes Oxlint act as a long-running LSP server over stdio (and the extension relies on that). [1], [2]
Sources: [1] [2]
eslint-plugin-depend via jsPlugins will not report in editor LSP, only in CLI.
jsPlugins are not supported in Oxlint's language server. The depend/ban-dependencies rule will run in vp lint (CLI) but never appear in VS Code diagnostics, undermining the PR's goal to standardize workspace diagnostics.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite.config.ts` around lines 191 - 201, The current override uses jsPlugins:
["eslint-plugin-depend"] which the Oxlint language server ignores, so the
depend/ban-dependencies rule only appears in CLI; to fix, move the plugin into
ESLint's standard plugin configuration (e.g., add "plugins": ["depend"] inside
the same override or at top-level) and remove jsPlugins, ensure the rule
"depend/ban-dependencies" remains in the override's rules, and confirm the
plugin is installed (package.json) so VS Code LSP will load and surface the rule
diagnostics.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/objecs/package.json (1)
63-65: Pin concrete versions in the workspace catalog to improve reproducibility.Lines 63-65 reference
@latestaliases forviteandvitest. The workspace catalog inpnpm-workspace.yaml(lines 12-14) is the source of these moving targets. Update the catalog entries to use concrete versions instead of@latest, then ensure package manifests reference the catalog consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/objecs/package.json` around lines 63 - 65, The workspace is using moving targets for tool aliases: change the workspace catalog entries that define the catalog aliases for "vite" and "vitest" from using `@latest` to pinned concrete versions, then update the package manifest entries "vite" and "vitest" (and ensure "vite-plus" still references the catalog entry if intended) to reference those catalog keys consistently; specifically edit the catalog definitions (the catalog items that provided npm:`@voidzero-dev/vite-plus-core`@latest and npm:`@voidzero-dev/vite-plus-test`@latest) to use explicit semver or exact version strings and ensure the package.json entries ("vite", "vitest") no longer point to `@latest` but to the catalog alias or the concrete versions you pinned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/objecs/package.json`:
- Around line 63-65: The workspace is using moving targets for tool aliases:
change the workspace catalog entries that define the catalog aliases for "vite"
and "vitest" from using `@latest` to pinned concrete versions, then update the
package manifest entries "vite" and "vitest" (and ensure "vite-plus" still
references the catalog entry if intended) to reference those catalog keys
consistently; specifically edit the catalog definitions (the catalog items that
provided npm:`@voidzero-dev/vite-plus-core`@latest and
npm:`@voidzero-dev/vite-plus-test`@latest) to use explicit semver or exact version
strings and ensure the package.json entries ("vite", "vitest") no longer point
to `@latest` but to the catalog alias or the concrete versions you pinned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b68bf794-0277-4683-ac2b-7eff02a7e52c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/objecs/package.jsonpackages/objecs/tsup.config.tspackages/objecs/vite.config.ts
💤 Files with no reviewable changes (1)
- packages/objecs/tsup.config.ts
1 participant
Summary
vp lint/vp lint --fix.oxlintrc.json(withextends) and.oxfmtrc.json(full copies, no nested config support) for VS Code multi-root workspaceconsistent-type-definitionsandarray-type, allow numbers in template expressionsecs-benchmarkCI workflowTest plan
vp lintpasses with 0 errorsvp testpassesvp check --fix) runs successfully🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
Documentation