Update CSSStyleDeclaration and export property related data #244

asamuzaK · 2025-09-08T14:03:22Z

Part of #235
Rebased #243, closes #242.

Spec: https://drafts.csswg.org/cssom/#the-cssstyledeclaration-interface

Breaking change: Arguments to new CSSStyleDeclaration() have changed.
- The first argument is a Window.
- The second argument is an options object.
  - The on change callback previously received as the first argument should be included in the options object using onChange key.
```
const node = document.createElement("div");
const callback = cssText => {
  console.log(cssText);
};
const style = new CSSStyleDeclaration(window, {
  onChange: callback
});
```
Switched main entry point from ./lib/CSSStyleDeclaration.js to ./lib/index.js.
- Exporting one class and two pieces of data from index.js.
  - CSSStyleDeclaration
  - propertyDefinitions
    - An object of properties and their definitions.
  - propertyNames
    - List of property names.
Updated dependencies and devDependencies.

lib/webidl/CSSStyleDeclaration.webidl

asamuzaK · 2025-09-19T11:55:20Z

Back to draft, will enable after jsdom@27 issues are addressed.

asamuzaK · 2025-10-03T23:36:21Z

Ready for review.

domenic

Have you tested this with a local fork of jsdom yet? It's probably best to merge and release only once we know it works.

lib/utils/cache.js

lib/CSSStyleDeclaration.js

scripts/downloadLatestProperties.mjs

scripts/generateProperties.js

test/properties.test.js

asamuzaK · 2025-11-24T11:57:22Z

Done most of what we can do now. Please review.

lib/CSSStyleDeclaration.js

domenic · 2025-12-03T07:20:02Z

lib/CSSStyleDeclaration.js

   */
