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

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

Closed
theengineear opened this issue Jul 19, 2020 · 12 comments
Closed

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

theengineear opened this issue Jul 19, 2020 · 12 comments

Comments

@theengineear
Copy link
Collaborator

theengineear commented Jul 19, 2020

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
Copy link
Collaborator Author

@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
Copy link
Collaborator

klebba commented Jul 20, 2020

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
Copy link
Collaborator Author

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
Copy link
Collaborator

klebba commented Jul 20, 2020

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
Copy link
Collaborator Author

theengineear commented Jul 20, 2020

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
Copy link
Collaborator Author

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
Copy link
Collaborator

klebba commented Jul 20, 2020

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator

klebba commented Jul 22, 2020

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

@klebba
Copy link
Collaborator

klebba commented Jul 23, 2020

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

@theengineear
Copy link
Collaborator Author

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

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

2 participants