Netflix / x-element

A dead simple starting point for custom elements.
Apache License 2.0
28 stars 12 forks source link

Allow import of x-element with lit-html imported from unpkg. #61

Closed theengineear closed 4 years ago

theengineear commented 4 years ago

While the supported way to use x-element is to perform a flat-install of lit-html as a sibling to the x-element such that the import ../../lit-html/lit-html.js will work — this presents a barrier to entry to trying x-element.

Since lit-html supports import from unpkg, we could expose a file with alternative imports that don't require the special setup. It would also allow online code editors import and use the element.

The following script would simply replace the import strings with new ones:

"bundle": "cat x-element.js | sed \"s/\\.\\.\\/\\.\\.\\/lit-html\\/\\([^']*\\)/https:\\/\\/unpkg\\.com\\/lit-html\\/\\1?module/\" > x-element-bundle.js"

E.g.,

import { asyncAppend } from '../../lit-html/directives/async-append.js';

turns to...

import { asyncAppend } from 'https://unpkg.com/lit-html/directives/async-append.js?module';

Then, you could pop into a code pen or something and write import XElement from 'https://unpkg.com/@netflix/x-elementx-element.js' and it would just work. I think this is pretty powerful and worthwhile.

I'd like to add this as part of #58.

Note that a simple gut-check test could be as follows where we minimally check functionality and assert that the text in the bundle file is as expected:

import XElement from '../x-element-bundle.js';
import { assert, it } from '../../../@netflix/x-test/x-test.js';

class TestElement extends XElement {
  static get properties() {
    return {
      property: {
        type: String,
        initial: 'Ferus',
      },
    };
  }
  static template(html) {
    return ({ normalProperty }) => {
      return html`<div id="normal">${normalProperty}</div>`;
    };
  }
}
customElements.define('test-element', TestElement);

it('bundle smoke test', () => {
  const el = document.createElement('test-element');
  document.body.append(el);
  assert(el.shadowRoot.getElementById('normal').textContent === 'Ferus');
});

it('bundle was built', async () => {
  const srcText = await (await fetch('../x-element.js')).text();
  const bundleText = await (await fetch('../x-element-bundle.js')).text();
  const expectedText = srcText.replace(
    /\.\.\/\.\.\/lit-html\/([^']+)/g,
    'https://unpkg.com/lit-html/$1?module'
  );
  assert(bundleText === expectedText);
});
theengineear commented 4 years ago

@klebba — what would you think about this? I think that if there's not a dead-simple way to "try it before you buy import it", folks aren't going to be able to easily tell if they should use it.

Obviously, feel free to suggest different names.

klebba commented 4 years ago

Let's discuss; I'm not sure I follow how this would impact the code yet. A diff depicting x-element-bundle.js might help

theengineear commented 4 years ago

It's just the imports. You can see the diff in #58. To be clear, it's not a diff, it would be a new file. I think what you're asking for is what's different between x-element.js and x-element-bundle.js? Here's that:

// x-element.js imports
import { asyncAppend } from '../../lit-html/directives/async-append.js';
import { asyncReplace } from '../../lit-html/directives/async-replace.js';
import { cache } from '../../lit-html/directives/cache.js';
import { classMap } from '../../lit-html/directives/class-map.js';
import { directive, html, render, svg } from '../../lit-html/lit-html.js';
import { guard } from '../../lit-html/directives/guard.js';
import { ifDefined } from '../../lit-html/directives/if-defined.js';
import { live } from '../../lit-html/directives/live.js';
import { repeat } from '../../lit-html/directives/repeat.js';
import { styleMap } from '../../lit-html/directives/style-map.js';
import { templateContent } from '../../lit-html/directives/template-content.js';
import { unsafeHTML } from '../../lit-html/directives/unsafe-html.js';
import { unsafeSVG } from '../../lit-html/directives/unsafe-svg.js';
import { until } from '../../lit-html/directives/until.js';
// x-element-bundle.js imports
import { asyncAppend } from 'https://unpkg.com/lit-html/directives/async-append.js?module';
import { asyncReplace } from 'https://unpkg.com/lit-html/directives/async-replace.js?module';
import { cache } from 'https://unpkg.com/lit-html/directives/cache.js?module';
import { classMap } from 'https://unpkg.com/lit-html/directives/class-map.js?module';
import { directive, html, render, svg } from 'https://unpkg.com/lit-html/lit-html.js?module';
import { guard } from 'https://unpkg.com/lit-html/directives/guard.js?module';
import { ifDefined } from 'https://unpkg.com/lit-html/directives/if-defined.js?module';
import { live } from 'https://unpkg.com/lit-html/directives/live.js?module';
import { repeat } from 'https://unpkg.com/lit-html/directives/repeat.js?module';
import { styleMap } from 'https://unpkg.com/lit-html/directives/style-map.js?module';
import { templateContent } from 'https://unpkg.com/lit-html/directives/template-content.js?module';
import { unsafeHTML } from 'https://unpkg.com/lit-html/directives/unsafe-html.js?module';
import { unsafeSVG } from 'https://unpkg.com/lit-html/directives/unsafe-svg.js?module';
import { until } from 'https://unpkg.com/lit-html/directives/until.js?module';
klebba commented 4 years ago

Ok I think I understand — does this need to be included with #58 or can we review it independently? I'm not averse to the idea, but would want more time to deliberate

theengineear commented 4 years ago

No it doesn't need to be included — I'm just feeling that if we call #58 1.0.0, we should have a simple way to try it.

Also, I only added it there because I figured you'd ask to look at it 😉

theengineear commented 4 years ago

I guess either way, what'd be more important is some documentation on how to use x-element. I wouldn't be familiar with the pattern of doing a flat install of all of my dependencies such that my application can find them via ../../ later on. We could also document that you need to just use element-server to run your application.

Either way, the idea is that it's a lot to ask folks to do all these backflips before they even know if they want to use it.

klebba commented 4 years ago

I see — let's not couple the merge of #58 to 1.0.0 necessarily, but it should follow soon after. If you want to open a PR that bases off that branch, I'm happy to deliberate this approach in more detail in a separate thread

theengineear commented 4 years ago

Any reason we can't just come to an agreement in this issue thread? Perhaps I'm misunderstanding though — is your ask for me to try and think of other ways we might be able to do this and enumerate them?

theengineear commented 4 years ago

I can also open up a different branch. But the change set isn't really that interesting.

  1. Add a new file called x-element-bundle.js with different imports.
  2. Test that the content of x-element-bundle.js was updated on new changes.
  3. Publish that file alongside x-element.js.
klebba commented 4 years ago

Yeah, I do think we should address this separately — I want to discuss potential alternatives and possibly fiddle a bit

klebba commented 4 years ago

Summarizing our chat: if we remove ?module from the unpkg.com paths, and stick with relative paths, this will "just work" 👨‍🎤

theengineear commented 4 years ago

Going to close this down since @klebba already set us up for success here 🎉