-  getPropertyPriority(property) {
-    return this._priorities.get(property) || "";
+  item(...args) {


All this boilerplate should not be necessary, the webidl2js layer should be able to take care of it. We can just reduce this to item(index) { return this[index]; }.

Unfortunately, making such a change introduced a regression.

Can you explain in more detail?

If we change item(index) like below:

item(index) { return this[index]; }

jsdom ci:

> npm run test:tuwpt -- --fgrep css/cssom 56 passing (5s) 3 failing 1) Local tests in web-platform-test format (to-upstream) css/cssom/inline-style.html: Error: 1/3 errors in test: Failed in "StylePropertyReflectsStyleAttribute": assert_equals: item-2 expected (string) "" but got (undefined) undefined at Test.<anonymous> (http://web-platform.test:10000/css/cssom/inline-style.html:31:3) at Test.step (http://web-platform.test:10000/resources/testharness.js:2642:25) at test (http://web-platform.test:10000/resources/testharness.js:633:30) at http://web-platform.test:10000/css/cssom/inline-style.html:22:1 at Script.runInContext (node:vm:149:12) at C:\Users\XXX\jsdom\test\web-platform-tests\run-single-wpt.js:222:22 at http://web-platform.test:10000/resources/testharness.js:3836:22 at forEach (http://web-platform.test:10000/resources/testharness.js:4663:26) at Tests.notify_complete (http://web-platform.test:10000/resources/testharness.js:3833:9) at all_complete (http://web-platform.test:10000/resources/testharness.js:3699:22) at Tests.complete (http://web-platform.test:10000/resources/testharness.js:3711:13) at http://web-platform.test:10000/resources/testharness.js:97:21 at Timeout.task [as _onTimeout] (lib\jsdom\browser\Window.js:603:19) at listOnTimeout (node:internal/timers:605:17) at process.processTimers (node:internal/timers:541:7) 2) Local tests in web-platform-test format (to-upstream) css/cssom/style-item.html: Error: 2/5 errors in test: Failed in "CSSStyleRule.style.item()": assert_equals: item 2 expected (string) "" but got (undefined) undefined at Test.<anonymous> (http://web-platform.test:10000/css/cssom/style-item.html:18:3) at Test.step (http://web-platform.test:10000/resources/testharness.js:2642:25) at test (http://web-platform.test:10000/resources/testharness.js:633:30) at http://web-platform.test:10000/css/cssom/style-item.html:14:1 at Script.runInContext (node:vm:149:12) Failed in "Element.style.item()": assert_equals: item 2 expected (string) "" but got (undefined) undefined at Test.<anonymous> (http://web-platform.test:10000/css/cssom/style-item.html:33:3) at Test.step (http://web-platform.test:10000/resources/testharness.js:2642:25) at test (http://web-platform.test:10000/resources/testharness.js:633:30) at http://web-platform.test:10000/css/cssom/style-item.html:28:1 at Script.runInContext (node:vm:149:12) at C:\Users\XXX\jsdom\test\web-platform-tests\run-single-wpt.js:222:22 at http://web-platform.test:10000/resources/testharness.js:3836:22 at forEach (http://web-platform.test:10000/resources/testharness.js:4663:26) at Tests.notify_complete (http://web-platform.test:10000/resources/testharness.js:3833:9) at all_complete (http://web-platform.test:10000/resources/testharness.js:3699:22) at Tests.complete (http://web-platform.test:10000/resources/testharness.js:3711:13) at http://web-platform.test:10000/resources/testharness.js:97:21 at Timeout.task [as _onTimeout] (lib\jsdom\browser\Window.js:603:19) at listOnTimeout (node:internal/timers:605:17) at process.processTimers (node:internal/timers:541:7) 3) Local tests in web-platform-test format (to-upstream) css/cssom/style-set-custom-property.html: Error: Failed in "Setting value for custom property": assert_equals: style.item(1) is empty string expected (string) "" but got (undefined) undefined at Test.<anonymous> (http://web-platform.test:10000/css/cssom/style-set-custom-property.html:11:3) at Test.step (http://web-platform.test:10000/resources/testharness.js:2642:25) at test (http://web-platform.test:10000/resources/testharness.js:633:30) at http://web-platform.test:10000/css/cssom/style-set-custom-property.html:4:1 at Script.runInContext (node:vm:149:12) at C:\Users\XXX\jsdom\test\web-platform-tests\run-single-wpt.js:220:22 at http://web-platform.test:10000/resources/testharness.js:3836:22 at forEach (http://web-platform.test:10000/resources/testharness.js:4663:26) at Tests.notify_complete (http://web-platform.test:10000/resources/testharness.js:3833:9) at all_complete (http://web-platform.test:10000/resources/testharness.js:3699:22) at Tests.complete (http://web-platform.test:10000/resources/testharness.js:3711:13) at http://web-platform.test:10000/resources/testharness.js:97:21 at Timeout.task [as _onTimeout] (lib\jsdom\browser\Window.js:603:19) at listOnTimeout (node:internal/timers:605:17) at process.processTimers (node:internal/timers:541:7)

Interesting. I guess probably it can be reduced to

if (index < 0 || index >= this._length) { return ""; } return this[index];

We need an error handler if no arguments are given.

It is very strange the generated item() in the wrapper class does not handle that...

domenic · 2025-12-03T07:21:29Z

lib/CSSStyleDeclaration.js

+   *
+   * @param {number} len
+   */
  set length(len) {


This setter isn't necessary for jsdom, nor is it in the spec. Can we delete it? Or are there internal uses of some sort? Probably those could be replaced...

I haven't made any changes to this setter, and it's been like this for a while. It's called by Array.prototype.splice.call() in _clearProperties() and _removeIndex(). I added a comment in JSDoc.
a9be7b4

I guess this and the index() might need to be cleaned up in a follow-up. Overall it would be nicer if we did not include the indexed properties on the public API, and let the webidl2js layer do them instead.

lib/CSSStyleDeclaration.js

domenic · 2025-12-03T07:25:34Z

lib/CSSStyleDeclaration.js

+  }
+}
+
+// Define standard CSS properties on the prototype from the generated properties map.


I thought the plan was not to include these properties on CSSStyleDeclaration, just like the spec. Instead we handle them in the jsdom layer.

The CSS specification has changed in a confusing way.
The previous CSSStyleDeclaration has been split, and the current CSSStyleProperties is the previous CSSStyleDeclaration.
Current specification:
https://drafts.csswg.org/cssom/#the-cssstyledeclaration-interface
Previous specification:
https://www.w3.org/TR/2021/WD-cssom-1-20210826/#the-cssstyledeclaration-interface

cssstyle.CSSStyleDeclaration is the implementation of current CSSStyleProperties.

If you find this confusing, we can rename it to cssstyle.CSSStyleProperties.
In any case, cssstyle.CSSStyleDeclaration (or cssstyle.CSSStyleProperties) need to be exported as a single object.

My proposal was that this package implements the current CSSStyleDeclaration. And we implement everything related to CSSStyleProperties in the jsdom layer.

That is, we remove from this package every public API that exposes individual properties from this package, and instead only expose cssText, length, item(), getPropertyValue(), getPropertyPriority(), setProperty(), removeProperty(), and parentRule.

Then, in jsdom, we can add a bunch of individual properties like cssFloat or backgroundImage or whatever, which delegate to getPropertyValue() and setProperty(). Like the spec says.

Is that possible to implement? I thought that was the major goal of this PR. But now I am unsure what the goal of this PR is.

If that's the case, I don't think there's much point in making cssstyle a separate module.
In other words, that would mean porting the whole of cssstyle to jsdom.

If you want to port cssstyle to jsdom, I'll go along with that, but I'd like to proceed in stages even if we go in that direction.

One more thing to keep in mind:
In parallel with this PR, I've submitted a draft PR to add cssstyle as a dependency of @acemir/CSSOM.
acemir/CSSOM#58

If we port cssstyle to jsdom, we'll need to take @acemir/CSSOM into account.

I am fine as either a separate module or in jsdom. But I don't understand why this is surprising to you, or why this seems like a big change in comparison to the rest of the PR. This mostly just means deleting some lines that install properties on CSSStyleDeclaration, I think.

I would love to do work in more small stages. However, that does not appear to fit well with your working style :). You seem to prefer mega patches that combine many things. E.g., this patch combines:

