search

riot / compiler

Riot.js compiler
MIT License
64 stars 29 forks source link

Allow (optional) choice of scoped selectors #168

Closed exside closed 1 year ago

exside commented 1 year ago

Sorry for the unprofessional PR, just edited on github directly and didn't test it, but I think it should work.

The idea is that we could give the scopedCSS property of the options object for riot.compile() a value other than true or false (those stay default and are ok), e.g. a string either 'tag' or 'is' which will then in turn only scope the CSS selectors by the chosen method.

This allows us to reduce CSS size by cutting down on the bloat the CSS scoping does by adding both the tag and the [is="<tag>"] prefixes to each selector within a components CSS. I believe (and certainly do personally) people probably use the is approach or the tag approach, but not mix both and if they do, the defaults are still fine adding both prefixes to selectors. For all others who use one approach consistently (I'm in the is camp) this change could give some nice size savings in the generated CSS.

Maybe you see a better way of doing this as well?

Thanks for your great work on riot and for looking into this!

GianlucaGuarini commented 1 year ago

Thank you for your PR. I guess that here a simple boolean scopedCSS: false should be enough. Please make sure to write a test for it as well. This seems to be the right place add a new test for it.

exside commented 1 year ago

Well, if scopedCSS: false then the CSS is not scoped at all, which is also not what I want (but currently use for one specific component that provides the "theme" for the entire app), I'd just like to reduce bundle size by not having those duplicate scope prefixes if my codebase is consistently using [is=""], but I was also thinking that maybe instead of some hardcoded fixed values, why not simply check if scopedCSS is a string and not a boolean and then have that string be the "scope" selector? what do you think?

So for example one could specify { scopedCSS: '[is]' } and then all the CSS from compiled components is simply prefixed by [is] .yourclass {}. An even more modular (but probably harder to implement) idea would be that a component object could itself set the scopedCSS property, and then scoping can be configured on a component level. If not set, it would simply inherit the global value for scopedCSS.

But maybe all of this is simply because I'm using riot the wrong way, have just recently stumbled upon the <theme-provider> example (I think it was around <slot />) and that is actually exactly what I was trying to do when this "requirement" came up, I want theme styles to be available to all components, so i needed to have them unscoped, but I didn't want the other components to be unscoped. But using a theme-provider component to wrap the others would proably solve the issue, but seems a bit unnecessary as well if you have riot components in multiple places on a page (just like widgets because not the entire site is riot, it's an integration) and then having to wrap every component with the theme-provider...

GianlucaGuarini commented 1 year ago

Probably we need an option like generateScopedCss. I would prefer to provide a function for such advanced use cases. This function might receive as first argument the matched string and as second the tag name. Please update the PR accordingly adding a unit test for this feature. Thanks

GianlucaGuarini commented 1 year ago

closing this PR because it's stale. @exside thank you for it and feel free to reopen it when you will have time for an update.