chore: replace eslint with biome - #33
Merged
Merged
Conversation
eslint-config-chartjs is eslintrc-only and has not been released since 0.3.0,
so this repo is pinned to eslint 8: flat config became mandatory in eslint 9
and the eslintrc escape hatch is gone in eslint 10. Biome needs no plugins for
what this repo used eslint-plugin-es for, formats as well as lints, and the
config lives here rather than in a shared package -- one file to read, and
nothing to keep in sync across releases.
The formatter is configured to the style rules `eslint-config-chartjs` already
enforced (2-space indent, single quotes, semicolons, `{a, b}` without inner
spacing, no trailing commas), so adopting it is not also a restyling. What it
does change in the sources is mechanical: `function()` becomes `function ()`,
which Biome has no option for, plus some line wrapping and blank-line trimming.
Rules that would have meant rewriting the ES5-era sources -- `var` inside
blocks, `arguments`, string concatenation, callbacks that could be arrow
functions -- are turned off with the reason written next to them, so that
modernizing stays a deliberate code change rather than a side effect of
switching tools. Two rules are off because the code is already right:
`isNaN` and `Number.isNaN` do not answer the same question, and the sources
already use the safe `Object.prototype.hasOwnProperty.call` form.
Three findings were fixed by hand rather than silenced: the redundant
`'use strict'` in an ES module, `assert` imported without the `node:` protocol,
and two assignments hidden inside expressions in `specFromFixture`.
CI moves to Node 24 with current action versions; it was pinned to Node 14,
which no runner should be asked for in 2026. `npm test` (lint + mocha) and
`npm run build` both pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
etimberg
approved these changes
Sep 12, 2026
kurkle
added a commit
to kurkle/chartjs-test-utils
that referenced
this pull request
Sep 12, 2026
Rebased on master now that chartjs#33 has landed. The mechanical part of the rebase kept this branch's files; this commit is the part that is not mechanical. - eslint and eslint-config-chartjs are gone from `devDependencies`, `lint` and `format` are Biome, and the `eslint-disable` pragmas in the new sources are gone: two for `callback-return`, a rule Biome does not have, and two for `no-console`, now `biome-ignore lint/suspicious/noConsole` with the reason on the same line -- a reason wrapped onto the next line suppresses nothing and reports itself as an unused suppression. - `biome.jsonc` lints `.ts` and `.mjs` too, so the Vitest configs and the fixture script are covered. - The rule exceptions chartjs#33 needed for the ES5-era sources are lifted: `useArrowFunction`, `noArguments`, `useOptionalChain`, `useTemplate` and `noInnerDeclarations` are back on Biome's recommended settings, because the rewrite has no `var`-in-block, `arguments` or string concatenation left. `src/spriting.js` keeps `useOptionalChain` off in an `overrides` block: it is a port of the 0.5.0 sprite sheet, and rewriting `text && text.charCodeAt` as `text?.charCodeAt` is equivalent only because the loops iterate over `text.length`. - With those rules on, Biome found four real things in the new code, all fixed rather than silenced: three `forEach` callbacks whose concise arrow bodies returned a value (now `for...of`), the `chart.$test || {}` guards (now optional chaining), a `var me = this` left useless once the mock context's method wrappers became arrows, and nine string concatenations in the matcher messages -- which the unit specs assert verbatim, so they are covered. - `recommended: true` is deprecated in Biome 2.5; it is now `preset: "recommended"`. `biome check` is clean with no warnings. The suites still pass: 7 node specs, 24 browser specs plus the 2 deliberate skips, and chartjs-chart-treemap's browser suite -- 142 tests across Chromium and Firefox -- still passes against this package with no reference image regenerated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kurkle
added a commit
that referenced
this pull request
Sep 12, 2026
* feat!: replace Karma and Jasmine with Vitest
Karma was deprecated in 2023, and most of what this package did existed to
work around it: scanning `__karma__.files` to find fixtures, reading every
fixture config back over `XMLHttpRequest`, and registering matchers through
`jasmine.addMatchers`. None of it has a counterpart in a bundler-driven
runner, so v1 drops the Karma and Jasmine peers instead of keeping a second
entry point alive beside them. Consumers still on Karma stay on 0.5.x.
What the rendering rules do is unchanged, and so are the reference images
captured with them: the sprite sheet, the wrapper CSS, `devicePixelRatio = 1`
and the pixelmatch comparison all behave as before. pixelmatch moves 5 -> 7,
where `checkerboard` blending became the default in 7.2.0; that is a different
measurement rather than a stricter one, so the matcher keeps blending against
white and a fixture opts into the checkerboard per comparison.
Notable changes:
- `setup({Chart})` takes the Chart.js constructor instead of reading a global.
Karma loaded the UMD bundle into `window`, a bundler does not.
- `createFixtures({configs, images})` takes the resolved file maps, because
`import.meta.glob` resolves against the file the literal pattern is written
in. The glob has to stay in the consumer; only the map can move here.
- `pending()` becomes `ctx.skip()`, so `useShadowDOM` and `useOffscreenCanvas`
need the test context passed to `acquireChart`.
- Fixture images are rewritten by a `saveFixtureImage` browser command
(`chartjs-test-utils/node`), registered only when updating. The suite detects
the mode from the command's presence rather than a `define` flag, which
Vitest re-encodes: `JSON.stringify(false)` arrives as the truthy string
"false" and every fixture quietly rewrites itself while reporting a pass.
- The package publishes its sources instead of a rollup bundle, so pixelmatch
resolves as a normal dependency rather than being inlined.
- The package now tests itself: node specs for the option matcher and the mock
context, and a browser suite that renders two fixtures in Chromium and
Firefox. CI installs both browsers and runs lint, typecheck and both suites.
Verified against a real suite: chartjs-chart-treemap's browser tests (63 pixel
fixtures plus the controller specs, 142 tests across both browsers) pass with
its `test/utils` replaced by this package and no reference image regenerated.
That run is what caught the sprite sheet being decoded lazily, which silently
dropped text from the first fixture that drew any.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BREAKING CHANGE: Karma and Jasmine are no longer supported. The package
requires Vitest, `setup({Chart})` must be called from a setup file, and
`specsFromFixtures` is now built by `createFixtures`.
* chore: adopt biome for the Vitest sources
Rebased on master now that #33 has landed. The mechanical part of the rebase
kept this branch's files; this commit is the part that is not mechanical.
- eslint and eslint-config-chartjs are gone from `devDependencies`, `lint` and
`format` are Biome, and the `eslint-disable` pragmas in the new sources are
gone: two for `callback-return`, a rule Biome does not have, and two for
`no-console`, now `biome-ignore lint/suspicious/noConsole` with the reason on
the same line -- a reason wrapped onto the next line suppresses nothing and
reports itself as an unused suppression.
- `biome.jsonc` lints `.ts` and `.mjs` too, so the Vitest configs and the
fixture script are covered.
- The rule exceptions #33 needed for the ES5-era sources are lifted:
`useArrowFunction`, `noArguments`, `useOptionalChain`, `useTemplate` and
`noInnerDeclarations` are back on Biome's recommended settings, because the
rewrite has no `var`-in-block, `arguments` or string concatenation left.
`src/spriting.js` keeps `useOptionalChain` off in an `overrides` block: it is
a port of the 0.5.0 sprite sheet, and rewriting `text && text.charCodeAt` as
`text?.charCodeAt` is equivalent only because the loops iterate over
`text.length`.
- With those rules on, Biome found four real things in the new code, all fixed
rather than silenced: three `forEach` callbacks whose concise arrow bodies
returned a value (now `for...of`), the `chart.$test || {}` guards (now
optional chaining), a `var me = this` left useless once the mock context's
method wrappers became arrows, and nine string concatenations in the matcher
messages -- which the unit specs assert verbatim, so they are covered.
- `recommended: true` is deprecated in Biome 2.5; it is now `preset:
"recommended"`.
`biome check` is clean with no warnings. The suites still pass: 7 node specs,
24 browser specs plus the 2 deliberate skips, and chartjs-chart-treemap's
browser suite -- 142 tests across Chromium and Firefox -- still passes against
this package with no reference image regenerated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
@etimberg — a proposal, independent of the Vitest one, and much smaller.
Why
eslint-config-chartjsis eslintrc-only and has not been released since0.3.0. Flat config became mandatory in ESLint 9 and the eslintrc escape hatch is gone in ESLint 10, so this repo is effectively pinned to ESLint 8 until that shared config is rewritten.eslint-plugin-es(used here only to forbid post-ES2019 syntax) has the same problem.Biome needs no plugins for what this repo asked of eslint, formats as well as lints, and runs the whole repo in ~6 ms. The config lives in this repository rather than in a shared package: one file to read, nothing to keep in sync across two release cycles, and no third repo to update before this one can move.
The formatter is configured to the style you already had
This is the part worth reviewing.
biome.jsoncmirrors theeslint-config-chartjsstylistic rules — 2-space indent, single quotes, semicolons,{a, b}with no inner spacing, no trailing commas, and a 120 column width becausemax-lenwas off. So adopting the formatter is not also a restyling of the sources.What it does change is mechanical and unavoidable:
function()becomesfunction ()(Biome, like Prettier, has no option for this), plus some line wrapping and blank-line trimming. That is the bulk of the diff.Rules that are off, and why
Each one carries its reason in the config file:
useArrowFunction,noArguments,useTemplate,useOptionalChain,useDateNow,noInnerDeclarations— these report the ES5 style the sources are written in (var,function () {}callbacks,arguments, string concatenation). Modernizing is a code change, not a tooling change, and mixing the two would make this diff unreviewable.noGlobalIsNan/noGlobalIsFinite—isNaNandNumber.isNaNdo not answer the same question. The matchers rely on the coercing global.noPrototypeBuiltins— the code already uses the safeObject.prototype.hasOwnProperty.callform; the rule only wants the newerObject.hasOwn.Kept on: everything in Biome's recommended set, plus
useBlockStatements(eslint'scurly: [2, all]),noConsoleallowingwarn/error(aseslint-config-chartjsdid), and cognitive complexity as a warning — the closest analogue tocomplexity: [2, 10], though it is a different measure, so the threshold is not the same number. It reports the same four functions the old setup warned about.Three findings fixed by hand rather than silenced
'use strict'in an ES module,assertimported without thenode:protocol,specFromFixture(json.description || (json.description = description)), now plainifstatements.Verification
npm test(Biome + the existing mocha suite, 5 tests) andnpm run buildboth pass, so the reformatting did not change behaviour of anything the tests cover.CI moves to Node 24 with current action versions. It was pinned to Node 14, which no runner should be asked for in 2026 — and while Biome declares support down to 14, I have not run it there and would rather not find out in CI.
Relationship to the other PR
This branches from
masterand is independent of #34 (the Vitest migration): either can be merged first. They touch the same files, so the second one will need a rebase, which I am happy to do.🤖 Generated with Claude Code