Skip to content

ci: add build/test pipeline and modernize the toolchain - #194

Merged
Eliav2 merged 9 commits into
masterfrom
ci/modern-pipeline
Aug 8, 2026
Merged

ci: add build/test pipeline and modernize the toolchain#194
Eliav2 merged 9 commits into
masterfrom
ci/modern-pipeline

Conversation

@Eliav2

@Eliav2 Eliav2 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Sets up CI for the first time. The repository had no workflows at all — .github/ contained only issue templates, so nothing has been built, linted or tested on push or PR since the project started.

Why this is more than a YAML file

The package could not be built from a clean checkout:

  • typescript was not in devDependencies at all, so tsc only worked on machines that happened to have it globally.
  • prepublishOnly ran pnpm run build, but there was no build script — only build:prod, build:dev and build:ts.
  • start and clear:lib were Windows-only (del /q, for /d %x, backslash paths), so no contributor on macOS or Linux could run the repo as documented.
  • The one existing test imported a component out of examples/ and had its only assertion commented out.

A workflow on top of that would have been red on the first run, so this PR fixes the underlying package before wiring the pipeline.

What is here

Toolchain

  • pnpm lockfile (the scripts already shelled out to pnpm), pinned via packageManager
  • ESLint 7 → ESLint 9 flat config with typescript-eslint 8
  • Prettier added, using the settings already declared in package.json. The deprecated jsxBracketSameLine key was being silently ignored; corrected to bracketSameLine so the existing JSX style is preserved
  • Jest 27 → Vitest, matching what the v3 branch already uses
  • Real script set: build, clean, type-check, lint, format, test

Workflows

  • ci.yml — format, lint, type-check, test, build across Node 22 and 24, then packs the tarball and asserts lib/index.js and lib/index.d.ts are present so a broken files field cannot reach npm unnoticed
  • publish.yml — publishes on a version change in package.json, using npm Trusted Publishing (no NPM_TOKEN secret, provenance attached automatically), then cuts the matching GitHub release. No-op when the version is already on the registry, so it is safe to re-run
  • dependabot.yml — monthly npm and github-actions updates, grouped

One source fix, because CI needs it

SVGGeometryElement.getTotalLength is not implemented by jsdom and is absent under SSR. Both call sites used optional chaining, which guards a null ref but not an element that exists without the method:

TypeError: lineRef.current?.getTotalLength is not a function

This is precisely why the project has no test suite — the component could not be mounted in one. Fixed with a small helper that falls back to 0. Closes #110.

Tests

Five smoke tests that actually mount the component, running against a bare jsdom with no SVG geometry polyfills, so the SSR-sensitive paths stay honest rather than being stubbed away.