A lot of style changes to use constants instead of strings

Updates to the CI stuff

Some refactoring related to shorthand property handling (at least for 3 properties)

Updates to the generated properties

The introduction of a caching system

Changes to what files parsing is done in (utils/propertyDescriptors.js changes)

Changes to what gets passed to construct the CSSStyleDeclaration

Changes to the public API of CSSStyleDeclaration

An optimization to do less serializing (set cssText: avoid serializing cssText when not needed #254)

Updates to dependencies and dev dependencies

Updates to the main module's exports to include property lists

Probably more things...

My ideal working style would be that each of these sorts of things gets its own small PR. Those would be easy to merge and review quickly. But you seem to prefer large giant PRs.

It is hard for me to accept large giant PRs that only go part of the way. If you are going to submit them, then I need to be able to give feedback and steer the direction of the large PRs, instead of us reaching a point where you have done a lot of work but we are stuck on the last part.

If you would like to move to smaller PRs instead, I would welcome that. We could do lots of small more-frequent releases.

My ideal working style would be that each of these sorts of things gets its own small PR. Those would be easy to merge and review quickly.

I would like to do the same.
However, this time

Per Update CSSStyleDeclaration and export property related data #244 (review), you requested that we should merge in one go.
So I combined related issues into one.

Fixing one issue resulted in the need for several other fixes, one after the other.

However, breaking this down into smaller PRs at this point would require even a lot more work.
I'm not so keen to do that.

I'm sorry for any miscommunication. #244 (review) did not indicate a preference for one large PR that includes all the things in the bullet pointed list. I was just asking if you had tested the changes so far with jsdom.

That is, the module as a whole needs to stay working when integrated with jsdom. But that is still very doable with small PRs.

asamuzaK marked this pull request as draft September 8, 2025 14:04

asamuzaK force-pushed the style2 branch 5 times, most recently from 3ae525e to ab291f8 Compare September 9, 2025 21:17

domenic reviewed Sep 10, 2025

View reviewed changes

lib/webidl/CSSStyleDeclaration.webidl Outdated Show resolved Hide resolved

asamuzaK force-pushed the style2 branch 7 times, most recently from 1a3321e to 46b08ca Compare September 14, 2025 06:07

asamuzaK marked this pull request as ready for review September 14, 2025 06:56

asamuzaK requested a review from domenic September 14, 2025 06:56

asamuzaK marked this pull request as draft September 19, 2025 11:55

asamuzaK force-pushed the style2 branch 2 times, most recently from 6f0481b to c8afbb5 Compare September 21, 2025 21:04

asamuzaK mentioned this pull request Oct 2, 2025

Regression: for el.style.transform = "translate((10%), 10%)" whitespace is removed (worked in v26, fails in v27) jsdom/jsdom#3939

Open

asamuzaK force-pushed the style2 branch 3 times, most recently from e115115 to 72a43fa Compare October 3, 2025 00:46

asamuzaK mentioned this pull request Oct 3, 2025

CSSStyleDeclaration getPropertyPriority method return nothing jsdom/jsdom#3940

Open

asamuzaK force-pushed the style2 branch from 8b0f3f3 to 7b7030d Compare October 3, 2025 21:01

asamuzaK marked this pull request as ready for review October 3, 2025 23:36

domenic reviewed Oct 6, 2025

View reviewed changes

asamuzaK force-pushed the style2 branch from 529c910 to f309bb8 Compare October 12, 2025 04:48

Update CSSStyleDeclaration.js

d7716cf

asamuzaK force-pushed the style2 branch from a7c9101 to d7716cf Compare November 22, 2025 14:13

asamuzaK added 2 commits November 22, 2025 23:13

Update parsers.js

817f4f2

Update CSSStyleDeclaration.js

a6ea026

asamuzaK force-pushed the style2 branch from d828600 to a6ea026 Compare November 22, 2025 14:31

Update dependencies and devDependencies

a016d1e

asamuzaK marked this pull request as ready for review November 22, 2025 14:40

Update JSDoc comments

d53baf3

asamuzaK force-pushed the style2 branch 3 times, most recently from da680de to 107897c Compare November 23, 2025 00:19

Update parsers.js

60c6d16

asamuzaK force-pushed the style2 branch 2 times, most recently from cb7aec0 to b4ba31c Compare November 23, 2025 11:57

Update CSSStyleDeclaration.js

fc7f5aa

asamuzaK force-pushed the style2 branch from b4ba31c to fc7f5aa Compare November 23, 2025 12:07

asamuzaK marked this pull request as draft November 23, 2025 23:42

asamuzaK added 2 commits November 24, 2025 11:01

Update properties

36e5e5d

Update generateProperties.js

1b372ad

asamuzaK marked this pull request as ready for review November 24, 2025 11:57

Require each handler

3c1a7da

asamuzaK force-pushed the style2 branch from 461ede9 to 3c1a7da Compare November 29, 2025 00:13

asamuzaK changed the title ~~Update CSSStyleDeclaration and export property related files~~ Update CSSStyleDeclaration and export property related data Nov 29, 2025

domenic reviewed Dec 3, 2025

View reviewed changes

asamuzaK added 5 commits December 3, 2025 23:29

Address try-catch-finally

8472b50

Address priority

990c070

Remove Symbol.iterator

71def32

Add comment.

a9be7b4

Fix setProperty() logic

05f240d

Uh oh!

Update CSSStyleDeclaration and export property related data #244

Are you sure you want to change the base?

Update CSSStyleDeclaration and export property related data #244

Conversation

asamuzaK commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

asamuzaK commented Sep 19, 2025

Uh oh!

asamuzaK commented Oct 3, 2025

Uh oh!

domenic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asamuzaK commented Nov 24, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asamuzaK Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

domenic Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

asamuzaK commented Sep 8, 2025 •

edited

Loading

asamuzaK Dec 3, 2025 •

edited

Loading

domenic Dec 4, 2025 •

edited

Loading