Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WCJS] Consider adding dynamic import based loader #63

Open
abdulhannanali opened this issue Jan 30, 2018 · 13 comments
Open

[WCJS] Consider adding dynamic import based loader #63

abdulhannanali opened this issue Jan 30, 2018 · 13 comments

Comments

@abdulhannanali
Copy link

I intend to use webcomponents-loader.js within my project which uses Webpack and webpack has its own dependency graph which requires explicit declarative import everything we need to use within the application and that means the current file structure doesn't get copied out as is which is the expectation for the current loader to work. However, I still want the polyfills to be loaded dynamically as I hope that most of the people in future who use the program will have latest browsers, so this will lead to major size reduction and load the polyfills only when needed.

Webpack's dynamic import can be used here and a new file created for the purpose of webpack can help solve the issues. I would like to know the thoughts of the polyfill maintainers if they are willing to accept it in this repo or if I should create a fork since it's something very build tool specific, since the dynamic imports are in stage 3 of the TC39 proposal maybe it's not that much specific to webpack now.

@justinfagnani
Copy link
Collaborator

The polyfills shouldn't be bundled with your app, but loaded via script tags as documented. There's no benefit from attempting to bundle.

@abdulhannanali
Copy link
Author

@justinfagnani Webpack's dependency graph doesn't really work with the webcomponents-loader.js file since it's not requiring anything else. I was thinking along the lines of making use of webpack specific code splitting feature for the polyfill. That's the whole point of using webcomponents-loader.js to not bundle unnecessary polyfills.

@abraham
Copy link

abraham commented Jul 5, 2018

BTW: You can load v1.0.1 with webpack. v1.0.2 and after won't work.

wcjs supporting webpack dynamic imports makes it so you don't have to do a bunch of wcjs custom code in your app/component.

const polyfills = [];
if (!('customElements' in window)) {
   polyfills.push(import('@webcomponents/webcomponentsjs/webcomponents-ce'));
}
if (!('attachShadow' in document.head)) {
   polyfills.push(import('@webcomponents/webcomponentsjs/webcomponents-sd'));
}
if (!('IntersectionObserver' in window)) {
  polyfills.push(import('intersection-observer'));
}
Promise.all(polyfills).then(() => {
  import('node-package');
  import('img-2');
});

@morewry
Copy link

morewry commented Jul 20, 2018

I'd like to see a version of these polyfills that works better with popular tools like Webpack as well.

Webpack and Babel are quite capable of only including the polyfills you need with a combination of babel-preset-env and babel-plugin-transform-runtime. Granted, the result there would include polyfills for browsers that don't need them if your babel-preset-env list includes IE11, since a single bundle is typically served to all browsers. But it'd be nice to have that option without having to reverse engineer/write your own.

There are other options it seems worthwhile to explore also. @abraham's post above sounds like it could be a great solution. Especially re-emphasizing that dynamic imports aren't just a Webpack thing, they're a potential standard (https://github.com/tc39/proposal-dynamic-import) at Stage 3.

I can also imagine it being nice if you were able to use Webpack to split a script similar to webcomponents-loader (maybe using those dynamic imports) as well as other polyfills used by your app into a bundle separate from your app. And not end up with two different Promise polyfills in IE11.

Seems like some of these options could have all the same benefits as webcomponents-loader currently has and make those benefits more available to the average framework and Webpack user using a "modern JS" development workflow.

As it currently is, the average framework and Webpack user who is using many future features very seamlessly elsewhere in their application has to pay a lot of special attention to use Web Components, and there are somewhat intractable issues that sound like they stem from the Web Component polyfills themselves needing polyfills like Promise or Symbol, like previous incarnations of my issue in #972.

In fact, if I could get some help with the reverse engineering ASAP, I'd be willing to take a stab at that, because I'm feeling pretty blocked and I'm not really into the idea of ditching my entire direction to rewrite my components in Angular or React. Which, if I don't get all these bumps in the road smoothed out soon, I'm going to have to do.

@morewry
Copy link

morewry commented Jul 31, 2018