Also included is a skipped regression test for the NaN geometry bug (#139, #171, #192), which reproduces reliably with curveness={0}:

d="M 16 16 C 16 16, NaN NaN, NaN NaN"
transform="translate(NaN,NaN) rotate(NaN) scale(24)"

It is skipped rather than deleted so the fix has a test already waiting for it.

Artifact diff against master

Master's src has moved on since 2.0.2 was published, so the npm tarball is the wrong baseline. Instead I built master HEAD with its own declared toolchain (TS 4.9 + @types/react 16) and diffed the emitted output against this branch, holding the compiler constant so source effects and toolchain effects could be told apart.

Typing — public surface unchanged. index.d.ts, Xarrow.d.ts, Xwrapper.d.ts, useXarrow.d.ts, constants.d.ts, anchors.d.ts, propTypes.d.ts, useXarrowProps.d.ts and privateTypes.d.ts are byte-identical. Three files change:

File Change Impact
types.d.ts typeof cPaths[number](typeof cPaths)[number] ×4 Prettier adding explicit parens. Proven identical types by bidirectional assignability.
utils/GetPosition.d.ts lineLength: anylineLength: number Strictly better. Not exported from the package entry.
utils/index.d.ts adds getTotalLength Additive. Not exported from the package entry.

Runtime — one intentional behaviour change. Comparing unminified tsc output (same compiler both sides) the entire delta is four items, three of which are provably equivalent:

Change Equivalent?
animateDrawing hoisted out of the destructure Yes — it was destructured then overwritten on the very next statement with no read in between
animDirection split out of a comma-declaration Yes — same var, same value, never reassigned
path / startAnchorPosition split into their own statements Yes — same plain object, same values, read immediately after
getTotalLength guard No, and deliberately so. In a browser both forms are identical; they differ only where the method is absent, where the old code threw

Xwrapper.js is byte-identical.

One caveat, stated plainly: the shipped bundle is compiled by TypeScript 5.9 rather than 4.9, and TS changed its own __importStar helper in between (it now enumerates with Object.getOwnPropertyNames instead of for...in). That is TypeScript's runtime helper rather than this project's code, but it is genuinely different bytes in the published file.

Merging this PR publishes nothing. version stays at 2.0.2. publish.yml will run on merge because package.json changed, but the already-published check short-circuits it before any publish step.

Verification

Cloned the branch fresh and ran the full pipeline locally, and CI is green on the branch:

step result
pnpm install --frozen-lockfile pass
format:check pass
lint pass (0 errors, 22 warnings)
type-check pass
test 5 passed, 1 skipped
build pass

Notes

  • The prefer-const sweep and Prettier pass touch src/. That is mechanical (auto-fix plus formatting) and is isolated in its own commits.
  • Remaining 22 lint warnings are pre-existing any usage and unused vars, deliberately left as warnings rather than silently disabled.
  • examples/ is untouched and still does not build (it has its own yarn.lock and a workspace: dependency with no workspace). Out of scope here; relates to Demo on codesandbox.io not working #172.
  • Bumping @types/react 16 → 18 surfaced a genuine type error in Xwrapper, fixed by declaring children explicitly.
  • Node 20 is not in the matrix: it reached end-of-life in April 2026 and jsdom 30 needs the undici that ships with Node 22+.
  • No engines field was added. The published artifact is ES5 UMD with no runtime Node requirement, and the package never declared one, so adding it would newly constrain consumers.

Vulnerability cleanup

webpack-dev-server 3 was the only source of every remaining audit finding, and it was dead weight: referenced solely by webpack.exampleConfig.js, which has been commented out of webpack.config.js for years. The examples app builds with react-scripts and never used it.

Removing it along with html-webpack-plugin, style-loader and css-loader takes pnpm audit from 27 findings (12 high, 13 moderate, 2 low) to zero, for both --prod and dev. The library build only ever used babel-loader, ts-loader and file-loader, so the bundle is unaffected.

Follow-ups this unblocks

  1. Merge the three ready PRs — Fix line SVG dotted animation in strict mode #174, Only import required lodash methods to reduce bundle size #175, Fixed undefined headRef when using animation without arrowheads #181 (12 lines, 3 issues)
  2. Fix the NaN geometry bug and unskip the waiting test
  3. Replace the UMD build with ESM + CJS, which is the real fix for the lodash bundle-size issue (Entirety of Lodash is Bundled (over 75% of package size) #118) since UMD cannot tree-shake

🤖 Generated with Claude Code

Eliav2 and others added 4 commits August 8, 2026 12:33
The package could not be built from a clean checkout: `typescript` was
missing from devDependencies entirely, and `prepublishOnly` invoked a
`build` script that did not exist.

- switch the lockfile to pnpm, matching the scripts that already shelled
  out to it, and pin the version via `packageManager`
- add the missing `typescript` dependency
- replace the deprecated ESLint 7 setup with ESLint 9 flat config and
  typescript-eslint 8
- add Prettier with the settings that were already declared in
  package.json, correcting the deprecated `jsxBracketSameLine` key to
  `bracketSameLine` so the existing JSX style is preserved
- give the package a real script set: build, clean, type-check, lint,
  format, test. The Windows-only `start` and `clear:lib` scripts are
  replaced with cross-platform equivalents.

`build` now reproduces exactly what was published for 2.0.2: a webpack
UMD bundle plus tsc-generated declarations.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SVGGeometryElement.getTotalLength is not implemented by jsdom, and is
absent under server-side rendering. Both call sites used optional
chaining, which guards a null ref but not an element that exists without
the method, so any attempt to render Xarrow in a test environment threw:

    TypeError: lineRef.current?.getTotalLength is not a function

This is why the project has no meaningful test suite - the component
could not be mounted in one. Adds a small helper that falls back to 0,
which leaves the draw animation inert rather than throwing.

Also splits two `let` destructuring blocks so that only the bindings that
are actually reassigned stay mutable, and declares `children` explicitly
on XarrowProvider now that React 18 types no longer imply it.

Fixes #110

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the previous placeholder test, which imported a component from
examples/ and had its only assertion commented out, with tests that
actually mount the component.

Runs against a bare jsdom with no SVG geometry polyfills, so the code
paths that fail under SSR stay covered rather than being stubbed away.

Includes a skipped regression test for the NaN geometry bug (#139, #171,
#192), which reproduces reliably with curveness={0}. It is skipped rather
than deleted so the fix has a test waiting for it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The repository had no CI at all - .github contained only issue
templates, so nothing was built, linted or tested on push or PR.

ci.yml runs format, lint, type-check, test and build across Node 20, 22
and 24, then packs the tarball and asserts the entry point and type
declarations are present, so a broken `files` field cannot reach npm
unnoticed.

publish.yml releases on a version change to package.json. It uses npm
Trusted Publishing, so no NPM_TOKEN secret is needed and provenance is
attached automatically. The run is a no-op when the version is already
on the registry, making it safe to re-run.

Also adds a monthly dependabot config for npm and github-actions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codesandbox

codesandbox Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Eliav2 and others added 5 commits August 8, 2026 12:36
Specifying both `version: 10` in the action and `packageManager` in
package.json makes the action abort with ERR_PNPM_BAD_PM_VERSION.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Node 20 reached end-of-life in April 2026, and jsdom 30 depends on the
undici build that ships with Node 22 or newer, so the test step fails
there with 'webidl.util.markAsUncloneable is not a function'.

Also drops the engines field. The published artifact is an ES5 UMD
bundle with no runtime Node requirement, and the package did not declare
engines before, so adding one would newly constrain consumers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The v4 actions run on the deprecated Node 20 runtime and are being
force-migrated by the runner.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Diffing the built artifact against master revealed that the Xwrapper edit
had also dropped `const log = console.log`. It is dead code, but removing
it was not intentional and Xarrow.tsx still has the same binding, so it
is restored to keep the runtime diff limited to the deliberate change.

Also collapses the CI matrix to a single Node 22 LTS job.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
webpack-dev-server, html-webpack-plugin, style-loader and css-loader were
only referenced by webpack.exampleConfig.js, which has been commented out
of webpack.config.js for years. The examples app builds with react-scripts
and does not use them either.

webpack-dev-server 3 was the sole source of every remaining audit finding,
so removing it takes the tree from 27 vulnerabilities (12 high, 13
moderate, 2 low) to zero. The library build is untouched - it only uses
babel-loader, ts-loader and file-loader.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Eliav2
Eliav2 merged commit 754058c into master Aug 8, 2026
1 check passed
@Eliav2
Eliav2 deleted the ci/modern-pipeline branch August 8, 2026 18:59
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.

.getTotalLength is not a function

1 participant