fix: self does not work in Lit SSR, replaced by globalThis

[maartenst]

as the title says, in Lit SSR (verified in the Lit eleventy ssr plugin) self does not work as it is not defined.

[aomarks]

as the title says, in Lit SSR (verified in the Lit eleventy ssr plugin) self does not work as it is not defined.

Hmm, we can't really use globalThis here because it's not supported by IE11, and I think we want this polyfill to run directly in IE11, like our other polyfills.

window would be an option, except that it won't be defined in workers. I'm not sure if that could be a valid place to use this polyfill, but maybe.

I wonder if we should actually define window.self = window in the Lit SSR global polyfill instead.

@kevinpschaaf @justinfagnani in case you have any further thoughts

[maartenst]

as the title says, in Lit SSR (verified in the Lit eleventy ssr plugin) self does not work as it is not defined.

Hmm, we can't really use globalThis here because it's not supported by IE11, and I think we want this polyfill to run directly in IE11, like our other polyfills.

window would be an option, except that it won't be defined in workers. I'm not sure if that could be a valid place to use this polyfill, but maybe.

window I can indeed confirms (had that before until Justin pointed out globalThis. Didn't know about the workers..

I wonder if we should actually define window.self = window in the Lit SSR global polyfill instead.

this would be fine too, depending on how often this is needed I guess.

[kevinpschaaf]

Only thoughts are:

1. This polyfill doesn't really target IE11, since ShadyCSS doesn't support styling scoped elements (it styles shadow roots by tag names, which this change would break), and it's not been tested on the CE polyfill either.
2. To use it in a worker, lots of other browser stuff would need to be polyfilled as well... But I guess the reason to try and avoid window gets back to the "that's how some node packages detect browser context" issue we had on Lit SSR.

So I think globalThis would probably be ok, although we might want to add a note on the README indicating browser support, since that's a bit unclear. It's really targeting modern browsers with native CE support (there's a few old versions of FF and Chrome that have CE but not globalThis, but probably not enough to worry about)?

OTOH window.self = window in the SSR dom-shim is maybe the safest overall?

[shaal]

According to https://death-to-ie11.com (funny domain name, but great source for resources on the matter), IE11 support ends in 4 months. Most Windows users already got an automatic update to Edge Chromium. General usage is below 1%. Is it worth holding features in order to support IE11?

[justinfagnani]

Our consideration for IE support in non-breaking changes is less about whether or not we want to support IE as a whole and more about whether we're ready for a breaking change to drop support. The polyfills aren't uniform here, as @kevinpschaaf points out: the core Shady DOM and Shady CSS libraries are mainly for IE at this point, while this polyfill is mainly not.

window would be mostly fine with me, except that we might be trying to not define that in the SSR's DOM shim. We can also use typeof to help .call(typeof globalThis === 'object' ? globalThis : window)

[aomarks]

Just for clarity:

	web (modern)	ie11	web worker	node	node with lit ssr polyfill
self	✅	✅	✅	❌	❌
window	✅	✅	❌	❌	✅
globalThis	✅	❌	✅	✅	✅

window would be mostly fine with me, except that we might be trying to not define that in the SSR's DOM shim. We can also use typeof to help .call(typeof globalThis === 'object' ? globalThis : window)

Yeah typeof globalThis === 'object' ? globalThis : window seems like a good safe choice for now. It will work everywhere.

I'm also curious as a side note if we can optimize the bundle, and maybe side-step this whole question. If you look at https://unpkg.com/@webcomponents/scoped-custom-element-registry@0.0.4/scoped-custom-element-registry.min.js, we pass in self, but then there's a whole bunch of "object" === typeof globalThis and similar checks going on as well... and then we end up just calling window.customElements anyway!

[maartenst]

implemented the typeof check

[maartenst]

not sure how to read the lint output (nor can I run it locally due to broken deps in the project it seems?)

[aomarks]

not sure how to read the lint output (nor can I run it locally due to broken deps in the project it seems?)

I think it's complaining about this:

-        output_wrapper: '(function(){\n%output%\n}).call(typeof globalThis === \'object\' ? globalThis : window);',
+        output_wrapper:
+          "(function(){\n%output%\n}).call(typeof globalThis === 'object' ? globalThis : window);",

So it wants double quotes, and a newline.

[maartenst]

not sure how to read the lint output (nor can I run it locally due to broken deps in the project it seems?)

I think it's complaining about this:

-        output_wrapper: '(function(){\n%output%\n}).call(typeof globalThis === \'object\' ? globalThis : window);',
+        output_wrapper:
+          "(function(){\n%output%\n}).call(typeof globalThis === 'object' ? globalThis : window);",

So it wants double quotes, and a newline.

done, please check

[maartenst]

this should be it for lint

[aomarks]

@maartenst This patch was just released in @webcomponents/scoped-custom-element-registry@0.0.5 Thanks again for the PR!