Skip to content

chore: replace eslint with biome - #33

Merged
kurkle merged 1 commit into
chartjs:masterfrom
kurkle:biome
Sep 12, 2026
Merged

kurkle merged 1 commit into
chartjs:masterfrom
kurkle:biome

Conversation

@kurkle

@kurkle kurkle commented Sep 12, 2026

Copy link
Copy Markdown
Member

@etimberg — a proposal, independent of the Vitest one, and much smaller.

Why

eslint-config-chartjs is eslintrc-only and has not been released since 0.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.jsonc mirrors the eslint-config-chartjs stylistic rules — 2-space indent, single quotes, semicolons, {a, b} with no inner spacing, no trailing commas, and a 120 column width because max-len was off. So adopting the formatter is not also a restyling of the sources.

What it does change is mechanical and unavoidable: function() becomes function () (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 / noGlobalIsFiniteisNaN and Number.isNaN do not answer the same question. The matchers rely on the coercing global.
  • noPrototypeBuiltins — the code already uses the safe Object.prototype.hasOwnProperty.call form; the rule only wants the newer Object.hasOwn.

Kept on: everything in Biome's recommended set, plus useBlockStatements (eslint's curly: [2, all]), noConsole allowing warn/error (as eslint-config-chartjs did), and cognitive complexity as a warning — the closest analogue to complexity: [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

  • a redundant 'use strict' in an ES module,
  • assert imported without the node: protocol,
  • two assignments hidden inside expressions in specFromFixture (json.description || (json.description = description)), now plain if statements.

Verification

npm test (Biome + the existing mocha suite, 5 tests) and npm run build both 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 master and 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

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>
@kurkle
kurkle merged commit e30614a into chartjs:master Sep 12, 2026
1 check passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants