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

Prebuilt package throws "TypeError: Cannot read property 'call' of undefined" #1310

Closed
jamescryer opened this issue Nov 29, 2018 · 5 comments
Assignees

Comments

@jamescryer
Copy link

jamescryer commented Nov 29, 2018

Issue or Feature

I guess this is an issue with the pre-built fallback packages for certain environments. Also worth being aware that in my more complex application (not shown) I saw intermittent success of tests and functionality.

      TypeError: Cannot read property 'call' of undefined
          at setSource (node_modules/canvas/lib/image.js:102:14)
          at Image.set (node_modules/canvas/lib/image.js:68:7)
          at expect (nodejs-tests/canvas.test.js:19:39)
          at Object.test (nodejs-tests/canvas.test.js:19:53)

Steps to Reproduce

These simple Jest tests fail on my machine.

/* eslint-env jest*/
const fs = require("fs");
const util = require("util");
const canvas = require("canvas");
const readFile = util.promisify(fs.readFile);

describe("Canvas test", () => {
    test("loadImage", async () => {
        expect.assertions(1);
        const data = await readFile("./image.png");
        await expect(canvas.loadImage(data)).rejects.not.toBeTruthy();
    });
    test("Set Image.src", async () => {
        expect.assertions(1);
        const data = await readFile("./image.png");
        const image = new canvas.Image;
        await expect( () => { image.src = data;} ).not.toThrowError();
    });
});    

Your Environment

  • Version of node-canvas: 2.1.0 using prebuilt package (I haven't tried building locally)
  • Environment: node v10.14.0 on Windows 10 Enterprise
@LinusU
Copy link
Collaborator

LinusU commented Nov 29, 2018

Could you post the entire stack trace? Thanks

@jamescryer
Copy link
Author

@LinusU, updated, thanks!

@chearon
Copy link
Collaborator

chearon commented Nov 29, 2018

Are you sure the issue is with prebuilds? I just reproduced the same stack trace, then forced a manual build and then got the same results again. It's a weird error, since setSource seems to be a function

require('canvas/lib/bindings').Image.prototype.setSource
[Function: setSource]

@zbjornson
Copy link
Collaborator

(This is the same error as #1294 and #1250, both unresolved.)

I can finally repro this (thanks!), and it seems to be because of Jest 😞 . The following works with mocha:

const fs = require("fs");
const util = require("util");
const canvas = require("canvas");
const readFile = util.promisify(fs.readFile);
const {expect} = require("chai");

describe("Canvas test", () => {
    it("Set Image.src", async () => {
        const data = await readFile("./image.png");
        const image = new canvas.Image;
        await expect( () => { image.src = data;} ).to.not.throw();
    });
});

I think this is because Jest breaks node's guarantee that require()'d modules are loaded exactly once and returned from cache on subsequent require() calls. With jest, lib/Image.js tries to store a reference to function that was deleted the first time the file is loaded.

(I've complained about this to the Jest folks and they won't change the behavior. It's caused problems in most public repos I've worked on though.)

As much as I don't want to pander to Jest, we can move the setSource/getSource methods off of the prototype to avoid this, I think.


@chearon it's defined there in the native code, but deleted here in the JS:

delete proto.setSource;

@zbjornson zbjornson self-assigned this Nov 29, 2018
zbjornson added a commit to zbjornson/node-canvas that referenced this issue Nov 29, 2018
Jest re-evaluates modules, whereas `require` is only supposed to evaluate them once. Fix: make reloading lib/image.js safe.

Fixes Automattic#1310
Fixes Automattic#1294
Fixes Automattic#1250
zbjornson added a commit that referenced this issue Dec 2, 2018
Jest re-evaluates modules, whereas `require` is only supposed to evaluate them once. Fix: make reloading lib/image.js safe.

Fixes #1310
Fixes #1294
Fixes #1250
@LinusU
Copy link
Collaborator

LinusU commented Dec 3, 2018

Should be fixed in 2.2.0 🚀

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

No branches or pull requests

4 participants