salesforce / lwc

⚡️ LWC - A Blazing Fast, Enterprise-Grade Web Components Foundation
https://lwc.dev
Other
1.62k stars 394 forks source link

Adding support for native custom element in the template #427

Closed pmdartus closed 4 years ago

pmdartus commented 6 years ago

Is your feature request related to a problem? Please describe. Currently, LWC doesn't support referencing a native custom element in the template. The compiler will transform it as a LWC component and the compilation would fail during bundling since the component would not be present on the disc.

As a workaround today, custom elements can be created imperatively and injected in the template. This approach would not solve the issue of data binding.

Describe the solution you'd like Multiple choices are offered to us here

caridy commented 6 years ago

Let's chat about this.

caridy commented 5 years ago

For the record, we do support this but only if the element is LWC compiled to web component. If it is something else, the compiler will still try to resolve it to something.

caridy commented 5 years ago

another alternative is to add a simple directive, following lwc:dom="manual" in non-custom elements, we can add lwc:resolver="native" only for custom elements. This is a very edge case, and in most cases, if you're doing LWC, all your components will be lwc, even if they are compiled to WC, but still, if you're confident that you're either using a web component that you will register somewhere else, or you just want to have a tagName that uses - on it, but you don't want that to be treated as a custom element, I think a directive is fine.

diervo commented 5 years ago

I like adding a new directive, we just need to bikesheed on the name... :)

davidturissini commented 5 years ago

I have concerns, especially around DX, with adding a new directive to support custom elements. On the surface, the compiler should know which components it can resolve and which components it cannot resolve. If it comes across a tag name that it cannot resolve, why not just create the element as-is and let the browser handle what happens?

diervo commented 5 years ago

@davidturissini that violates a very core invariant of the compiler: We compile one bundle at the time and we have no knowledge of any other component on the system.

Is not business of the compiler to understand the whole world of potential components registered. If anything it should be the registry holding the modules and the metadata for all dependencies (what in platform we call second pass compilation).

There were a lot of discussions in TPAC about this, our take is that we can't just rely on the browser async registration to do this (creates a lot of ambiguity), and we must apply the right directive at compile time, hence the need for a directive to instruct the compile explicitly that we don't know this element.

@caridy will probably have more opinions :)

davidturissini commented 5 years ago

@diervo the compiler knows about namespaces and where to resolve those namespaces, correct? If the compiler encounters a tag name that doesn't fall into that namespace could it just assume that element can just be created natively?

diervo commented 5 years ago

Nope, we just have a special behaviour for a special unique namespace (in this case "c"). @apapko to add more details.

We could technically pass a list of namespaces, but still can be ambiguous... I rather prefer fail fast and be explicit, but I can be convinced of passing namespaces maybe?

caridy commented 5 years ago

Yes, I agree with @diervo here. If you are importing something, that must be present somehow, otherwise it is an error. In our template, we decided to resolve things for you for convenience, and that put us on this spot where the things you use in your template must be resolved by a loader via the import mechanism.

In theory, in the open source, you could literally resolve to a web component exported by some file, and we can make changes in engine to support that case where the Ctor happens to be an extension of HTMLElement instead of LightningElement, and make some adjustments there... but that is not trivial because the VNode for what was supposed to be a custom element is not longer valid, and we need to transform it back into a regular VNode. That's why I think a signal for the compiler to treat the element as a regular element rather than an import, seems to be easier, safer.

jarrodek commented 5 years ago

We are working on a POC for LWC used in Anypoint platform. Also there's a discussion around moving to WC in general. We are considering LWC to use with new component but not being able to use other custom elements is a huge obstacle. We do not want to develop everything by ourselves for one library while there's a lot of existing open WCs (Vaadin, Polymer to name some).

As for new directive for custom elements it feels like a bad idea to me. It's not the web way. Custom elements should be either recognized or ignored (like the UA would do). Therefore @caridy idea seems better to me.

caridy commented 5 years ago

@jarrodek question for you:

do you want to bring other WC inside the templates that you author in LWC? or you're thinking on writing them all in LWC, and give them to your consumers as registered WC, while also you use them in your templates?

It is a complex question, apologies if it is confusing. I just want to clarify that if you have 3 components, and they all use each other as LWC, they will work fine even if they are registered under the same tagName, that works fine today, but using a polymer component inside a LWC template, is the problem that we are trying to solve here.

jarrodek commented 5 years ago

