Skip to content
This repository has been archived by the owner on Sep 20, 2019. It is now read-only.

Should libraries import the polyfill? #588

Closed
trusktr opened this issue Aug 6, 2016 · 3 comments
Closed

Should libraries import the polyfill? #588

trusktr opened this issue Aug 6, 2016 · 3 comments

Comments

@trusktr
Copy link

trusktr commented Aug 6, 2016

If I'm making a library that gives the user of the library custom element classes, should I also import the Custom Elements polyfill? Or should I mention that the library should run in an environment that supports Custom Elements, and suggest ways they can get the polyfill if needed?

I have a feeling importing the polyfill and assuring it's only applied if the API isn't already present is the best way to go, but I just wanted to get a second opinion.

@treshugart
Copy link
Contributor

Short answer, I wouldn't.

If you're distributing a custom element - or custom elements - to be consumed by third parties, I'd consider not importing because if you do, you'd be making assumptions about the consumer's browser support matrix. For example, they may only care about v1 supported browsers and not need the polyfill.

They may also be including the same polyfill (or a different version). Thus you're also making assumptions about the dependencies they're bringing in. You can dedupe in Bower and NPM, but then you're making assumptions about package managers and in part about build systems due to path resolution semantics.

There are cases where you might want to in order to make consumption simpler. For example, at Atlassian, I work on the front-end component library used in all the products. Since products are pretty much guaranteed to use our component library, we can make certain assumptions about this, but I'd say this would be an edge case in the grand scheme of things.

@trusktr
Copy link
Author

trusktr commented Aug 6, 2016

@treshugart Thanks Trey, currently my classes are written as v1 classes, so I have a WebComponent base-class mixin like this:

// code shortened for simplicity, missing mixin caching and some other things.

let createWebComponentClass = elementClass =>
class WebComponent extends elementClass.prototype instanceof HTMLElement ? elementClass : HTMLElement {

    // constructor() is used in v1 Custom Elements instead of
    // createdCallback() as in v0.
    constructor() {
        super()

        // If the following is true, then we know the user should be using
        // `document.registerElement()` to define an element from this class.
        // `document.registerElement()` creates a new constructor, so if the
        // constructor here is being called then that means the user is not
        // instantiating a DOM HTMLElement as expected because it is required
        // that the constructor returned from `document.registerElement` be used
        // instead (this is a flaw of Custom Elements v0 which is fixed in v1
        // where class constructors can be used directly).
        if (document.registerElement && !customElements.define) {

            // TODO: link to docs.
            throw new Error(`
                You cannot instantiate this class directly without first registering it
                with \`document.registerElement(...)\`. See an example at http://....
            `)

        }

        if (!document.registerElement && !customElements.define) {

            throw new Error(`
                Your browser does not support the Custom Elements API. You'll
                need to install a polyfill. See how at http://....
            `)

        }

        // otherwise the V1 API exists, so call the createdCallback, which
        // is what Custom Elements v0 would call, and we're putting
        // instantiation logic there instead of here in the constructor so
        // that the API is backwards compatible.
        this.createdCallback()
    }

    createdCallback() {
        // ...
    }

    connectedCallback() {
        // ...
    }
    attachedCallback() { this.connectedCallback() } // back-compat with v0

    async disconnectedCallback() {
        // ...
    }
    detachedCallback() { this.disconnectedCallback() } // back-compat with v0
}

Classes that extend from it then use v1 API, except that constructor logic is still placed into createCallback as you see that requirement up there in WebComponent:

class SomeElement extends createWebComponentClass(HTMLButtonElement) {
    // I am purposefully avoiding using `constructor` here so that the component
    // works in both v0 and v1. But if for example we know we are in a v1
    // environemnt, then it would be fine to use the constructor.

    createdCallback() {
        super.createdCallback()
        // ...
    }

    connectedCallback() {
        super.connectedCallback()
        // ...
    }

    disconnectedCallback() {
        super.disconnectedCallback()
        // ...
    }

    attributeChangedCalllback() {
        super.attributeChangedCalllback()
        // ...
    }
}

Currently the library I'm working on uses document.registerElement directly, which is why I'm importing a polyfill, like this:

import 'document-register-element' // Note, this
import createWebComponentClass from 'somewhere'

let SomeElement = class SomeElement createWebComponentClass(HTMLButtonElement) {
  // ...
}

// It would seem to use `document.registerElement` without importing the polyfill in an environment where modules are highly promoted.
SomeElement = document.registerElement('some-element', SomeElement)
// ^ this is why I'm sticking to `createdCallback`, otherwise constructor logic will fail to run.

export {SomeElement as default}

But, this pre-defines what the name of the element will be (some-element in the example), and therefore requires that a polyfill be imported in order for the code to be 'proper' within a module-based environment.

With that said, I was thinking that I'd much rather encourage end-users to name their elements, and I think that really makes sense, especially in light of the fact that we don't have something like scoped components, and therefore I would hate for someone to have to go through the pain of my pre-defined names to clash with some other library in the future (even if it's unlikely).

So, I was thinking of changing the previous example to this:

import createWebComponentClass from 'somewhere'

class SomeElement createWebComponentClass(HTMLButtonElement) {
  // ...
}

// no more document.registerElement here.

export {SomeElement as default}

and then the documentation for my library would suggest that the user do something like this in which they obtain the polyfill themselves if needed:

// in a v1 environment:

import 'custom-elements-v1' // if needed
import SomeElement from 'some-element'

customElements.define('any-name-desired', SomeElement)

let el = new SomeElement
// ...

or

// in a v0 environment:

import 'document-register-element' // if needed
import SomeElementClass from 'some-element'

const SomeElement = document.registerElement('any-name-desired', SomeElementClass)

let el = new SomeElement
// ...

The docs would also suggest that they could load the polyfills with script tags and not need to import them in the modules:

<script src="registerElement.js"></script>

or

<script src="customElements.v1.js"></script>

and the previous JS example would be the same except without importing the polyfills. If custom element authors make it a pattern to encourage their users to name the elements themselves, then it will make the whole Custom Element ecosystem easier to work with in cases of name collisions (especially in big applications), without needing to export some helper tool for that.

@justinfagnani
Copy link
Contributor

justinfagnani commented Aug 7, 2016

I'm mobile, so this is short, but never import polyfills, always let the application choose the particular implementation to use.

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

No branches or pull requests

4 participants