Remove generated/properties in favor of direct mixin #278

asamuzaK · 2026-01-02T09:27:42Z

Motivation

This PR refactors the property handling mechanism.
generated/properties.js is a duplicate of the logic already present in properties/*.js.
By referencing properties/*.js directly from the new generated/propertyHandlers.js, we eliminate this redundancy and ensure a single source of truth for property definitions.

Part of #255 (comment)
However, this PR does not include a fix for CSSStyleDeclaration itself.
Planning to address it in a separate PR.

Key Changes

Core Logic Update:
- Added scripts/generatePropertyHandlers.mjs, which generates generated/propertyHandlers.js that directly references definitions in properties/*.js.
- Created ./property.js to manage property definitions dinamically.
- Updated CSSStyleDeclaration to use propertyDefinitions and the mixinProperties() function from ./property.js.
Cleanup: Removed obsolete generator scripts.
Data Update: Downloaded and synced the latest CSS property definitions from W3C.

domenic · 2026-01-02T10:16:47Z

This is extremely exciting. Thank you for all your work to get us to this point. Now that I compare the generated code before and after, I see what a good idea this is. It is also great to remove the complicated codegen dependencies.

However, I think we can go even further. The architecture in this PR does the following:

At build time, generates generated/propertyHandlers.js which, for every handler we have manually implemented, maps the canonical CSS property name (e.g. "background-color") to the handler.
At module startup:
- Loops through every CSS property definition. For each property name X,
  - For each property alias Y,
    - If we have a handler, install Y -> X's property descriptor from the handler on CSSStyleDeclaration.prototype.
    - Otherwise, install Y -> generic property descriptor on CSSStyleDeclaration.prototype.

This approach is much better than the previous approach, which generated a lot of redundant code for the generic handlers. However, it does a lot of work at startup.

We could do better by changing the build-time script to generate generated/propertyDescriptors.js, which contains an object literal (not a Map) from all property names and aliases, to the correct property descriptor. The file would look something like this:

"use strict";
const backgroundImage = require("background-image");
const { getGenericPropertyDescriptor } = require("../utils/propertyDescriptors.js");

module.exports = {
  // ...
  "background-image": backgroundImage,
  "backgroundImage": backgroundImage
  // ...
  "block-ellipsis": getGenericPropertyDescriptor("block-ellipsis"),
  "blockEllipsis": getGenericPropertyDescriptor("block-ellipsis")
  // ...
};

Then, we do not need your new property.js file at all. Instead, CSSStyleDeclaration.js can contain the following:

const propertyDescriptors = require("./generated/propertyDescriptors.js");
const propertyNames = require("./generated/allProperties.js");

// ...
    if (!propertyNames.has(property)) {
// ...

Object.defineProperties(CSSStyleDeclaration.prototype, propertyDescriptors);

What do you think?

We only use camelize for scripts.

asamuzaK · 2026-01-02T12:08:26Z

PR ready.

asamuzaK · 2026-01-02T13:34:16Z

Additional notes:

The generated file name remains generated/propertyHandlers.js.
It is because the difference between propertyDescriptors as proposed and propertyDefinitions may not be immediately clear and may be confusing.
Since these are setters and getters, propertyHandlers seems a little easier to understand what it does.

domenic · 2026-01-02T13:48:43Z

No, please use the proper term, "propertyDescriptors". See, e.g., https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getOwnPropertyDescriptor#description .

We should, as a lower-priority future to-do, fix the exports from the property handler files to be descriptor instead of definition. (Or you can do it in this PR if you want.)

But, the hardest part of reviewing this PR was properly distinguishing the three things:

CSS property definitions
JavaScript property descriptors
What you are calling "property handlers", which are the files in properties/*.js which have several exports.

We should not use the term "handlers" for (2). Only for (3).

asamuzaK · 2026-01-02T21:52:56Z

PR ready.

domenic

Looking great, just a few more tweaks.

lib/property.js

domenic · 2026-01-03T02:03:17Z

scripts/generatePropertyDescriptors.mjs

+    for (const property of styleDeclaration) {
+      descriptors.push(`"${property}": ${camel}.definition`);
+    }
+  } else if (camelCasedProperties.has(aliasCamel)) {


This branch is never hit (and never should be hit; we should always ensure the files are named after their canonical property names).

Delete it and delete all the stuff around aliasCamel and legacyAliasOf.

For now, I would like to maintain the current implementation.
As mentioned in the comment below, the conditions here are not met because we don't prepare a standard implementation.
Even after implementing webkit-foo standardization and not hit here, I would like to remove it at that time.

I don't understand this, sorry. This code is literally never executed. It is dead code. And it is new code. I would like to not commit new code that is never run to this repository.

domenic · 2026-01-03T02:05:20Z

scripts/generatePropertyDescriptors.mjs

+
+const list = fs
+  .readdirSync(path.resolve(dirname, "../lib/properties"))
+  .map((file) => file.replace(/\.[cm]?js$/, ""));


I would prefer if we were a bit more careful with file extensions:

Don't get rid of them here. It's nicer if the generated require()s contain file extensions. This works better in the future if we move to ESM, and is slightly faster today.

Don't be tolerant of \.[cm]?js. Only accept \.js. If we change our file extensions in the future, we can change this script. Accepting different extensions now just lets the project be inconsistent.

Removed \.[cm]?js, and added file extension like properties/foo.js in require().

domenic · 2026-01-03T02:05:46Z

scripts/generatePropertyDescriptors.mjs

+// autogenerated - ${dateTodayFormatted}
+
+${requires.sort().join("\n")}
+const { getPropertyDescriptor } = require("../utils/propertyDescriptors");


Suggested change

const { getPropertyDescriptor } = require("../utils/propertyDescriptors");

const { getPropertyDescriptor } = require("../utils/propertyDescriptors.js");

domenic · 2026-01-03T02:11:04Z

test/CSSStyleDeclaration.test.js

  });

-  it("has all functions", () => {
+  it("has all camelCase properties", () => {


Let's change this into a smoke test instead of an exhaustive test. Just pick one property and test that its camelCase version is there manually.

domenic · 2026-01-03T02:11:19Z

test/CSSStyleDeclaration.test.js

  });

-  it("has PascalCase for webkit prefixed properties", () => {
+  it("has PascalCase property for all webkit prefixed properties", () => {


Similarly just pick a single one that we know should have WebKit.

domenic · 2026-01-03T02:43:19Z

While writing up #279, I noticed something interesting we should probably fix in this PR.

Previously, lib/generated/properties.js contained entries for webkitBorderAfterColor, which was a property from lib/utils/allExtraProperties.js. Importantly, it is a property which we have specific handler code for, in lib/properties/webkitBorderAfterColor.js.

After this PR, lib/properties/webkitBorderAfterColor.js is not referenced anywhere, because it does not appear in the @webref/css database. So, it is dead code.

Per my initial thoughts in #279, I think this is OK. We can have a breaking change that removes all properties not in the @webref/css database.

But, we should delete the dead code ASAP. I think doing it in this PR would be best.

You should be able to quickly ask an AI to delete all files in lib/properties/ which do not appear in generated/propertyDescriptors.js.

If you think deleting these files loses valuable code, then we should stop to reconsider and discuss. But I think it will probably be fine.

domenic · 2026-01-03T02:44:50Z

Also, sorry, some of my drafts from an earlier revision seem to have been kept in my latest review. I will go mark them all as resolved.

asamuzaK · 2026-01-03T08:08:51Z

After this PR, lib/properties/webkitBorderAfterColor.js is not referenced anywhere, because it does not appear in the @webref/css database. So, it is dead code.
(snip)
But, we should delete the dead code ASAP. I think doing it in this PR would be best.

This is tracked in #210.

Some things have been standardized in -webkit-foo, so I don't think it's appropriate to simply remove everything just because it's not in @webref/css.

I'm planning to address this before making the major bump.
But, not in this PR.

domenic · 2026-01-03T08:32:21Z

Let me a bit more clear on the requirements for this PR.

Either:

This PR should not remove any functionality. In that case, we need to do more work so that propertyDescriptors.js contains a reference to lib/properties/webkitBorderAfterColor.js. Right now, it does not.

OR

This PR should remove functionality (like it currently does), and delete all dead code.

I prefer (2). But if you prefer (1), we need to do some work there too.

domenic · 2026-01-03T08:34:34Z

Some things have been standardized in -webkit-foo, so I don't think it's appropriate to simply remove everything just because it's not in @webref/css.

I think these two things are not in conflict. @webref/css contains all standardized -webkit-foo properties. For example, it contains -webkit-flex-wrap. It does not contain non-standardized -webkit-foo properties. For example, it does not contain -webkit-border-after-color.

asamuzaK · 2026-01-03T08:40:04Z

Some things have been standardized in -webkit-foo, so I don't think it's appropriate to simply remove everything just because it's not in @webref/css.

I think these two things are not in conflict. @webref/css contains all standardized -webkit-foo properties. For example, it contains -webkit-flex-wrap. It does not contain non-standardized -webkit-foo properties. For example, it does not contain -webkit-border-after-color.

I'm talking about things like -webkit-border-after-color -> border-block-end-color.

It has been standardized, but renamed.

asamuzaK · 2026-01-03T08:45:42Z

2. This PR should remove functionality (like it currently does), and delete all dead code.

I also prefer 2, but in a separate PR to address that issue.

domenic · 2026-01-03T10:18:28Z

I'm talking about things like -webkit-border-after-color -> border-block-end-color.

It has been standardized, but renamed.

We should not include support for non-standardized aliases like -webkit-border-after-color -> border-block-end-color. Only standardized aliases like -webkit-flex-wrap -> flex-wrap.

I also prefer 2, but in a separate PR to address that issue.

OK. So will you do 1 in this PR?

asamuzaK added 5 commits January 2, 2026 16:40

Add propertyHandlers generator script

3c7f367

Create property.js

d15bbd4

Update CSSStyleDeclaration.js

112fc37

Remove unneeded generator scripts

6212ffc

Update allProperties.js

0d9d7c1

asamuzaK added 3 commits January 2, 2026 20:30

Merge property.js into generated propertyHandlers.js

27a88ca

Move camelize under scripts/

d331428

We only use camelize for scripts.

Update generatePropertyHandlers.mjs

aec0b3b

asamuzaK force-pushed the prop branch from 3547c2e to 2eb64f5 Compare January 2, 2026 14:01

asamuzaK marked this pull request as draft January 2, 2026 14:30

asamuzaK force-pushed the prop branch from 2eb64f5 to e2c0924 Compare January 2, 2026 15:07

asamuzaK marked this pull request as ready for review January 2, 2026 15:12

asamuzaK force-pushed the prop branch 2 times, most recently from 32a91db to e0c6d48 Compare January 2, 2026 15:37

Rename

cce12ee

asamuzaK force-pushed the prop branch from e0c6d48 to cce12ee Compare January 2, 2026 15:45

Export object by default

bfaef3f

domenic reviewed Jan 3, 2026

View reviewed changes

This was referenced Jan 3, 2026

Reduce the amount of work done to assemble a list of all properties #279

Open

Cleanups after removing generated/properties #280

Open

Update generatePropertyDescriptors.mjs

e9d9c2e

Update CSSStyleDeclaration.test.js

6b6065d

asamuzaK force-pushed the prop branch from e42a047 to 6b6065d Compare January 3, 2026 08:33

asamuzaK force-pushed the prop branch from 2bf2861 to 2519a13 Compare January 3, 2026 08:56

Update generatePropertyDescriptors.mjs

1b898ef

asamuzaK force-pushed the prop branch from 2519a13 to 1b898ef Compare January 3, 2026 08:57

Update generatePropertyDescriptors.mjs

d1e9582

	const { getPropertyDescriptor } = require("../utils/propertyDescriptors");
	const { getPropertyDescriptor } = require("../utils/propertyDescriptors.js");

Uh oh!

Remove generated/properties in favor of direct mixin #278

Are you sure you want to change the base?

Remove generated/properties in favor of direct mixin #278

Conversation

asamuzaK commented Jan 2, 2026

Motivation

Key Changes

Uh oh!

domenic commented Jan 2, 2026

Uh oh!

asamuzaK commented Jan 2, 2026

Uh oh!

asamuzaK commented Jan 2, 2026

Uh oh!

domenic commented Jan 2, 2026

Uh oh!

asamuzaK commented Jan 2, 2026

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!

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!

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 commented Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

domenic commented Jan 3, 2026

Uh oh!

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

Uh oh!

Uh oh!

domenic commented Jan 3, 2026

Uh oh!

domenic commented Jan 3, 2026

Uh oh!

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

Uh oh!

Uh oh!

asamuzaK commented Jan 3, 2026

Uh oh!

domenic commented Jan 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

domenic commented Jan 3, 2026 •

edited

Loading

asamuzaK commented Jan 3, 2026 •

edited

Loading

asamuzaK commented Jan 3, 2026 •

edited

Loading