And this is my problem with current state. I have a library of over 200 components build on top of Polymer/LitElement library. This set of components is build for API tooling. I don't see a path for me to rewrite them to LWC to use them in our other components based on LWC.

caridy commented 5 years ago

Got it! thanks. Last question for you @jarrodek, e.g.:

<template>
    <x-foo bar={something}></x-foo>
</template>

If x-foo is written in polymer, do you want your LWC component to import it and get it ready? or do you prefer the LWC component to assume that this dependency is provisioned by someone else at some point before or after? Basically, explicit vs implicit imports of those external deps?

jarrodek commented 5 years ago

What I would like to do is to import x-foo from npm. The component is not pre-processed so LWC would have to import it and get it ready.

diervo commented 5 years ago

Couple of possibilities I can think of:

1) You explicitly add a directive: You own the LWC component and you know what you depend on is not and LWC component, therefore explicitly stating that a component is external should be fine. Seems for your use case that this is exactly the use case you are looking for.

2) We could have a flag which will allow any non-resolved import to be a yet-to-be-upgraded-element, hence allowing the browsers to do its thing. You can do this today but resolving a particular import from a component to an empty element... The problem with this approach is that is ambiguous: if you load that component from LWC at a later time, what happens?

3) You could just build every LWC component independently as a pure WC, but then you get none of the benefits, of reactivity, bindings, etc.

At the end of the day, you know which components you are bringing from where, so there is nothing stopping you to implement it whatever you want at build time where you do that equivalent whitelisting.

Let me restate my position that we will not implement anything in the compiler with a list of things to indicate wether or not a component is "external" or not. Anything that requires "global" knowledge of the world is a no-go.

caridy commented 5 years ago

Here is my proposal implementation: https://github.com/salesforce/lwc/pull/1379

It doesn't require any changes anywhere, and the import from templates must be still implicit and resolved by the LWC resolver, even though it resolves to a vanilla component.

diervo commented 5 years ago

My proposal somewhat related to what @davidturissini said but without making the list on the compiler: https://github.com/salesforce/lwc-todomvc/pull/28/files

diervo commented 5 years ago

If we want to really preserve the semantics of WC and the lazy upgradability, it means that we can't make assumptions on when the constructor will be available, which we are doing on your proposal (we generate the import automatically at compile time hence the import must be resolved there).

What if we combine both proposals? In my proposal for all non resolved components (at build time) I create an empty component, maybe we can have a special constructor so we can do those checks easier at runtime? That will allow for both resolution at build time or runtime.

davidturissini commented 5 years ago

I think an issue here is that both implementations assume that the developer has access to the custom element constructor.

I’m concerned that these approaches will not be compatible with developers loading custom elements from a CDN and simply using them with the html tag.

jarrodek commented 5 years ago

I am not sure how the engine works so correct me if I am wrong. Why can't we just allow custom elements in the <template> to be as any other element which is not known LWC and just update properties / attach listeners as declared in the template? From LWC PoV it doesn't matter when exactly the element gets upgraded from unknown element to x-foo. Properties will still be there, event listeners too. General logic will hold. Modules in most cases are loaded with the component so at the time when LWC logic is executed the x-foo component is already registered and upgraded. In other cases (lazy load, whatever) it's author's problem to make sure the component is upgraded before accessing it's API.

caridy commented 5 years ago

@jarrodek that's not possible until after we get the scope registry! otherwise you have to register all components as web components, and they how to apply access checks in lightning platform? That's not how LWC works for that particular reason.

rtm commented 5 years ago

I know very little about your internal architectures, so am unable to say anything intelligent on this topic other than yes, we need to be able to call regular-old WCs from within LWCs, and no, I would prefer not to have to pre-process them, or wrap them, or declare them somehow.

I don't even understand what the template compiler does. So I also don't understand why it needs to "resolve" anything. In the lit-element world or low-level WC world, the compiler just compiles a command to create an element with some name, and when the HTML is "executed" the browser instantiates the <my-custom-element> element, and then if it's registered, or when it is, the logic in the class specified at time of registration kicks in.

