perf(engine-dom): use adoptedStyleSheets.push

[nolanlawson]

Details

Chrome recently made adoptedStyleSheets mutable (see this post and WICG/construct-stylesheets#45). The idea is that, instead of doing:

shadowRoot.adoptedStyleSheets = [...adoptedStyleSheets, sheet]

...you can do:

shadowRoot.adoptedStyleSheets.push(sheet)

Benchmarking

In prior testing (WICG/construct-stylesheets#94 (comment)), I indeed found some overhead from cloning and re-assigning the array. This is actually why we don't use adoptedStyleSheets at the document level – it tends to have so many stylesheets that the perf cost of repeatedly cloning the array is too high.

Just to be sure, I wrote a new microbenchmark. It confirms that calling push is indeed faster than re-assigning the array. With 1000 components and 100 stylesheets per component, testing the median of 5 runs on Chrome Canary 100, I got 4491.3ms (re-assignment) vs 334.5ms (push), so it's a pretty big difference.

Feature detection

Unfortunately, there doesn't seem to be a straightforward way of testing that adoptedStyleSheets.push is supported. In the old (frozen) array, it has all of the mutation methods (push, shift, pop, etc.), but if you call them, they throw an error:

TypeError: Cannot add property 0, object is not extensible

The most succinct detection method I found is:

const supported = Object.getOwnPropertyDescriptor(document.adoptedStyleSheets, 'length').writable

This is correctly true in Chrome Canary 100 and false in Chrome 97. It's also correctly false in Firefox Nightly 98 with the constructable stylesheets flag enabled.

Does this pull request introduce a breaking change?

• ✅ No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

• ✅ No, it does not introduce an observable change.

[nolanlawson]

FWIW, I also ran all our Karma tests through Chrome Canary 100 with DISABLE_SYNTHETIC=1 to make sure they pass.

[ekashida]

Nit: Instead of relying on the implementation of length, maybe we should instead invoke the push() in a try-catch to determine whether we can use it? No strong feelings about this either way.

[nolanlawson]

@ekashida Yeah, I waffled between the two, but the length trick is much shorter. And it seems like... if it's mutable, then setting length should work? It works with normal arrays:

const arr = [1,2,3]
arr.length = 0
arr // []