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

feat(runtime-dom): Apply nested component styles when using defineCustomElements #4309

Closed
wants to merge 1 commit into from
Closed

feat(runtime-dom): Apply nested component styles when using defineCustomElements #4309

wants to merge 1 commit into from

Conversation

TatsuyaYamamoto
Copy link

@TatsuyaYamamoto TatsuyaYamamoto commented Aug 11, 2021

Motivation

defineCustomElements is supporting SFC having style tag, but nested SFC's styles are not applied.
Even if SFC tools import nested SFCs in custom element mode and each SFCs have styles as property,_applyStyles is received root SFC's style only and nested SFCs' styles are ignored.

I think it's commonly that a SFC has some smaller SFCs.

Changes

apiCustomElement.ts

  • add private method _getStylesRecursively
    • When a SFC has nested components, a SFC has components property.
    • When a SFC has styles, a SFC imported by SFC tools of custom element mode has styles property.
      -_getStylesRecursively read component recursively using each components properties and read styles property if a component has.
    • All read styles are pushed to an array and _getStylesRecursively returns the array.
  • Provide all styles including nested component's styles to _applyStyles in connectedCallback (_resolveDef).

customElement.spec.ts

  • add test case (test('should attach styles recursively to shadow dom'))

@yyx990803
Copy link
Member

If the child components are compiled under custom elements mode, it's probably better to register them and use them as custom elements.

That said, this could be a use case where the library author don't want to pre-determine the tags the elements are registered as. Unfortunately this PR wouldn't cover the case if child components are not registered using the components option (e.g. <script setup>)

@LinusBorg
Copy link
Member

LinusBorg commented Aug 11, 2021

If the child components are compiled under custom elements mode, it's probably better to register them and use them as custom elements.

Generally I'd agree, but for tightly coupled components you wouldn't be able to use provide/Inject any longer, which is quite common in these scenarios.

If the child components are compiled under custom elements mode, it's probably better to register them and use them as custom elements.

Indeed. Would it work and be an acceptable workaround to require component authors to register them in a second script block?

<script setup>
import Child from './Child.vue'
</script>
<script>
import Child from './Child.vue'

export default {
  Child
}
</script>

Not exactly pretty but ...

@TatsuyaYamamoto
Copy link
Author

sorry, I accidentally close this PR.......


If it default-exports an object with a component property, this PR can recursively read SFCs with the current _getStylesRecursively . (@LinusBorg, thank you for your idea!)

<script>
import Child from './Child.vue'
export default {
  components: { Child }
}
</script>

@yyx990803 thank you for checking this PR!

it's probably better to register them and use them as custom elements.

I understood that ​component authors have two options to creating web-components with nested vue components.

  1. register child SFCs as custom elements and use them in parent SFCs.
  2. export child SFCs in parent SFCs.

I think "export child SFCs" has smaller impact on a SFC implementation as vue components.
In the case of "register child SFCs as custom elements", an implementation to output web-components affects a SFC implementation as vue components

@TatsuyaYamamoto
Copy link
Author

The document announces usage alongside normal <script> for declaring additional options.
https://v3.vuejs.org/api/sfc-script-setup.html#usage-alongside-normal-script

After reading this, I feel natural to declare components option in script .

@pawel-marciniak
Copy link

Hi, for me exporting child component in second <script> tag results in Syntax Error: TypeError: Cannot read property 'content' of null error :/

@TatsuyaYamamoto
Copy link
Author

@pawel-marciniak I can check the cause of error if there is a reproduction code(repository).

@pawel-marciniak
Copy link

pawel-marciniak commented Aug 31, 2021

Is there any plan to fix this? Because for now I will need to get back to Vue 2.x. Regarding my above error - I'm using TypeScript and I have the same error when I'm using one <script setup> tag but without lang="ts", so it seems to me that using two <script> tags along with TypeScript results in error.

@Justinidlerz
Copy link

Hey, I'm facing the same problem now. Is there any plan to fix this?
I have an idea to add a feature for mini-css-extract-plugin to resolve this.
If we need to inject all CSS (including child components or load by other dependencies) into the root shadow-dom.
Can provide a runtime container for storing all the extract-plugin extracted CSS and import that to inject into the root.
This is inspirated by the runtimeGenerator option of svg-sprite-loader .

@Justinidlerz
Copy link

Justinidlerz commented Sep 14, 2021

Hey, I'm facing the same problem now. Is there any plan to fix this?
I have an idea to add a feature for mini-css-extract-plugin to resolve this.
If we need to inject all CSS (including child components or load by other dependencies) into the root shadow-dom.
Can provide a runtime container for storing all the extract-plugin extracted CSS and import that to inject into the root.
This is inspirated by the runtimeGenerator option of svg-sprite-loader .

I've added the shadow-DOM type for the injectType option of the style-loader
But seems they don't want to add the feature for the plugin. Can see the PR: webpack-contrib/style-loader#536
So I create a new loader to temporarily fix this issue. https://github.com/Justinidlerz/style-shadow-dom-loader#shadowdom

@@ -341,4 +342,28 @@ export class VueElement extends BaseClass {
})
}
}

