Skip to content

Conversation

@jsnajdr
Copy link

@jsnajdr jsnajdr commented Dec 1, 2025

Fixes jsdom/jsdom#3985, at least a big part of it.

During set cssText the library calls setProperty many times but wants to fire the onChange callback only once at the end. That's what the this._setInProgress field is for. But despite _setInProgress being true, the get cssText getter is called many times: to get the originalText, and because of suboptimal order of conditions.

This patch helps avoid these unneeded and expensive get cssText calls. They are expensive because they serialize the AST structure to a string.

I've been testing this on a little JSDOM benchmark script:

console.time('create-100-divs');
for (let i = 0; i < 100; i++) {
  const div = document.createElement('div');
  div.style.cssText = 'color: blue; background-color: white; padding: 10px; border: 1px solid black;';
  div.textContent = `Div #${i + 1}`;
  document.body.appendChild(div);
}
console.timeEnd('create-100-divs');

Before this PR, it takes 230ms to execute, after this PR it's 130ms, almost 2x improvement.

I think there are going to be even more optimization opportunities. If I remove the div.style.cssText assignment, the whole loop runs in 1ms.

@asamuzaK
Copy link
Contributor

asamuzaK commented Dec 1, 2025

I think this fix is already applied in another PR:
https://github.com/jsdom/cssstyle/pull/244/files#diff-c648a99dd91040422ecfb71b6bdaf3f98dee66a7860de9b015a09653f4761ea2R318

const originalText =
  typeof this._onChange === "function" && !this._updating ? this.cssText : null;

@jsnajdr
Copy link
Author

jsnajdr commented Dec 1, 2025

I think this fix is already applied in another PR:

Yes, I can confirm that both changes are applied there in the rewritten code.

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.

Performance regression in jsdom@27

2 participants