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

Add support for using lazyStyleTag use() from a JavaScript module (injecting into multiple shadow DOMs) #565

Open
mikeaustin opened this issue May 20, 2022 · 21 comments

Comments

@mikeaustin
Copy link

Feature Proposal

When using lazyStyleTag and use({ target: shadowRoot }) inside a shared JavaScript module, only the first call to use injects a style element. The other shadow elements that call use() have no styles. I've duplicated the module and loaded each and the problem does not exist. There may be other options such somehow forcing separate module instances, but I'm not sure.

In index.js, I believe the following condition prevents use() from injecting duplicate styles:

if (!(refs++)) {
    update = API(content, options);
}

https://github.com/webpack-contrib/style-loader/blob/master/src/index.js#L131

One possible solution is to give an id to each injected style element, instead of having a global refs value.

Feature Use Case

loading JS modules injected into shadow elements, so modules can be loaded dynamically.

Please paste the results of npx webpack-cli info here, and mention other relevant information

System:
OS: macOS 12.1
CPU: (4) x64 Intel(R) Core(TM) i5-8210Y CPU @ 1.60GHz
Memory: 122.11 MB / 16.00 GB
Binaries:
Node: 14.18.1 - /usr/local/bin/node
npm: 6.14.15 - /usr/local/bin/npm
Browsers:
Chrome: 101.0.4951.64
Chrome Canary: 104.0.5071.0
Firefox: 98.0.1
Safari: 15.2
Safari Technology Preview: 15.4
Packages:
css-loader: ^6.7.1 => 6.7.1
css-modules-typescript-loader: ^4.0.1 => 4.0.1
styles-loader: ^4.0.1 => 4.0.1
ts-loader: ^9.3.0 => 9.3.0
webpack-cli: ^4.9.2 => 4.9.2

@alexander-akait
Copy link
Member

It it possible to create memory leaking and it will have very bad perf...

@mikeaustin
Copy link
Author

HI @alexander-akait, can you elaborate on bad performance? I'm open to other suggestions. What about passing a key in options that can be used to associate the instance?

@alexander-akait
Copy link
Member

@mikeaustin Getting DOM nodes using querySelectorAll/etc is not good for perf (we still support old browser and there is a problem too), because we need to traverstly all DOM nodes, also you can manually delete some DOM from HTML document, especial in shadow DOM, and due to this we can have leaking DOM nodes.

As I undestand you want to reuse some styles in multiple shadow doms, right? If you provide a problem which you try to solve we can try to find better solution

@mikeaustin
Copy link
Author

Hi @alexander-akait, thank you for the reply. The original message covers the issue – loading a JS module dynamically, multiple times, only the first call to use() injects the style element. Since JS module instances are shared (loading a second time returns the original instance), only one style element is ever injected.
At the root of the issue, it’s because import(), static or dynamic, shares instances, and that instance uses the use() function. I don’t know if there’s a way to force a module to return a new instance. import() is used to load a React component dynamically. A function “mount” is exported in each module which passes the shadow root. The component then calls use() with that shadow root as target.

Thank you. Would another option to be to create a new instance manually? So instead of styles.use(), styles.create().

@alexander-akait
Copy link
Member

@mikeaustin Can you create small example? I want to look how you use it to provide better solution

@andriichumak
Copy link

I'm having the same issue. It can be reproduced easily with the example from the docs: https://webpack.js.org/loaders/style-loader/#custom-elements-shadow-dom Just import the resulting js to some html file and create several instances of the custom element, like so:

<custom-square></custom-square>
<custom-square></custom-square>

You'll notice that only the first element is styled. Would appreciate any advice on how to work around this properly. So far I'm thinking of some sort of MiniCssExtractPlugin + AssetsPlugin combination, but don't really like that option.

Ideally, there should be a lazyLinkTag injectType + an option to insert several links per runtime (one per Shadow Root).

@alexander-akait
Copy link
Member

@andriichumak Do you keep styles in JS file or extract them for production?

@andriichumak
Copy link

andriichumak commented Jul 26, 2022

I would prefer to have styles extracted, but it's not a hard requirement for my project. If we would be able to inject style tag into multiple places with lazyStyleTag - that would already be a big improvement.

Another option I can think of is to make the loader return the actual path to the generated css file instead of an empty object when using regular linkTag. This way I would be able to inject the link tags manually.

import pathToFile from 'my.css';

// pathToFile = e.g. /assets/my.css

@alexander-akait
Copy link
Member

@andriichumak Can you create small reproducible example to show usage?

@mditullio
Copy link

mditullio commented Oct 18, 2022

Had exactly the same issue today. It's a pity that in webpack there's no easy way to export a CSS module URL(s) to extracted files.

