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

add error reporting when html template versions conflict #1195

Closed
DanielKehoe opened this issue Jul 30, 2020 · 10 comments
Closed

add error reporting when html template versions conflict #1195

DanielKehoe opened this issue Jul 30, 2020 · 10 comments

Comments

@DanielKehoe
Copy link

Description

Suggestion to improve the developer experience by providing a helpful error message when conflicting versions of html have been imported.

I'm using LitElement and LitHtml to build custom elements and importing necessary modules using a smart CDN (no Node or build tools). I've discovered that custom elements built on LitElement tend to break if multiple versions of LitHtml are imported. I discovered this when my deployed custom elements broke after the LitHtml version 1.3.0-pre.1 was released. Due to an error at jspm.dev (that has since been fixed), my code was importing different versions of html and my browser displayed [object Object] instead of the custom element output. Hopefully it is unlikely that anyone will again experience this problem, but should it happen again, it would be better for the user experience if nothing was displayed in the browser and better for developer debugging if an error message was displayed in the console.

Steps to Reproduce

  1. Write this code
<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>JS Bin</title>
</head>
<body>

  <script type="module">
    import { LitElement } from 'https://jspm.dev/lit-element@2.3.1';
    import { html } from 'https://jspm.dev/lit-html@1.3.0-pre.1';
    class HelloWorld extends LitElement {
      render(){
        return html`
          <p>Hello World</p>
        `;
      }
    }
    customElements.define('my-hello', HelloWorld);
  </script>

  <my-hello></my-hello>

</body>
</html>
  1. See this output...

Live Reproduction Link

https://jsbin.com/muyipicusu/edit?html,output

Expected Results

Without conflicting versions of html, the browser displays Hello World.

With conflicting versions of html, it would be ideal to see nothing displayed in the browser and a console error with a message about the error such as "conflicting versions of html imported".

Actual Results

With conflicting versions of html, the browser displays [object Object].

Browsers Affected

I've checked Chrome only.

@justinfagnani
Copy link
Member

The problem here is that if the check instanceof TemplateResult fails due to two instances of the library, we don't have a way of telling that the object was actually a lit-html TemplateResult from the copy. If we could, maybe we could use it.

@sorvell
Copy link
Contributor

sorvell commented Jul 30, 2020

Just a quick note: removing the use of instanceof TempalteResult is being considered for lit-html 2.0 for just this reason. See #1182.

@guybedford
Copy link

@justinfagnani responding on this since it came up in a jspm scenario - it might possible to use a heuristical branding technique like a unique string on the TemplateResult prototype. Given that this is a surprising user error otherwise, coming up with a correcting error, at least in a development build, might be beneficial to users.

Since individual components all import from lit-html multiple running versions does seem like it will be common though given the difficulties with peerDependencies - perhaps this might even want to be explicitly allowed, but being able to warn at least seems useful.

@justinfagnani
Copy link
Member

@guybedford yeah, we're considering this for 2.0. Currently we don't have a development or production build at all, we published the unminified ES2017. So we don't have a great way of adding extra checks or error messages that won't bloat production. We do push lit-html versions to a global array named litHtmlVersions so we can check that for multiple versions, but we don't have a check now. It's also not always the case that multiple versions is a problem - if they're isolated within a component and don't share objects it's fine.

But also as part of 2.0 we're likely going to publish a minified production build by default, then have a separate npm package with a development build with extra checks that were compiled out of the prod build. Developers will be able to use the dev build via something like Rollup's alias plugin.

@guybedford
Copy link

Note that the package.json "exports" field can permit "development" and "production" conditions... we're just working on getting tools to want to use that. jspm right now will select the "development" condition though by default on jspm.dev. I did try and get a node --dev PR going but that was blocked so we will likely ship a node --conditions=development in the coming weeks I expect (plus supporting any other custom exports conditions).

Separating the development build as another package makes the library story tricky. I wouldn't advise following React here in their bad directions :) Portable components are the big story.

@guybedford
Copy link

I believe Webpack may support "development" and "production" exports conditions as well, but perhaps @sokra can confirm too.

@sokra
Copy link

sokra commented Jul 31, 2020

Yes webpack 5 support development and production as conditions depending on the mode set.

@DanielKehoe
Copy link
Author

I just want to advocate loudly here for development without reliance on Node and build tools. The attraction (for me) of combining web components with ES6 modules is building custom elements with just a browser and text editor. Will that be possible without surprises from dependency conflicts?

@guybedford
Copy link

Will that be possible without surprises from dependency conflicts?

Not on jspm.dev unfortunately since conflicts are always possible, unless lit-html is made to support these cases more gracefully.

For the jspm CLI and import map generator it will be possible to ensure a single resolution though, which is the new tooling being released in a few weeks.

@justinfagnani
Copy link
Member

In lit-html 2.0 and going forward we detect TemplateResult with a specially property. This makes TemplateResults compatible across versions. If we make a breaking change to TemplateResult we'll change the property name.

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

No branches or pull requests

5 participants