[WCJS] document.write does not work when webcomponents-loader.js is loaded asynchronously

bschlenk commented 4 years ago

Description

If webcomponents-loader.js is loaded asynchronously, but runs while document.readyState is still loading, then it fails to load the polyfills.

Steps to Reproduce

Load the webcomponents-loader.js script asynchronously on a page
Ensure that document.readyState === 'loading' when the script is run by doing a lot of work in <head>
document.write is now a noop, see https://stackoverflow.com/a/9806699/2451889
Polyfills fail to load. In browsers that don't have Promise, the script then fails at https://github.com/webcomponents/polyfills/blob/master/packages/webcomponentsjs/webcomponents-loader.js#L84

Expected Results

The loader works when loaded asynchronously while document.readyState is still loading.

Actual Results

Error thrown because Promise is not defined.

Browsers Affected

[ ] Chrome
[ ] Firefox
[ ] Edge
[ ] Safari
[x] IE 11

Prior Work

I'm basically reopening https://github.com/webcomponents/polyfills/issues/57. There was a pull request in the pre-monorepo world that tried solving this: https://github.com/webcomponents/webcomponentsjs/pull/1036

It never got merged because tests for ie11 & edge were failing. The issue with that PR is that it relies on document.currentScript to check if the script that initiated the polyfills is async, which is not supported in ie11.

Discussion

I've come up with a different approach that I'd like to discuss & potentially open up a PR for. The patch is pasted below. I'd like to understand why the loader currently uses document.write, and what sort of issues switching away from that approach would cause. The changes that I'm using make it so that polyfills and DOMContentLoaded are always waited on, and the polyfills script tag is always added via document.head.appendChild.

diff --git a/node_modules/@webcomponents/webcomponentsjs/webcomponents-loader.js b/node_modules/@webcomponents/webcomponentsjs/webcomponents-loader.js
index d3ccb6b..97acb4e 100644
--- a/node_modules/@webcomponents/webcomponentsjs/webcomponents-loader.js
+++ b/node_modules/@webcomponents/webcomponentsjs/webcomponents-loader.js
@@ -153,21 +153,31 @@

     var newScript = document.createElement('script');
     newScript.src = url;
-    // if readyState is 'loading', this script is synchronous
-    if (document.readyState === 'loading') {
-      // make sure custom elements are batched whenever parser gets to the injected script
-      newScript.setAttribute('onload', 'window.WebComponents._batchCustomElements()');
-      document.write(newScript.outerHTML);
-      document.addEventListener('DOMContentLoaded', ready);
-    } else {
-      newScript.addEventListener('load', function () {
+
+    let domLoaded = document.readyState !== 'loading';
+    let scriptLoaded = false;
+
+    newScript.addEventListener('load', function () {
+      scriptLoaded = true;
+      if (domLoaded) {
         asyncReady();
+      }
+    });
+
+    newScript.addEventListener('error', function () {
+      throw new Error('Could not load polyfill bundle' + url);
+    });
+
+    if (!domLoaded) {
+      document.addEventListener('DOMContentLoaded', function () {
+        domLoaded = true;
+        if (scriptLoaded) {
+          asyncReady();
+        }
       });
-      newScript.addEventListener('error', function () {
-        throw new Error('Could not load polyfill bundle' + url);
-      });
-      document.head.appendChild(newScript);
     }
+
+    document.head.appendChild(newScript);
   } else {
     // if readyState is 'complete', script is loaded imperatively on a spec-compliant browser, so just fire WCR
     if (document.readyState === 'complete') {

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

busynest commented 3 years ago

lit Project Browser gives error to not use document.write

changing this line would be nice:

var newScript = document.createElement('script');
    newScript.src = url;
    // if readyState is 'loading', this script is synchronous
    if (document.readyState === 'loading') {
      // make sure custom elements are batched whenever parser gets to the injected script
      newScript.setAttribute(
        'onload',
        'window.WebComponents._batchCustomElements()'
      );
      document.write(newScript.outerHTML);

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

busynest commented 1 year ago

Fix document.write() error: https://developer.chrome.com/docs/lighthouse/best-practices/no-document-write/?utm_source=lighthouse&utm_medium=devtools

webcomponents / polyfills