I'm considering reverting back and use just css-loader, exporting styles as CSSStyleSheet items.
And then, use the adoptedStyleSheets options, in order to style pages and components.

But the issue with these interfaces, is that they're not supported from Safari (and polyfill does not work great)

@alexander-akait
Copy link
Member

@mikeaustin Can you prodvide example of the problem (small repo), so I provide configuration solutions how I will solve the same problems

@mditullio
Copy link

mditullio commented Oct 18, 2022

Hello @alexander-akait

I did a small example of the issue: https://github.com/mditullio/demo-webpack-style-loader.

The need behind would be to share the same CSS from multiple web component instances that use shadow DOM.

Ideally, this could be, as suggested by @andriichumak a module which exports the URL / path to the extracted CSS.
I think it as well, would be great to have a new inject type called lazyLinkTag which could do something like that.

Thanks in advance for the attention

@alexander-akait
Copy link
Member

@mikeaustin Tha main problem style-loader is not extract CSS, it is for CSS in runtime code, anyway if you can use mini-css-extract-plugin and dynamic import(...) (alternative solution - set runtime: false and implement own logic for loading), but yes, CSSStyleSheet is right solution (that is bad it has a bad support right now, it was design for Shadow DOM). You have written polyfill is not wroking, can you desribe what is broken? Maybe you can create own small polyfill if you don't need extra complex logic

@mditullio
Copy link

Thanks for the suggestion about dynamic import, I will look onto it.

For the polyfill of adoptedStyleSheets, I tried this lib: https://github.com/calebdwilliams/construct-style-sheets
But there are some issues regarding Safari: calebdwilliams/construct-style-sheets#110
But in general, to polyfill it, no other way than replicating style tags in each component instance.

So, for now, I decided to use Shadow DOM for intranet-only needs (just supporting Edge / Chrome), waiting for Apple updating its browsers... Safari is the new IE11 🐢

@alexander-akait
Copy link
Member

Feel free to feedback and ask questions

@aw1875
Copy link

aw1875 commented Jan 22, 2023

Just came across this issue as I'm having an issue with the same thing currently (I posted it on StackOverflow also). Reading through this I'm a little confused if someone found an appropriate workaround to this issue as I've been stuck on this for a few days now and I feel like I've tried a few different approaches.

Edit
Wanted to make an edit saying I did technically find a way to do it, but it doesn't seem like the most productive way so would love to hear more from someone else. What I got working was loading the styles with an id like my-styles and then grabbing those styles on my connectedCallback:

// webpack.config.js:
{ loader: "style-loader", options: { attributes: { id: "my-styles" } } }

// component.tsx
connectedCallback() {
    // Inject Styles
    const css = new CSSStyleSheet();
    css.replace(document.querySelector("#my-styles").innerHTML);
    this.shadowRoot.adoptedStyleSheets = [css];

    // Inject Component into ShadowRoot
    createRoot(this.shadowRoot).render(this.render());
}

Also, going this approach creates complications for my extension popup as I'm unable to access the styles there as they are injected into the page instead.

@mditullio
Copy link

Hello @aw1875

What you could do, if you can use CSSStyleSheet class (remark it's not available on every browser, Safari does not support it) is to directly export your stylesheet files in the form of CSSStyleSheet using the css-loader - look at https://webpack.js.org/loaders/css-loader/#exporttype.

The problem in my case, is that I needed to support also Safari browsers, so I ended up just by not using ShadowRoot, injecting them global styles using SASS with a root selector name-of-my-component { ... }.

@aw1875
Copy link

aw1875 commented Jan 22, 2023

@mditullio appreciate the response! I looked through the docs and gave it a try based on them. It didn't seem to work, giving me the error "TypeError: Failed to set the 'adoptedStyleSheets' property on 'ShadowRoot': Failed to convert value to 'CSSStyleSheet'." I'm not sure if this is because I'm using tailwind and postcss but the documentation seemed pretty straight forward. Also, I should mention that this is specifically for a chrome extension so I'm not super concerned with browser support.

Quick edit:
After spending some time debugging, the adopted styles are being passed to the shadow root but for whatever reason, they aren't actually applied. I'm assuming this may have something to do with the style-loader not working properly in conjunction with this.

@mditullio
Copy link

Yes, style-loader should be removed if you use injectType = css-style-sheet.

@aw1875
Copy link

aw1875 commented Jan 22, 2023

Just responded on StackOverflow but thank you it works perfectly now!

@mditullio
Copy link

Perfect ! You're welcome ;)

GeorgianStan added a commit to GeorgianStan/vanilla-context-menu that referenced this issue Jun 10, 2023
Remove 'style-loader' because of the issued mentioned here,
webpack-contrib/style-loader#565, and only use
'css-loader'.
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

5 participants