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

Fix class properties should have defaults as standard js classes would have #2409

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

mcfedr
Copy link
Contributor

@mcfedr mcfedr commented Jul 18, 2024

Fixes #2390

jsdom tries to replace some functions

in a normal js canvas, all the props are like standard js classes

this change adds the napi_property_attributes for this

also some InstanceValue are replaced with StaticValue as this is how they should have been

This has changed in the 3.x RC with the move to napi

  • Have you updated CHANGELOG.md?

Copy link
Collaborator

@chearon chearon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The descriptor changes look right 👍

src/Canvas.cc Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@mcfedr mcfedr force-pushed the props branch 2 times, most recently from a4898dc to 1e058c9 Compare August 13, 2024 08:11
@mcfedr mcfedr requested a review from chearon August 13, 2024 09:16
Copy link
Collaborator

@chearon chearon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looking good except it has changes from #2410

src/Canvas.cc Outdated Show resolved Hide resolved
…d have

Changed PNG consts to static properties
Copy link
Collaborator

@chearon chearon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@chearon chearon merged commit efcde93 into Automattic:master Sep 27, 2024
6 checks passed
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.

Problem with RC and jsdom
3 participants