Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

limit numerical precision of test snapshots; fix node 22 #2234

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mbostock
Copy link
Member

This limits numerical precision to three fractional digits, similar to what we already do via d3-shape and d3-path (ref. #1193 #1143). Reduces the test snapshots from 58M to 51M. It looks like one test (projectionClipAngle) is still broken, I would guess to some other numerical instability that will be harder to fix. So, I may just not worry about having the test snapshots work on both Node 20 and Node 22, and only test against 22.

Related, it would be nice if Plot limited the precision of numeric attributes and numbers interpolated into templates by default, since we already do this for path data.

@mbostock mbostock requested a review from Fil November 15, 2024 01:11
@mbostock mbostock marked this pull request as draft November 15, 2024 01:45
@mbostock
Copy link
Member Author

mbostock commented Nov 15, 2024

This is now blocked by Automattic/node-canvas#2448.

I made some pretty good progress switching to skia-canvas instead, and using module-alias to point JSDOM at skia-canvas instead of canvas. But it looks like image rendering (raster mark) still isn’t working, so that hit a dead end.

Then I looked at upgrading to canvas 3.0.0-rc2, which supports Node 22, but that also changes the API in a way that breaks JSDOM Automattic/node-canvas#2390. In fact they fixed it in Automattic/node-canvas#2409, but they haven’t released a new release candidate yet, so there’s not currently a way to install a version that’s compatible with both Node 22 and JSDOM. 😓

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.

1 participant