A few things I've noticed so far that make what I envision challenging:

  • Feature detection is scattered throughout the polyfills (some files polyfill multiple features based on detection)
  • Feature detection is usually inline in if (can't borrow the feature detection condition easily)
  • Unclear why some polyfills are needed to use "Web Components" (meaning Template, Custom Elements, and Shadow DOM), e.g.
import '@webcomponents/webcomponentsjs/src/symbol.js'; // what's this needed for re: "web components"?
import '@webcomponents/webcomponentsjs/src/flag-parser.js'; // what's this needed for re: "web components"?
import '@webcomponents/url/url.js'; // what's this needed for re: "web components"?
import '@webcomponents/webcomponentsjs/src/baseuri.js'; // what's this needed for re: "web components"?
import '@webcomponents/webcomponentsjs/src/unresolved.js'; // what's this needed for re: "web components"?
  • Some polyfills appear to be "stateful", with a shared singleton that seems to be used in several different ways including storing information about what the polyfills have done.

@thescientist13
Copy link

thescientist13 commented Aug 11, 2018

For what my $.02 is worth, with tools like webpack becoming more popular and npm being the place to go for frontend JavaScript now, being able to just import xxx from 'xxx' will become a more and more familiar / intuitive workflow to developers.

Having ES2015+ (e.g. import) usage instructions makes a lot of since (since not to many modern libraries reference code paths directly in node_modules from what I have seen). To that point, I see similar issue across some popular libraries where the solution is something like import * from xxx as 'xxx', so It seems anticipating for these kind of users seems reasonable, IMO.

Could documentation be added for those who are using something like webpack?

Option 1: npm + webpack

  1. npm install @webcomponents/webcomponentsjs
  2. Use CopyWebpackPlugin to copy the file you need out of node_modules into where your webpack output folder will be
  3. Update index.html with the output path used in step 2

Options 2: UNPKG

  1. Add https://unpkg.com/@webcomponents/webcomponentsjs@2.0.4/webcomponents-bundle.js to the <head> of your index.html

Happy to contribute some docs if you like!

FWIW, I have made some progress using webpack and import with this, but can't tell if it's the reason why Shadow DOM ian't working in FF, though it definitely makes Custom Elements work in Firefox. 🤔
I get the same thing if I pull it in the using UNPKG. Noting some CSS related things in the README of this repo around CSS so will continue to investigate further.

@TimvdLippe
Copy link
Contributor

There is a Webpack starter pack from Vaadin that shows how they can be combined: https://github.com/web-padawan/polymer3-webpack-starter Does that answer your questions on how to use the tools together?

@abraham
Copy link

abraham commented Aug 27, 2018

Looking at web-padawan/polymer3-webpack-starter, it seems to simply be copying the webcomponentjs polyfills to be served locally by a blocking script tag. It's no different from the standard webcomponentjs approach other than the path is different.

@web-padawan
Copy link
Contributor

web-padawan commented Aug 27, 2018

@abraham you are right, currently approach is the same. It does make sense because of some nits, e.g. bootstrapping <template> elements in IE11. I will try your suggestion from the above comment.

Honestly, having Symbol polyfill seems a bit of tradeoff to me, at least it is a bit unclear what purpose does it serve for v2 (and not needed in v1), except for private properties in lit-element.
However, its impact on the blocking script size should be relatively small.

@morewry
Copy link

morewry commented Oct 24, 2018

I'd say that while, yes, that documentation is useful, without rethinking the way these polyfills are implemented documentation is a salve. From my perspective, the issue isn't "I don't know how to use this with Webpack." I know several ways to use this with Webpack.

The issue is that I can't use them in the typical way--and, worse, I can't take advantage of the value-adds of Webpack and other commonly paired tools like Babel.

With Webpack, all the imports from entry points on down to leaf dependencies are traversed. Opportunities are created as a result, such as detecting duplicates or being able to split complex application code into multiple bundles. You can use Babel's babel-preset-env and babel-plugin-transform-runtime with babel-runtime to automatically polyfill JavaScript features based on your support spectrum.

...And thereby avoid using any polyfills you don't need or duplicating them. Which is one of the biggest challenges I see in the current implementation. There are assumptions such as: if you need to polyfill customElements.define, well, then you also must need to polyfill Array.from and Object.assign. You might not need them, but these are hard-coded into the same file with a polyfill you may need.

Take lit-element as an example; if I were using it, I'd need to address that private static isn't supported yet, because lit-element uses it in its source code. I wouldn't expect that to have anything to do with webcomponentsjs. What I would expect is that I should install and import lit-element, run a build with my babel-preset-env and bundler, and be done.

But I'm not using lit-element and I can't get rid of the Symbol polyfill without making my own entrypoint for webcomponentsjs. Making my own entry point means having to re-implement the entry points' async support and optional loading of CE vs SD...which I absolutely do not have time for. Not to reverse engineer it, not to write it, nor to support it.

It's not only an issue of "blocking script size." While that is certainly an issue I care about, it's not the only issue. There's also the issue of overall application size. There's also an issue of collisions between different polyfills. There's also the issue of bugs arising around the use of these unnecessary polyfills, such as webcomponents/webcomponentsjs#983 webcomponents/webcomponentsjs#976 webcomponents/webcomponentsjs#972 webcomponents/webcomponentsjs#968 webcomponents/webcomponentsjs#942. There's also the issue of giving your users what they need and expect, instead of what they don't need and don't expect--which I consider to be both a usability and security concern.

I'll be honest, I am heavily biased toward the web platform. I want to see web components recognized as having all the potential I know they have--potential that is often scoffed at by the audience these problems impact and frustrate. Knowing this makes me take those problems more seriously than I might otherwise. I speak for myself, but I also speak for how I see the bigger picture, here. While I realize these polyfills are often preoccupied with supporting Polymer, they're also the defacto web component polyfills, period.

ezr-ondrej referenced this issue in ezr-ondrej/foreman Apr 29, 2019
@dfreedm dfreedm changed the title Use webcomponents-loader in webpack Consider adding dynamic import based loader Jun 7, 2019
@dfreedm dfreedm transferred this issue from webcomponents/webcomponentsjs Jun 7, 2019
dfreedm added a commit that referenced this issue Jun 12, 2019
Use responseURL to compute redirectedUrl
@dfreedm dfreedm changed the title Consider adding dynamic import based loader [WCJS] Consider adding dynamic import based loader Jun 13, 2019
@dfreedm dfreedm added Severity: Medium Type: Feature New feature or request labels Jun 13, 2019
@SampsonCrowley
Copy link

SampsonCrowley commented Jul 9, 2019

just a sample of using dynamic imports (only tested in development with IE 11 and latest chrome)

export default (async function webComponentLoader() {
  const whenLoadedFns = [];
  let polyfillsLoaded = false,
      allowUpgrades = false,
      flushFn;

  function fireEvent() {
    window.WebComponents.ready = true;
    document.dispatchEvent(new CustomEvent('WebComponentsReady', { bubbles: true }));
  }

  function batchCustomElements() {
    if (window.customElements && customElements.polyfillWrapFlushCallback) {
      customElements.polyfillWrapFlushCallback(function (flushCallback) {
        flushFn = flushCallback;
        if (allowUpgrades) {
          flushFn();
        }
      });
    }
  }

  function asyncReady() {
    batchCustomElements();
    ready();
  }

  function ready() {
    console.log('components ready')
    // bootstrap <template> elements before custom elements
    if (window.HTMLTemplateElement && HTMLTemplateElement.bootstrap) {
      HTMLTemplateElement.bootstrap(window.document);
    }
    polyfillsLoaded = true;
    runWhenLoadedFns().then(fireEvent);
  }

  function runWhenLoadedFns() {
    allowUpgrades = false;
    let fnsMap = whenLoadedFns.map(function(fn) {
      return fn instanceof Function ? fn() : fn;
    });
    whenLoadedFns.splice(0);
    return Promise.all(fnsMap).then(function() {
      allowUpgrades = true;
      flushFn && flushFn();
    }).catch(function(err) {
      console.error(err);
    });
  }

  window.WebComponents = window.WebComponents || {};
  window.WebComponents.ready = window.WebComponents.ready || false;
  window.WebComponents.waitFor = window.WebComponents.waitFor || function(waitFn) {
    if (!waitFn) {
      return;
    }
    whenLoadedFns.push(waitFn);
    if (polyfillsLoaded) {
      runWhenLoadedFns();
    }
  };
  window.WebComponents._batchCustomElements = batchCustomElements;

  var name = 'webcomponents-loader.js';
  // Feature detect which polyfill needs to be imported.
  var polyfills = [];
  if (!('attachShadow' in Element.prototype && 'getRootNode' in Element.prototype) ||
    (window.ShadyDOM && window.ShadyDOM.force)) {
    polyfills.push('sd');
  }
  if (!window.customElements || window.customElements.forcePolyfill) {
    polyfills.push('ce');
  }

  var needsTemplate = (function() {
    // no real <template> because no `content` property (IE and older browsers)
    var t = document.createElement('template');
    if (!('content' in t)) {
      return true;
    }
    // broken doc fragment (older Edge)
    if (!(t.content.cloneNode() instanceof DocumentFragment)) {
      return true;
    }
    // broken <template> cloning (Edge up to at least version 17)
    var t2 = document.createElement('template');
    t2.content.appendChild(document.createElement('div'));
    t.content.appendChild(t2);
    var clone = t.cloneNode(true);
    return (clone.content.childNodes.length === 0 ||
        clone.content.firstChild.content.childNodes.length === 0);
  })();

  // NOTE: any browser that does not have template or ES6 features
  // must load the full suite of polyfills.
  if (!window.Promise || !Array.from || !window.URL || !window.Symbol || needsTemplate) {
    polyfills = ['sd-ce-pf'];
  }

  if (polyfills.length) {
    await import(/* webpackPrefetch: 1, webpackChunkName: webcomponent-polyfills */ `@webcomponents/webcomponentsjs/bundles/webcomponents-${polyfills.join('-')}`)

    if (document.readyState === 'loading') {
      window.addEventListener('load', asyncReady);
      window.addEventListener('DOMContentLoaded', function() {
        window.removeEventListener('load', asyncReady);
        asyncReady();
      })
    } else {
      asyncReady();
    }
  } else {
    // if readyState is 'complete', script is loaded imperatively on a spec-compliant browser, so just fire WCR
    if (document.readyState === 'complete') {
      polyfillsLoaded = true;
      fireEvent();
    } else {
      // this script may come between DCL and load, so listen for both, and cancel load listener if DCL fires
      window.addEventListener('load', ready);
      window.addEventListener('DOMContentLoaded', function() {
        window.removeEventListener('load', ready);
        ready();
      })
    }
  }

  return new Promise((res, rej) => {
    window.WebComponents.waitFor(res)
  })
}())

@stale
Copy link

stale bot commented Apr 24, 2021

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 stale bot added the wontfix label Apr 24, 2021
@SampsonCrowley
Copy link

SampsonCrowley commented Apr 24, 2021

Really? Won't fix? Webpack is pretty much the de facto standard for web tooling, and making this work with the common standard would do a lot for moving things forward with the community

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants