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

ImageData not stringifying consistently on Node 14.7 #1646

Closed
brettz9 opened this issue Aug 11, 2020 · 12 comments · Fixed by #2214
Closed

ImageData not stringifying consistently on Node 14.7 #1646

brettz9 opened this issue Aug 11, 2020 · 12 comments · Fixed by #2214
Assignees

Comments

@brettz9
Copy link
Contributor

brettz9 commented Aug 11, 2020

Issue or Feature

In typeson-registry, we need to detect object type in order to properly serialize objects for JSON.

For some reason, on Node 14 in Linux (Xenial), this is failing, but it is not failing in Node 10 or 12 in Linux, nor in Node 14 on the Mac. This is failing on Node 14.7, but not in Node 10 or 12.

In performing some logging, it turns out that calling Object.prototype.toString.call() on ImageData objects will give [object Object] in Node 14.7, but in other environments, as expected, it gives [object ImageData].

Perhaps this is due to a proper fix in Node 14.7 since objects should only give [object Object] unless Symbol.toStringTag is used, but in any case, if it is not doing so already, I'd hope your build process could add the equivalent of the following to ImageData to ensure it behaves as in the browser so that one can detect the type of such objects (this approach will, unlike instanceof, also ensure detection continues to work cross-frame when used in polyglot browser code).

                get [Symbol.toStringTag] () {
                    return 'ImageData';
                }

Steps to Reproduce

const {ImageData} = require('canvas');
const imageData = new ImageData(1, 3);
console.log(Object.prototype.toString.call(imageData));
// Expected `[object ImageData]` but get `[object Object]` in Node 14 Linux

Your Environment

  • Version of node-canvas (output of npm list canvas or yarn list canvas): 2.6.1
  • Environment (e.g. node 4.2.0 on Mac OS X 10.8): Node 14.7 on Linux xenial or MacOSX; see the Travis job

Thanks!

@zbjornson
Copy link
Collaborator

This is probably related to #1639, which @asturur said is new since 14.7. Is you Mac running <14.7?

Think this is a straightforward fix I can work on in the coming week or two. Also happy to review a PR.

@brettz9
Copy link
Contributor Author

brettz9 commented Aug 11, 2020

Sorry, turns out I hadn't successfully switched nvm to Node 14. Am on a very slow/flaky connection, so can't really test Node 14 on the Mac locally again atm, but on Travis, it is giving the same [object Object] result as in Linux, so it appears it may indeed be a universal Node 14.7 problem.

Unfortunately, am not at all familiar with C/C++ (e.g., to see about adding Symbol.toStringTag in src/ImageData.*).

@brettz9 brettz9 changed the title ImageData not stringifying consistently with other envts in Node 14 on Linux ImageData not stringifying consistently on Node 14.7 Aug 11, 2020
@brettz9
Copy link
Contributor Author

brettz9 commented Feb 27, 2021

I'm on v14.15.5 (through nvm) and still getting the issue.

@brettz9
Copy link
Contributor Author

brettz9 commented Apr 13, 2021

@zbjornson : Any chance you'd be free to take a look?

@brettz9
Copy link
Contributor Author

brettz9 commented Dec 23, 2021

bump...

@zbjornson zbjornson self-assigned this Dec 31, 2021
@zbjornson
Copy link
Collaborator

Meant to make a PR but accidentally pushed the fix to master. Will be in the next release.

@brettz9
Copy link
Contributor Author

brettz9 commented Mar 20, 2022

@zbjornson : I'm still not seeing this work in the 2.9.1 release. Running Object.prototype.toString.call(new canvas.ImageData(1, 3)).slice(8, -1)) still gives "Object" instead of "ImageData".

@brettz9
Copy link
Contributor Author

brettz9 commented Mar 20, 2022

I think what the issue is in my case is that I am not running toString on the instance. I am using the class-type detection of Object.prototype.toString.call, which necessitates some code like:

                get [Symbol.toStringTag] () {
                    return 'ImageData';
                }

@brettz9
Copy link
Contributor Author

brettz9 commented Jan 1, 2023

Can you please reopen this? This is not fixed. Note that it is not a toString issue but toStringTag issue.

@chearon chearon reopened this Jan 3, 2023
@brettz9
Copy link
Contributor Author

brettz9 commented Mar 18, 2023

Is there anything I can help clarify about this topic to speed it along? This should I think really be a no-brainer as it is consistent with browser behavior and is a minor addition.

toStringTag may not be required of all interfaces in web specs, but as Node itself agreed by implementing nodejs/node#45987 recently for another API, keeping consistency with browsers in this regard is beneficial to users. (Though not always implementing as toStringTag, browsers do produce the same result, and in JavaScript, toStringTag is the way to do it at this time).

@zbjornson
Copy link
Collaborator

@brettz9 I've had almost no time for open-source work lately, but I'm happy to review a PR with a test if you want to make one.

@brettz9
Copy link
Contributor Author

brettz9 commented Apr 14, 2023

@zbjornson : I've submitted #2214 if you could take a look... Thanks!

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 a pull request may close this issue.

3 participants