Skip to content

Conversation

@asamuzaK
Copy link
Contributor

Another approach to fix #303
Closes #304

@asamuzaK asamuzaK force-pushed the case2 branch 4 times, most recently from 344b177 to 62b41a2 Compare January 24, 2026 11:14
@asamuzaK asamuzaK marked this pull request as draft January 24, 2026 23:07
@asamuzaK asamuzaK marked this pull request as ready for review January 25, 2026 00:37
@asamuzaK asamuzaK force-pushed the case2 branch 4 times, most recently from c4d1b16 to 2fb76b0 Compare January 25, 2026 01:47
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

This doesn't quite do what I suggested. It looks up the case-sensitivity of the property every time the setter is called, which is not necessary. Instead the case-sensitivity should be looked up at prepare-time, so that when we generate lib/generated/propertyDescriptors.js, the calls look like createGenericPropertyDescriptor("animation-name", { caseSensitive: true }).

@asamuzaK
Copy link
Contributor Author

Looking back at the PR, I wondered if it would be better to combine generatePropertyDefinitions.mjs and generatePropertyDescriptors.mjs into a single file.
WDYT?

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Looking back at the PR, I wondered if it would be better to combine generatePropertyDefinitions.mjs and generatePropertyDescriptors.mjs into a single file.
WDYT?

I think this is a good idea. It is very related to the discussion at #304 (comment).

But, let me merge this first as it is a good improvement by itself.

@domenic domenic merged commit 256e277 into jsdom:main Jan 28, 2026
3 checks passed
@asamuzaK asamuzaK deleted the case2 branch January 28, 2026 00:38
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.

Defect: 5.3.6+ forces animation-name to be lowercase

2 participants