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

Feature: allow components to opt out of custom elements build #4228

Closed
DesignByOnyx opened this issue Jan 8, 2020 · 12 comments
Closed

Feature: allow components to opt out of custom elements build #4228

DesignByOnyx opened this issue Jan 8, 2020 · 12 comments

Comments

@DesignByOnyx
Copy link

Is your feature request related to a problem? Please describe.

Minor problem: when using customElement: true, it would be nice if individual components could opt out of inheriting from HTMLElement (SvelteElement) somehow and instead inherit from SvelteComponent. This would allow component library authors to have control over "public" components vs internal components. Consider this:

Outer.svelte

<svelte:options tag="my-custom-tag" />
<script>
	import Nested from './Nested.svelte';
</script>
<Nested />

Nested.svelte

<div>Hello World</div>

When using <my-custom-element> in another application, it results in a "Illegal constructor" error. The error happens because the Nested component inherits from HTMLElement but is never defined with customElement.define(...). When Svelte goes to instantiate the Nested component, the browser complains... rightfully so. The request I am making is this: I shouldn't have to define a custom element for my Nested component. It's sort of "private" and should not be consumable outside of the Outer component.

Describe the solution you'd like
Only elements which define a "tag" should inherit from HTMLElement (SvelteElement), including tag={null}. If a component does not define a "tag", then let it inherit from SvelteComponent.

How important is this feature to you?
This would be pretty nice to have as we are evaluating different tools and the ability to control which components get exported.

Additional context
We are building a component library and we will have dozens of "components" used throughout, but our "public" interface is really only 8 or 9 components.

@Conduitry
Copy link
Member

I don't think we want to change components compiled with customElement: true but no <svelte:options tag/> to not actually compile to custom elements - this would be a breaking change,

The compiler itself is only ever compiling one component at a time, so if you have complete control over how it's getting called, there's nothing that actually needs to change here.

This, then, I think, is more of a request for the bundler plugins. I sort of recall that there is a built-in way in webpack to get it to use loader configs that vary per file (although I don't remember how to use it), but I don't believe there is in Rollup. I'm not sure what sorts of mechanisms the ecosystem has settled on for handling this in Rollup plugins - probably specifying callbacks that are passed the path of the current file.

@ghost
Copy link

ghost commented Jan 8, 2020

Playing a bit of devil's advocate, it does seem silly that the compiler will moan about <svelte:options tag/> being present instead of just inferring that it means customElement: true. Think of it as a customElement != true meaning if it has a tag then it is a custom element and the normal customElement == true meaning it must have a tag otherwise bail out.

The problem is that this compiler option is already well-established and to change it now might bend some projects way out of shape.

@DesignByOnyx
Copy link
Author

DesignByOnyx commented Jan 8, 2020

Thanks for the feedback. I made this request based on looking at the compiled code, so let me see if I can clarify.

First, I agree wholewheartedly about the backwards compat thing. I think @dkondrad gets where I was going with the "absense" of a tag meaning "not a custom element". However, I have no opinion on how this should be implemented, and for the sake of conversation let's say that a special value tag="NONE" might satisfy what I am asking. If the svelte compiler sees this special value, then the bundled code would look like this for the Nested component:

// <svelte:options tag="my-custom-tag" />
class Outer extends SvelteElement { ... } //-> extends HTMLElement
customElements.define('my-custom-tag', Outer);

// <svelte:options tag="NONE" />
class Nested extends SvelteComponent { ... } // -> does not extend HTMLElement

As it currently stands, the Nested component always extends the SvelteElement, and this suffers from the following issue (type this in your console):

class Foo extends HTMLElement {}
new Foo();  // Illegal construction
customElements.define('my-foo', Foo);
new Foo();  // <my-foo></my-foo>

@DesignByOnyx
Copy link
Author

DesignByOnyx commented Jan 10, 2020

As I'm playing more with svelte, this same issue is causing me other problems. Because of the all-or-nothing nature of customElement: true, it breaks the ability to use "normal" svelte components and get all their goodness. For example, the following REPL does not work with customElement: true:

https://svelte.dev/repl/282605849ac046da81cdc880b065a3ae?version=3.16.7

The problem is that when AsyncRenderer is compiled as a webcomponent, it can only be consumed as a web components (eg. <async-renderer ...>) which causes slots to work differently. It's hard to describe, but if you were to download that REPL and set customElement: true, you will observe the following:

  • Using <AsyncRenderer {promise}> doesn't render styles, complains that required prop "promise" is not set, the slotted content never renders
  • Using <async-renderer {promise}> doesn't have dynamic slot goodness, making the component useless - the slot renders once early and then never again.

@ghost
Copy link

ghost commented Jan 10, 2020

@DesignByOnyx,

Yeah, mixing and matching would need custom bundler support or automatic detection that I mentioned.
On somewhat related topic, custom elements don't work right in the REPL... see sveltejs/svelte-repl#79.

@mrbarletta
Copy link

I am not able to use Svelte Material UI Components when setting customElement:true as it complains about the illegalConstructor.

I have no preference for how this should be handled but I don't think it should be All or Nothing scenario.

@jawish
Copy link

jawish commented May 9, 2020

Running into the same issue. I tend to agree with @DesignByOnyx #4228 (comment).

Not too familiar with the compiler yet, but might adding a customElements=true/false option to svelte:options be a workable solution instead of the global setting?

@jawish
Copy link

jawish commented May 10, 2020

I've fixed this by changing the build/bundling via Rollup.

Svelte Components to be built as Custom Elements get the file extension *.wc.svelte and normal Svelte components retain the *.svelte extension. The *.wc.svelte components have the svelte-options with tag set.

The Rollup config then sets the Svelte "customElement" custom depending on the file extension.

svelte({ customElement: true, include: /\.wc\.svelte$/ }),
svelte({ customElement: false, exclude: /\.wc\.svelte$/ }),

I've put up a demo git repo in case anyone wants to pursue the same solution: https://github.com/jawish/svelte-customelement-rollup

@akauppi
Copy link

akauppi commented Jan 17, 2021

@jawish I took your solution to use, with a little naming twist:

svelte({  // web components
  include: /\/[a-z][^/]+\.svelte$/,
  compilerOptions: {
    customElement: true 
  }
}),
svelte({  // normal Svelte classes
  include: /\/[A-Z][^/]+\.svelte$/,
  compilerOptions: {
    customElement: false
  }
}),

Web components are lower-case.svelte whereas normal ones are Capital.svelte.


As a welcome side effect, this also solved another problem: was not able to pass props to subcomponents. Now, though using $$props['aaa'] instead of let aaa, I am. 😄

@Crono1981
Copy link

Many thanks to @jawish and @akauppi for this clever workaround! Works like a charm!

@stale
Copy link

stale bot commented Jun 29, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Jun 29, 2021
@stale
Copy link

stale bot commented Jul 13, 2021

This issue has been closed as it was previously marked as stale and saw no subsequent activity.

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

No branches or pull requests

7 participants