Block Editor: Simulated Media Query: Skip CSS rule if not style rule

[aduth]

Related: #19082

This pull request seeks to resolve an error preventing the editor from loading in Internet Explorer.

See TBD for more context:

Specifically, it seems that Internet Explorer chokes when trying to access rule.cssText if the rule is not a style rule.

gutenberg/packages/block-editor/src/components/simulate-media-query/index.js

Line 79 in 5a5564b

!! rule.cssText.match( new RegExp( `#start-${ marker }` ) )

Error: Member not found.

  at Anonymous function (http://192.168.1.162:8888/wp-content/plugins/gutenberg/build/block-editor/index.js?ver=4b589b399cd6b646f1ffb628aaece1e3:26000:9)
  at Anonymous function (http://192.168.1.162:8888/wp-content/plugins/gutenberg/build/block-editor/index.js?ver=4b589b399cd6b646f1ffb628aaece1e3:25994:5)
  at Vb (http://192.168.1.162:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.b694e242.js?ver=16.9.0:104:421)
  at Xi (http://192.168.1.162:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.b694e242.js?ver=16.9.0:151:136)
  at Scheduler.unstable_runWithPriority (http://192.168.1.162:8888/wp-content/plugins/gutenberg/vendor/react.min.0212dc62.js?ver=16.9.0:26:333)
  at Ma (http://192.168.1.162:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.b694e242.js?ver=16.9.0:52:273)
  at Yb (http://192.168.1.162:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.b694e242.js?ver=16.9.0:150:413)
  at O (http://192.168.1.162:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.b694e242.js?ver=16.9.0:120:254)
  at ze (http://192.168.1.162:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.b694e242.js?ver=16.9.0:118:5)
  at Anonymous function (http://192.168.1.162:8888/wp-content/plugins/gutenberg/vendor/react-dom.min.b694e242.js?ver=16.9.0:53:47)

On some debugging, this seems to be caused by mejs-loading-spinner, of type CSSRule.KEYFRAMES_RULE. It's unclear, but given this is "experimental", I imagine that may be connected to why it errors.

In any case, I imagine that we're only concerned with replacing CSS style rules? In which case, we can skip anything which is not a style rule. For me, this resolves the error. I imagine it could also marginally improve the performance of this implementation.

Testing Instructions:

Repeat testing instructions from #19082.

Verify that the editor loads in Internet Explorer.

[tellthemachines]

Thanks for working on this! Looks good apart from my comment below. Would you like to fix the window check here, or shall I in a separate PR? I should be doing a PR anyway to address the other issues you mentioned.

[aduth]

Would you like to fix the window check here, or shall I in a separate PR? I should be doing a PR anyway to address the other issues you mentioned.

I'm still unclear whether we might expect window to be undeclared in the environments we're trying to anticipate (I'm assuming that this code is being run in Native as well?). In any case, the combination of a09c38b and ad2d145 should cover whether it's undefined or undeclared. Maybe in the future, we can drop the condition altogether like I note in #20226 (comment) if we can be certain that window would be at least undefined.

[aduth]

It is slightly sketchy to not be guarding window here, but if we assume that getStyleSheetsThatMatchHostname is returning an empty array for any non-DOM environments, then this logic (within styleSheets.forEach) would never be reached.

[tellthemachines]

I'm still unclear whether we might expect window to be undeclared in the environments we're trying to anticipate (I'm assuming that this code is being run in Native as well?).

I'm not sure about this, or even how to test it - would I need a dev version of the app?

Approving in any case as this fixes the IE issue.

[aduth]

I'm still unclear whether we might expect window to be undeclared in the environments we're trying to anticipate (I'm assuming that this code is being run in Native as well?).

I'm not sure about this, or even how to test it - would I need a dev version of the app?

Approving in any case as this fixes the IE issue.

In speaking with @hypest about this, it appears that React Native does define a window global, with some mocked properties (reference). So technically, window would never be undefined.

But I suspect it's still a good idea to have some guardedness, related to the same ideas proposed in #16227, for modules intended to be used server-side (by third-parties). It's more a problem for code which executes as a side-effect of being imported, so maybe not strictly necessary here. Perhaps we may decide to remove it in the future, but in the meantime the updated condition is at least more accurate to what we hope to achieve with it (resolving thrown error if window was not declared).