Automattic / node-canvas

Node canvas is a Cairo backed Canvas implementation for NodeJS.
9.99k stars 1.15k forks source link

🚢 3.0.0 planning issue #2232

Open LinusU opened 1 year ago

LinusU commented 1 year ago

Things to consider for the 3.0.0 major version:

Please note that this issue is for discussing breaking changes and not a place for general feature requests!

chearon commented 1 year ago

Added Pango 2, which will solve a ton of font issues.

Should N-API be considered a breaking change? I plan on testing a lot, but it will introduce at least a few regressions.

LinusU commented 1 year ago

Added Pango 2, which will solve a ton of font issues.

Nice! 🙌

Should N-API be considered a breaking change? I plan on testing a lot, but it will introduce at least a few regressions.

Might be a good idea 👍

piranna commented 1 year ago

If we replace Canvas for OffscreenCanvas, can we get merged my PRs, specially https://github.com/Automattic/node-canvas/pull/1540 with the ScreenBackend class so we can have proper support for on-screen Canvas?

jimmywarting commented 10 months ago

can we instead remove the hole Image and http stuff with a createImageBitmap(blob)?

jimmywarting commented 10 months ago

why should OffscreenCanvas extend EventTarget?

aoor9 commented 7 months ago

Is v3.0.0 released? Because it doesn't seem to be available on npm registry.

LinusU commented 7 months ago

@zbjornson I saw that you pushed a v3.0.0 commit & tag to the repo, but we haven't published it to npm yet. The package.json still says that Node.js 10 is minimum version, and no other of the breaking changes are in yet.

Maybe we can add a few more breaking changes now before we re-tag and publish 3.0.0 to Npm? Or we consider 3.0.0 shipped and the breaking changes discussed above will go in 4.x...

I'll open some PRs which we could consider.

pinging @chearon also, do you have any input?

LinusU commented 7 months ago

why should OffscreenCanvas extend EventTarget?

According to MDN, OffscreenCanvas in the browser extends EventTarget? https://developer.mozilla.org/en-US/docs/Web/API/OffscreenCanvas

alumni commented 7 months ago

Seems that the Mac build and the Windows tests are failing after the N-API refactor, maybe those would need to be fixed first in order to publish the package? :)

chearon commented 7 months ago

It's not published to NPM because the Windows prebuilds aren't working (and neither @zbjornson or I have looked - that's always a huge time sink). package.json node got changed in #2235, so that technically is a breaking change.

zbjornson commented 7 months ago

Specifically, the NAPI port means that prebuilds need to be reworked (https://github.com/Automattic/node-canvas/pull/2235#issuecomment-1747461108) and I haven't had time to do that.

In the meantime, I tried to make Node.js v21 prebuilds for the latest 2.x version, but those failed on Windows as Caleb said. https://github.com/Automattic/node-canvas/issues/2295#issuecomment-1783294105

As far as more changes, I favor deferring other changes to 4.x+ since the NAPI port unblocks using node-canvas in Bun and that work is done.

LinusU commented 7 months ago

As far as more changes, I favor deferring other changes to 4.x+ since the NAPI port unblocks using node-canvas in Bun and that work is done.

I'm all for releasing early! So I don't think we should block on anything else, but e.g. just bumping our minimum supported Node.js version (#2310) I think would be really nice to get in just so that we don't have to support old versions of Node.js that probably aren't used much, since they are end of life.

Are you two open on merging breaking changes until the prebuilds are fixed and we are ready to publish to npm? How do you feel about raising supported version as I did in #2310?

zbjornson commented 7 months ago

Are you two open on merging breaking changes until the prebuilds are fixed and we are ready to publish to npm?

Good with me as long as they don't make the current prebuild troubles worse :)

How do you feel about raising supported version as I did in #2310?

👍

LinusU commented 7 months ago

I've fixed the tests and increased the minimum version of Node.js.

Since the Node.js version is increased now, I don't think that #2309 is a breaking change anymore so no need to rush that.

Pango 2 doesn't seem to be released yet, so that change will not make it I guess.

I would have loved to land Canvas -> OffscreenCanvas, and the new Image & ImageData. But since that will probably affect the native code a bit it might make it harder to work on the prebuilds? And it might also take some time...

If I have time, I could implement those in a branch and then we could hold off on merging until prebuilds are done and we see that they work together...


We also need to update the v3.0.0 tag, which solution do you think would work best?

1️⃣ I move the v3.0.0 commit using git rebase and then force push to master 2️⃣ We create a new empty commit that will be the tagged v3.0.0 commit 3️⃣ We point the v3.0.0 tag to whatever commit is the latest when we want to release v3.0.0 (probably ad793da)

If we don't think it will cause too many issues, I kind of prefer 1️⃣, just so that every release to npm matches up with a commit that bumps the version. This is probably something that should be enforced using Npm package provenance in the future...

YuMuuu commented 6 months ago

The v3.0.0 release needs to merge #2309. When will you plan to merge it?

quinton-ashley commented 5 months ago

Is v3 done yet?