polyfills icon indicating copy to clipboard operation
polyfills copied to clipboard

Published 20 hours ago • verified publisher icon webcomponents

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

Open bschlenk opened this issue 5 years ago • 4 comments

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

  1. Load the webcomponents-loader.js script asynchronously on a page
  2. Ensure that document.readyState === 'loading' when the script is run by doing a lot of work in <head>
  3. document.write is now a noop, see https://stackoverflow.com/a/9806699/2451889
  4. 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') {

bschlenk avatar Jan 22 '20 17:01 bschlenk

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.

stale[bot] avatar May 22 '21 09:05 stale[bot]

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);

busynest avatar Sep 10 '21 23:09 busynest

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.

stale[bot] avatar Sep 28 '22 03:09 stale[bot]

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

busynest avatar Jun 26 '23 22:06 busynest