private _getStylesRecursively(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only addresses styles for locally registered non-async components. Async or global components would still have no styles.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider a web component to be an isolated and self-contained component in the shadow DOM, which is fully loaded and registered as a module.
I would be concerned if code was dynamically reloaded in the shadow DOM, so I don't see the need for async components.

@adamdehaven
Copy link

Also, would this inject styles into the shadow DOM from child components imported within the defineCustomElement component that originate from external packages?

As an example, if I define and export a component with defineCustomElement, but within that element is a button component, imported from an external package, that was not created with defineCustomElement.

@woldtwerk
Copy link

You could do a workaround. It's situational and doesn't cover everything, but it might suit your usecase.
In your components you simple call useStyles(). This uses the fact that the styles are exposed on the style property. You need to name your components *.ce.vue or change the compiler options.

import { ref, onMounted, getCurrentInstance } from 'vue'

export const useStyles = (removeStyleTag: boolean = true) => {
  const shadowRoot = ref<ShadowRoot>()

  onMounted(() => {
    const instance = getCurrentInstance()
    if (!instance) return

    shadowRoot.value = instance.vnode?.el?.getRootNode()
    if (!shadowRoot.value) return

    // @ts-expect-error
    const styles = instance.type.styles.join('')

    // Use __hmrId as UUID for replacing styles, instead of adding new ones every time.
    // @ts-expect-error
    const __hmrId = instance.type.__hmrId

    if (instance.isCE) {
      // Don't remove styletag in dev, because runtime would try to remove non existing dom element on update.
      if (import.meta.env.DEV || !removeStyleTag) return
      // Not the best to let the runtime first add a styletag and than remove it, but we don't want to touch the vue runtime code.
      shadowRoot.value
        .querySelectorAll('style')
        .forEach((style) => style.remove())
    }
    adoptStyles(shadowRoot.value, styles, __hmrId)
  })
}

/**
 * Whether the current browser supports `adoptedStyleSheets`.
 */
export const supportsAdoptingStyleSheets =
  window.ShadowRoot &&
  'adoptedStyleSheets' in Document.prototype &&
  'replaceSync' in CSSStyleSheet.prototype

/**
 * Add constructed Stylesheet or style tag to Shadowroot of VueCE.
 * @param renderRoot The shadowroot of the vueCE..
 * @param styles The styles of the Element.
 * @param __hmrId hmr id of vite used as an UUID.
 */
export const adoptStyles = (
  renderRoot: ShadowRoot,
  styles: string,
  __hmrId: string
) => {
  if (supportsAdoptingStyleSheets) {
    const sheets = renderRoot.adoptedStyleSheets
    const oldSheet = sheets.find((sheet) => sheet.__hmrId === __hmrId)

    // Check if this StyleSheet exists already. Replace content if it does. Otherwise construct a new CSSStyleSheet.
    if (oldSheet) {
      oldSheet.replaceSync(styles)
    } else {
      const styleSheet: CSSStyleSheet = new CSSStyleSheet()
      styleSheet.__hmrId = __hmrId
      styleSheet.replaceSync(styles)
      renderRoot.adoptedStyleSheets = [
        ...renderRoot.adoptedStyleSheets,
        styleSheet,
      ]
    }
  } else {
    const existingStyleElements = renderRoot.querySelectorAll('style')
    const oldStyleElement = Array.from(existingStyleElements).find(
      (sheet) => sheet.title === __hmrId
    )

    // Check if this Style Element exists already. Replace content if it does. Otherwise construct a new HTMLStyleElement.
    if (oldStyleElement) {
      oldStyleElement.innerHTML = styles
    } else {
      const styleElement = document.createElement('style')
      styleElement.title = __hmrId
      styleElement.innerHTML = styles
      renderRoot.appendChild(styleElement)
    }
  }
}
<template>
  <div class="abc">ABC</div>
</template>

<script setup lang="ts">
import { useStyles } from '../composables/useStyles'

useStyles()

</script>

<style>
.abc {
  background: red;
}
</style>

<style>
.abc {
  color: purple;
}
</style>
<template>
  <Abc />
</template>

<script setup lang="ts">
import Abc from './Abc.ce.vue'

</script>

@muelbr
Copy link

muelbr commented Apr 1, 2022

If the child components are compiled under custom elements mode, it's probably better to register them and use them as custom elements.

Using children this way means then you have to deal with DOM style props (slightly different syntax, arrays as string trap) and grabbing values from an ugly attribute array inside of custom events. This is just as unsatisfying as defining all styles in the root element. I may be wrong, but the developer experience for custom elements is currently not nearly as good as for regular Vue applications (although still a great project).

@shareefhadid
Copy link

Is there any update on this? Not including the styles of child components makes it impossible to use component-based UI libraries in a custom element. Is that a bad practice to begin with, am I missing something?

@larserikfinholt
Copy link

Are there a workaround for this? Some indication of where this is going would be nice. A simple "not in the near future", "or not possible in v3" is fine. A long running open issue like this is a bit difficult when taking decitions.

@alexlyul
Copy link

alexlyul commented Jun 6, 2023

Hello! Any news on this PR?

@simvol
Copy link

simvol commented Jul 19, 2023

Bump

@woldtwerk
Copy link

I think the whole custom element api should get a look at. There are a lot of painpoints.
E.g.:

  • nested styles
  • access to the constructor
  • access to the host element
  • access to the shadowroot
  • prefer constructed stylesheets over style tags
  • allow attachElementInternals
  • better support for defineExpose/defineEmits

@TatsuyaYamamoto TatsuyaYamamoto closed this by deleting the head repository Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.