lit / lit-element

LEGACY REPO. This repository is for maintenance of the legacy LitElement library. The LitElement base class is now part of the Lit library, which is developed in the lit monorepo.
https://lit-element.polymer-project.org
BSD 3-Clause "New" or "Revised" License
4.49k stars 319 forks source link

CSS minification not possible #696

Closed LarsDenBakker closed 2 years ago

LarsDenBakker commented 5 years ago

lit-html HTML inside template literals are easy to minify because lit-html requires the HTML to be parseable as valid HTML. For example using https://github.com/cfware/babel-plugin-template-html-minifier

With lit-element's CSS templates this is unfortunately not the case because we can concatenate partial (invalid) css to eventually become valid css. I love this feature a lot, but unfortunately now it's no longer possible to use tools for minifying CSS because there are very few assumptions that can be made about the code.

For example given this piece of CSS:

import { css } from 'lit-element';

const XL = 24;
const fontSizeXL = css`
  ${24}px
`;
const myStyles = css`
  .foo {
    font-size: ${fontSizeXL};
  }
`;

All a minifier would get is px and font-size: ; which is invalid CSS. At best the tool will throw an error, at worst it will silently mangle the CSS leading to unexpected results.

I'm wondering if this is something that has already been considered, and if there are existing tools/solutions which handle css minification.

Otherwise is there something we can think of to differentiate partial and non-partial css so that tools can at least minify the non-partial css. For example perhaps introducing a separate css template tag?

MarkiyanPyts commented 5 years ago

Possible with this https://www.npmjs.com/package/lit-scss-loader

LarsDenBakker commented 5 years ago

Thanks for your response. In a CSS file you cant write JS expressions and create invalid CSS. So im not sure how this solves my problem.

MarkiyanPyts commented 5 years ago

Webpack will minify your css and insert it in minified state into your web component, you can write normal sass in external file. Why would you want to write js expressions in css?

If you want font size to be controlled via web component attribute you can insert it inline on one of the component tags. In lit html template.

Else than that you can use css variables to control styles from outside the component

LarsDenBakker commented 5 years ago

I dont want want to build and bundle during development, so something that relies on webpack is not an option.

Js expressions can be used to share constants without needing special tools.

My question is really what strageties do we have for minifying with the existing syntax and options.

MarkiyanPyts commented 5 years ago

To that I don’t have answer maybe someone from core team can tell us that.

danielbarion commented 5 years ago

747

Guys, maybe this PR help this question.

CaptainCodeman commented 5 years ago

The rollup-plugin-minify-html-literals plugin works great for me - minifies both html and css. I just skip it during development:

import minifyHTML from 'rollup-plugin-minify-html-literals';
import { terser } from 'rollup-plugin-terser';
...
const production = !process.env.ROLLUP_WATCH;
...
plugins: [
    ...
    production && minifyHTML(),
    production && terser()
  ],
LarsDenBakker commented 5 years ago

@danielbarion not sure how documentation fixes the issue?

@CaptainCodeman does that handle partial/invalid css correctly like explained above?

danielbarion commented 5 years ago

@LarsDenBakker if you configure your "compilation" proccess, you can transpile SASS, Stylus, etc... To css before import, so, when your minify runs, it will be only a normal css...

I'm using stylus in my test project (a lib of web componentes written with LitElement), in the main app, when I import some component, I'm just importing js and css and the minify can do they work as expected.

CaptainCodeman commented 5 years ago

does that handle partial/invalid css correctly like explained above?

I don't know, all my CSS is static - IMO the correct way to update a font size (for example) is to make it a CSS variable and set that variable from code, not change the CSS that is output. Any combining of CSS is done by just pulling in multiple CSS blocks into the static styles getter. e.g. common to all elements (display block, [hidden] attribute support) then another block for card / shadow styles etc...

If the variable part comes at runtime, nothing will be able to process it as proper CSS without having a whole JS runtime as part of the process which is unrealistic and going to be slow.

LarsDenBakker commented 5 years ago

@danielbarion thanks, but if you check the issue description I'm not using SASS. I use regular css as provided by lit-element.

@CaptainCodeman thanks, but this whole issue is about using partial css which is an intended use case of lit-element. I'm not doing any dynamic css changes, so I don't need css variables for this.

I'm using it for constants for colors, font size etc. all using static css tags and sharing these across stylesheets. This works perfectly fine when unbuilt, it's probably actually faster than css variables as it's static.

You could argue about whether this is a good usage of the css stylesheets, but the point is that it's possible. So anyone which has dependencies which use lit-element, and are using a css minifier, are opening themselves up for potential bugs because anyone could be doing this type of thing.

I'm looking if we can establish conventions or tooling to make this work.

justinfagnani commented 5 years ago

I suspect to enable proper linting and minification of CSS with expressions that we might need a layer that injects placeholders similar to lit-html. The difficulty with CSS is that the syntax is more complex than HTML and picking a valid placeholder will depend on the context. We might have to limit where expressions can occur (again similar to lit-html).

danielbarion commented 5 years ago

@LarsDenBakker I understand, but if you check my PR, you will found this section.

Maybe you want to do something like this ? (minify css and html/js in separated files...) and you could use css vars... But if you want to use JS vars, I'm sorry, the PR will not help with anything...

sorvell commented 2 years ago

Closing due to age. Please make a new issue if necessary.