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

css: false does not disable CSS injection for customElement builds #4124

Closed
pospi opened this issue Dec 18, 2019 · 5 comments
Closed

css: false does not disable CSS injection for customElement builds #4124

pospi opened this issue Dec 18, 2019 · 5 comments

Comments

@pospi
Copy link

pospi commented Dec 18, 2019

In pursuit of #4117 I'm trying to generate both Svelte and WebComponent versions of my build in a way that retains separation between the style and script bundle. If I use css: false with the default compilation mode this does exactly what I want- add_css is no longer injected into the output component code.

However, when using customElement: true the component still ends up with styles being injected via assignment to this.shadowRoot.innerHTML. So I think there is a possible bug with related questions:

  • Could the compiler be changed such that no CSS is added to customElement versions of components if css: false is specified?
  • If not, is that because there isn't yet a workable way to dynamically inject styles into a WebComponent at runtime?
  • Would it be possible to inject a custom method into the customElement build output that allows such dynamic injection of styles?

Running Svelte 3.16.4; Ubuntu 18.04; Firefox 71. Not using Webpack or Rollup (doing low-level compiler integrations).

@Conduitry
Copy link
Member

If not, is that because there isn't yet a workable way to dynamically inject styles into a WebComponent at runtime?

That's my understanding of the current situation, yes. See point 2 of Rich's post: https://dev.to/richharris/why-i-don-t-use-web-components-2cia

Given that, I don't think it makes much sense to let the CSS be extracted - what would you do with it?

@pospi
Copy link
Author

pospi commented Dec 18, 2019

Hmm, it feels to me like these tradeoffs could be workable. I don't really care about FOUC; because I think the reality with runtime-heavy frameworks like React and Angular is that you get a FOUC waiting for the render step anyway; which is why you need to use SSR and client-side hydration. What I'm saying is- you should deal with FOUC by separating your CSS and JS in your build process and rendering all of it statically into the head & body HTML blocks on output; not by ensuring your components come with CSS bundled internally.

Given that the style assignments are done in the constructor of SvelteElement, and that extends HTMLElement, there's already a runtime hook that's performing the style injection- it's not embedded. So I'm pretty sure all I need to do get custom styles integrated is to create a higher-order function which creates SvelteElement subclasses that have custom constructor logic encapsulated in a closure that accepts a style / theme object.

So where previously a customElement build would output a class like this:

class MyComponent extends SvelteElement {
  // ...
}

customElements.define("my-component", MyComponent);
export default MyComponent;

I would have it instead output this:

class MyComponentUnstyled extends SvelteElement {
  // ...
}

export function createStyledElement(styleString) {
  return class styled extends MyComponentUnstyled {
    constructor(options) {
      this.shadowRoot.innerHTML = `<style>${styleString}</style>`;
      super();
    }
  }
}

const MyComponent = createStyledElement("...");

customElements.define("my-component", MyComponent);
export default MyComponent;

If css: false is specified in the compiler options, the output would be conditionally changed to:

  • skip the const MyComponent = createStyledElement("..."); line and customElements.define call
  • export default MyComponent; is removed- the only export for an unstyled component is the named createStyledElement function.

This gives the developer all necessary flexibility to create new component sub-classes with individual, runtime-defined styles.

There might be complications depending on when the constructor of an HTMLElement is called- if it's at the time of calling customElements.define rather than when initialising each element, then there may indeed be FOUC issues. Couldn't find info on this on MDN. Worth experimenting with- but I think this will require the above PRs to Svelte itself in order to do cleanly. And it may be the case that adding the additional wrapper function is considered code bloat... no reason that MyComponentUnstyled can't also be passed in to createStyledElement, though.

@pospi
Copy link
Author

pospi commented Aug 24, 2020

I've made a pass at this, based on what I believe should be the expected behaviour. But it feels strange in a way that bears talking about— confusion RE the css compiler option:

It's recommended that you set this to false and use the CSS that is statically generated, as it will result in smaller JavaScript bundles and better performance.

What does this mean regarding the expectations of a customElement build target with Svelte? My interpretation was always that css: false in the config meant "omit styles", but is it actually more like "don't do anything else with styles"?

Regardless, I think I'd like to see the addition of runtime injection for customElement CSS; and the requisite code changes underpinning the change needed to address this issue are what provide for this facility. Essentially, there are now higher-order functions for each component that allow one to inject custom CSS at app initialisation:

customElements.define("someCustomStyledComponent", createStyledElement(someCustomCSSString));

See also #4117 (comment) where I am keeping an informal requirements list & test cases for a related issue building on this.

@stale
Copy link

stale bot commented Jun 27, 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 27, 2021
@stale
Copy link

stale bot commented Jul 11, 2021

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

@stale stale bot closed this as completed Jul 11, 2021
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

2 participants