NapiClass constructor cannot be subclassed

[Jarred-Sumner]

Filing so I remember to fix this

This currently impacts @resvg/resvg-js

module.exports.Resvg = class Resvg extends _Resvg {
  constructor(svg, options) {
     super(svg, JSON.stringify(options))

The problem in Bun is this code:

   NapiClass* napi = jsDynamicCast<NapiClass*>(newTarget);
    if (UNLIKELY(!napi)) {
        JSC::throwVMError(globalObject, scope, JSC::createTypeError(globalObject, "NapiClass constructor called on an object that is not a NapiClass"_s));
        return JSC::JSValue::encode(JSC::jsUndefined());
    }

That !napi is being returned because newTarget is a subclass of NapiClass.

Maybe the prototype needs to be checked instead? I'm not sure.

[Jarred-Sumner]

I believe the fix here is, instead of jsDynamicCast, to check inherits and then do jsCast. I see that pattern in WebKit code a decent amount.

[yisibl]

I'm the author of resvg-js, thank you very much for bringing this to your attention. It would be great if this could be fixed before bun 1.0 is released.

[yisibl]

I tested bun v0.8.0 in macOS and it still doesn't work. @Jarred-Sumner Can you help and look at it again?

It won't run on a Mac M1 Pro or Mac Intel CPU.

[Jarred-Sumner]

Very strange. This is the regression test we run in CI which tries to catch this https://github.com/oven-sh/bun/blob/main/test/js/third_party/resvg/bbox.test.js

…

On Thu, Aug 24, 2023 at 2:32 AM 一丝 ***@***.***> wrote: I tested bun v0.8.0 in macOS and it still doesn't work. @Jarred-Sumner <https://github.com/Jarred-Sumner> Can you help and look at it again? [image: image] <https://user-images.githubusercontent.com/2784308/262929289-6e7b25bd-e185-4da3-8220-216e1ae53688.png> It won't run on a Mac M1 Pro or Intel CPU. — Reply to this email directly, view it on GitHub <#172 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFNGS4C7IED7YTOPWI2HX3XW4NRPANCNFSM5WEHIXNA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[yisibl]

I submitted a test repository for reference: https://github.com/yisibl/resvg-js-bun-test

[Jarred-Sumner]

The cause is a transpiler bug introduced after the napi-related parts were fixed

Input:

localFileExisted = existsSync(
  join(__dirname, "resvgjs.darwin-arm64.node")
);
try {
  localFileExisted
    ? (nativeBinding = require("./resvgjs.darwin-arm64.node"))
    : (nativeBinding = require("@resvg/resvg-js-darwin-arm64"));
} catch (e) {
  loadError = e;
}

Output:

localFileExisted = existsSync(join(__dirname, "resvgjs.darwin-arm64.node"));
try {
  localFileExisted
    ? (nativeBinding = () => ({}))
    : (nativeBinding = require("/path/to/resvgjs.darwin-arm64.node"));
} catch (e) {
  loadError = e;
}

[yisibl]

Thanks for the quick find of the cause of the problem

[yisibl]

Thanks, I confirmed the fix in bun v0.8.1