Skip to content

Check case-sensitivity#304

Closed
asamuzaK wants to merge 1 commit intojsdom:mainfrom
asamuzaK:case
Closed

Check case-sensitivity#304
asamuzaK wants to merge 1 commit intojsdom:mainfrom
asamuzaK:case

Conversation

@asamuzaK
Copy link
Contributor

@asamuzaK asamuzaK commented Jan 24, 2026 •
edited
Loading

Fixes #303

  • Renamed generatePropertyDefinitions.mjs to generateDefinitions.mjs.
  • Generates syntax definitions from @webref/css and saves them in lib/generated/syntaxDefinitions.js.
  • Added a new function to lib/parsers.js that checks whether syntax is case-sensitive.
  • Added a helper function to lib/parsers.js to integrate cache checking and setting.
  • Added a caseSensitive option to lib/utils/propertyDescriptors.js.

Regression test

jsdom/jsdom#4032

Benchmark results

jsdom/jsdom#4031

jsdom main + cssstyle@5.3.7:

> jsdom@27.4.0 benchmark
> node ./benchmark/runner --suites style

# style/createElement + setAttribute('style') #
jsdom  x 89.05 ops/sec ±2.99% (65 runs sampled)

# style/createElement + style properties #
jsdom  x 134 ops/sec ±3.09% (75 runs sampled)

# style/createElement + style.cssText #
jsdom  x 93.93 ops/sec ±2.33% (68 runs sampled)

# style/innerHTML: divs with inline styles #
jsdom  x 84.97 ops/sec ±2.80% (70 runs sampled)

# style/innerHTML: simple divs (no styles) #
jsdom  x 2,790 ops/sec ±2.58% (85 runs sampled)

jsdom main + linked cssstyle patch without cache:

> jsdom@27.4.0 benchmark
> node ./benchmark/runner --suites style

# style/createElement + setAttribute('style') #
jsdom  x 101 ops/sec ±2.64% (71 runs sampled)

# style/createElement + style properties #
jsdom  x 112 ops/sec ±2.52% (70 runs sampled)

# style/createElement + style.cssText #
jsdom  x 107 ops/sec ±2.18% (66 runs sampled)

# style/innerHTML: divs with inline styles #
jsdom  x 94.27 ops/sec ±2.82% (68 runs sampled)

# style/innerHTML: simple divs (no styles) #
jsdom  x 3,441 ops/sec ±2.47% (80 runs sampled)

jsdom main + linked cssstyle patch with cache:

> jsdom@27.4.0 benchmark
> node ./benchmark/runner --suites style

# style/createElement + setAttribute('style') #
jsdom  x 130 ops/sec ±2.90% (72 runs sampled)

# style/createElement + style properties #
jsdom  x 134 ops/sec ±2.23% (74 runs sampled)

# style/createElement + style.cssText #
jsdom  x 137 ops/sec ±2.30% (76 runs sampled)

# style/innerHTML: divs with inline styles #
jsdom  x 129 ops/sec ±2.77% (72 runs sampled)

# style/innerHTML: simple divs (no styles) #
jsdom  x 3,498 ops/sec ±2.71% (80 runs sampled)

@asamuzaK asamuzaK force-pushed the case branch 3 times, most recently from 1ef7e5c to 98d72dd Compare January 24, 2026 02:46
domenic
domenic reviewed Jan 24, 2026
View reviewed changes
Copy link
Member

@domenic domenic left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It seems like this approach involves, at runtime, looking up whether the property is case sensitive. This requires loading a lot of data into the distributed package and into memory, a caching layer to speed it up, a large package bundle size, etc.

But that information is available at npm run prepare time instead.

Could we instead split createGenericPropertyDescriptor() into two functions, createGenericCaseSensitivePropertyDescriptor() and createGenericCaseInsensitivePropertyDescriptor(), and inside generatePropertyDescriptors.mjs pick the correct one?

Edit: actually, it could still be one function createGenericPropertyDescriptor(property, { caseSensitive }).

@asamuzaK
Copy link
Contributor Author

asamuzaK commented Jan 24, 2026

If we want to process it at prepare time, it would be better to add caseSensitive key and value ​​to the properties in propertyDefinitions Map?

@domenic
Copy link
Member

domenic commented Jan 24, 2026 •
edited
Loading

I'm not sure which is best and OK to leave the choice to you. My understanding is that current architecture is split into two steps:

  1. Generate propertyDefinitions.js from @webref/css. Some fields of propertyDefinitions.js will be used at runtime, and some in the next prepare-time step.
  2. Generate propertyDescriptors.js from propertyDefinitions.js.

We could extend this to add a bit more information in step 1 to propertyDefinitions.js, so that step 2 can be written simply. I think that is your proposal.

I guess the most efficient architecture would be something like:

  1. Generate propertyDefinitions.js from @webref/css. Only fields that are used at runtime are included.
  2. Generate propertyDescriptors.js from @webref/css.

Then we could run 1) and 2) in parallel at prepare-time, and the package size + runtime memory overhead would be minimal.

But I do not fully understand which fields (name, href, legacyAliasOf, styleDeclaration, extended, syntax) we plan to use at runtime, so I don't know how different the most efficient architecture vs. the current architecture is. It does not feel urgent to switch.

(There is also compromise architecture, which could start with current architecture but then have a step 3 which deletes fields from propertyDescriptors.js that are not used at runtime. This would give the same runtime efficiency as the most efficient architecture, but the prepare-time code would be more messy.)

@asamuzaK asamuzaK mentioned this pull request Jan 24, 2026
Merged
@asamuzaK
Copy link
Contributor Author

asamuzaK commented Jan 24, 2026

Submitted another PR.

@domenic domenic closed this in 256e277 Jan 28, 2026
@asamuzaK asamuzaK deleted the case branch January 28, 2026 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@domenic domenic domenic left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Defect: 5.3.6+ forces animation-name to be lowercase

2 participants

@asamuzaK @domenic