Is it not an option to change the LWC template compiler to be more "stupid" and not resolve anything but simply compile unknown tags into the logic for creating an HTML element by that name, use the browser's native CustomElementRegistry.define() to register the LWCs, and let the user import the JS via <script> tags or whatever which represent the non-LWC custom elements with their associated calls to register themselves? Does this break down because the environment doesn't or can't use the native CustomElementReigistry.define, or because it can't easily import chunks of JS such as might result from a build step on a stencil repo, although I guess you could use loadScript (although it has always seemed strange to me to load a script, which one would normally think of as being a one-time affair, by means of a component which in theory might be instantiated multiple times, and a bit of a perversion to use "components" for such "control" tasks.

Anyway, sorry for the confused post. If there are questions in there which are meaningful and have reasonable answers, I'd love to hear them.

priandsf commented 4 years ago

I've already been facing the issue several times, the last one came with the integration of Google's model-viewer within an LWC Commerce application. The solution was to modify the DOM in renderedCallback(), which is cumbersome and doesn't take advantage of the virtual DOM. For more complex cases, like the use of a-frame, it is impractical. I'm also not a fan of an LWC directive which is not builder friendly (I hate lwc:dom as it forces us to create wrapper components). I understand the value of compile time checking, so I would suggest a directive at the component class level (or the associated template), saying that unknown component tags should be passed through.

jarrodek commented 4 years ago

@caridy I would suggest an alternative mechanism for adding support for custom external elements without scoped registry. The idea is based on the upcoming scoping registry spec and the project that folks from @open-wc were working on. It's a scoped elements mixin that allows to internally register an external WC without this component to be globally registered. What it does (in standard web) is:

In LWC this could work quite similarly :

This would allow to bring other components to LWC much faster than scoped elements implementation.

diervo commented 4 years ago

I think first we should allow bringing components from other frameworks to which caridy had a PR half done we need to finish, this will allow OSS to interoperate all the things.

We could even explore having Locker making sure the different developers can register namespaces they own to allow this is platform too.

As per the scoped elements, @leobalter is going to be pushing more directly into the proposal to see if we at least can land the API ergonomics settled soon.

The limitations on the approach @jarrodek mention are a no-go:

First because there is an assumption that you already know which components you need to "scope", which is not true when you don't own the full page (which is the case of platform and many other apps)

And second because we can't just tell our customers "please don't reference your custom elements in your css or don't try to find them via querySelector... The whole thing is just a hack.

I'm not going to even get in the perf argument... :)

A lot of people are asking for interop, so I'm completely onboard of finish the work to allow 3party wc to work seamlessly. But as per scoped registries we can't do half-done implementations. Let's do it right.

I would even entertain the idea of have an experimentalScopedFlag or something with a ponyfill if someone wnats to do the work.

caridy commented 4 years ago

@ravijayaramappa promised to work with me on that soon :)

@leobalter, Justin and myself were in a meeting today to unblock scoped custom registry progress... we are now starting to write the spec (good progress today). Once we get a clear direction from the group, we can align LWC with that even if we don't get them in browsers yet.

jarrodek commented 4 years ago

@diervo let's discuss and not dismiss at first glance. I am looking for ways that we can co-op.

The first you said about knowing what has to be scooped. I am not sure if you got the idea right. You (as a component author) are only interested in the components you have included inside your component. Take a look on an example use in a new project I am working on. In here literally no component is registered in the registry until the logic of the mixin kicks in and performs own registration. Other component, even from the same package would register different names to ensure that you cannot reuse someone's component. But in the end you don't care (as an author) what other components may have been registered / used on the page. Obviously giving the specific requirements for the core this would probably work somewhat differently (idk, like having registration enabled per namespace to speed up things a bit) giving that the DOM in the core is synthetic. There's a field to play around.

With the second argument, you tell your customers to divert from web standards already by asking them to use #template instead of #shadowRoot, you tell them to use classes instead of ids, to use classes instead of attributes, to scaffold a project in a very specific way. Why suddenly you think this would be something that would be an inconvenience?

Finally the performance. According to open-wc they noticed 8% perf degradation. I am pretty sure you could improve this as open-wc solves different problem (registering of multiple versions of the same component) vs no being able to register someone else's component as own and hijack component definition (at least according to one of our discussions about that).

I am not saying that this is the solution. Probably it isn't. I see the limitations. Currently I am experimenting with this approach to see how this work in practice. I would like to point out different directions to evaluate. I noticed that in @caridy 's PR he works on already registered in the registry components (via packages/integration-karma/test/component/native-components/x/child/child.js). Maybe (maybe!) the right approach would to take control over registration of a foreign component by requiring to register this component with LWC engine like others LWC components (with ccreateElement)?