postcss-lit icon indicating copy to clipboard operation
postcss-lit copied to clipboard

Support more variants of css inside lit components

Open iSokrat opened this issue 1 year ago • 6 comments

Now, the plugin works with css places inside css tag. However, css may often be stored near html

Example 1 (html tag):

import {styles} from 'somewhere';

//...

html`
  ${styles}
  <style>
     .abc {
       color: green;
     }
  </style>
  <h1> ... </h1>
  ....
`

Example 2 (svg tag):

import {styles} from 'somewhere';

//...

svg`
  ${styles}
  <style>
     // ....
  </style>
  <svg> ... </svg>
  ....
`

There are also some minor cases when some styles are inlined:

html`
  <h1 style="color: green"> ... </h1>
`

The proposal is to at least add support of first 2 major cases (Example 1, 2), however, would be nice to support all possible variants.

iSokrat avatar Mar 31 '23 18:03 iSokrat

@43081j Hello, what do you think about this?

iSokrat avatar Apr 05 '23 10:04 iSokrat

It's a difficult one to solve as it means we need to start parsing html/SVG templates in lit components just in case they contain CSS.

These days I would hope everyone uses stylesheets instead.

Do you have an example of why someone would want inline CSS instead of writing it in a style template?

It's a fair chunk of work so if we can instead just tell people to use CSS templates, it'll save a lot of effort

43081j avatar Apr 08 '23 11:04 43081j

So, I'll try to describe my case:

My project uses hundreds of lit components, some of them were created long time ago, some of them - not.

Do you have an example of why someone would want inline CSS instead of writing it in a style template? Usually, inline CSS means this:

html`
  <h1 style="color: green"> ... </h1>
`

I fully agree that it's not a good practice. Above I mentioned this case as minor one, it's not really critical - on my project only couple of places have this in some legacy code. In case if it's possible to support such cases without big efforts (we'll try to parse style attribute), then that's cool, if not then it's not a big problem.

About html and svg

So, svg is also in my case mostly an exception since I have only one component fully based on some graphical data. But html is not. This is widely used practice on my project since it has own benefits:

  1. Often there are not a lot of styles for some components and based on the practice it's easy to store them near html-like code to quickly see the total picture (our render functions are stored usually in the separate files, not in the element's body). https://lit.dev/docs/components/styles/#style-element
  2. From performance perspective I personally didn't see any suspicious things (we don't use expressions inside styles to not have some performance issues).
  3. This approach supports the same scoping of styles inside Shadow DOM.

So, even in case if it would be needed in the future to re-write everything to css, for my project it will not be so simple task.

iSokrat avatar Apr 10 '23 09:04 iSokrat

@43081j Hello, what do you think about described above?

iSokrat avatar Apr 12 '23 16:04 iSokrat

it would introduce a fair amount of overhead in the syntax parsing to support the <style> tags.

ideally you would migrate to style templates, for the various benefits of adopted stylesheets (e.g. the fact that an adopted stylesheet can be shared across many instances of an element, while style tags will be duplicated).

though i understand such a migration takes a lot of time, if you even have the chance to do it (or want to do it).

we could possibly put <style> tag extraction behind a flag, but it still means this module needs to then pull in parse5 and implement some basic html traversal.

its difficult, as i know it would help you and some others a lot but its an expensive piece of work (time consuming). we'd have to implement html template extraction, using parse5 to parse the templates and meanwhile replacing interpolations with placeholder html. basically what eslint-plugin-lit does under the hood.

its a lot of logic to introduce for a user case people should be moving away from

43081j avatar Apr 16 '23 21:04 43081j

though i understand such a migration takes a lot of time, if you even have the chance to do it (or want to do it).

Yep, this may take some time to do this. We already have own eslint plugin which checks some basic project-related things in <style> created long time ago, so, this was the first reason why I want to use tome other tool used more often for such things.

iSokrat avatar Apr 17 '23 09:04 iSokrat