webcomponents / webcomponents/polyfills

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

Open
#251 5 comments 0 reactions 0 assignees View on GitHub
Package: webcomponentsjs Severity: Low Type: Bug wontfix
Dominant language
HTML
Stars
1.2k
Forks
168
PR merge metrics
No merged PRs in 30d

Description

### 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 ``
3. [document.write](https://github.com/webcomponents/polyfills/blob/master/packages/webcomponentsjs/webcomponents-loader.js#L160) 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](https://developer.mozilla.org/en-US/docs/Web/API/Document/currentScript#Browser_compatibility) 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') {
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.