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

FEATURE: CSS build refactor (make react-ui-components easy to use) #3366

Merged
merged 50 commits into from
Feb 22, 2023

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Feb 14, 2023

solves #3213
solves #3330

Puhh im happy to finally present you this pr.
I have been working on this concept since we started the esbuild pr but now everything just fell in place and ive got a fully working* improved css build and react-ui-components build.

*I currently cant find any style shifts in a side to side comparison :D

I took my previous refactoring at #3358 even further, now with a better written cssModules plugin https://github.com/neos/neos-ui/blob/aa9f5497678225b8b9414b247f97042aaa165447/cssModules.js and an actual consumer (dev) friendly build of the @neos-project/react-ui-components

What I did

  • reduce the build speed by a third!!! esbuild + postcss + sass = 1500ms -> now esbuild + lightningcss = 500ms!!!
  • replace sass @extends with css-modules composes https://github.com/css-modules/css-modules#composition
    • less code duplication in the css bundle 82.8kb vs 116.1kb :D
  • replace post-css-modules with lightningcss (in Rust by parcel) as it offers an api that makes composes from "./file.css" usable as esbuild plugin: see: https://github.com/neos/neos-ui/blob/aa9f5497678225b8b9414b247f97042aaa165447/cssModules.js
    • the post-css-modules plugin offers no fully working way to build an integration with things like esbuild. The api is so cruel and unusable. In the webpack build it only worked by using a huge pile of shit - so im really happy that lightningcss offers a so simple api.
  • Our.reset css modules compose mixin now uses the :where() selector, to force a low specificity at all time (just in case somehow the reset class lands at the bottom at the stylesheet, where it would have a higher specificity otherwise, which was our problem in the first place so we used sass. But after investigating if found out that since using my esbuild plugin the css class is as wanted at the top so it would be fine either way for now) edit i reverted to not using :where as we dont need it atm
  • figure out to create a prepack build script for the react-ui-components
    • source maps
    • full build css (applied cssmodules, nesting and cssvariables)
    • typescript declarations
    • able to import only certain components
    • api surface should not change
  • rewrote the css at some places to make it work with lightningcss
  • make the build-essentials obsolete (the css variables reside now in top level as css file)
    • infact the package is now only used by the webpack extensibility package
    • moved css var declaration from packages/build-essentials/src/styles/styleConstants.js to cssVariables.css
      • the file cssVariables.js creates a plugin for lightningcss which embeddes the css variables as usual
  • renamed packages/react-ui-components/src/index.js to a typescript file so we have types when using import {Button} from "index.js"

Todos

How I did it

blood. sweat. tears.

How to verify it

use the lib-esm build in the ui ^^

react-ui-components build

see if it works correctly, as you have used before (by linking the package locally or some other hack)

or create a js and html file like:

import React from "react"; 
import ReactDom from "react-dom";
import {Headline, Button, Label, IconButton} from "./lib-esm/index"

ReactDom.render(
    <div style={{background: "#2b2b2b"}}>
        <Headline type="h3">Hello</Headline>
        <Button>Click</Button>
        <Label>Hi</Label>
        <IconButton icon="neos">Hello</IconButton>
    </div>,
    document.getElementById("app"),
)

build it with yarn esbuild test.jsx --outdir=dist --bundle and view the html in the browser.

Upgrade instructions for Neos Ui components @neos-project/react-ui-components:

With this change the CSS build stack of the Neos Ui was modernized, due to this it became possible to make the package react-ui-components easier to use: Developers requiring the npm package will no longer need to worry about the required CSS preprocessing from the sources (lowering nesting and applying css-modules). A prepack script will build the css to native css and also take care of css modules. This means that for consumption of the components you will only need a simple bundler to load the vanilla css - nothing else. It is advised to read the new readme before updating so you can identify any other changes.

When a nested rule starts with an element selector, it may be ambiguous with a property. In these cases, the spec requires the selector be prefixed with a nesting selector (&).
https://drafts.csswg.org/css-nesting/#nest-selector
my esbuild plugin @mhsdesign/esbuild-composes-from-css-modules works better than the previously used esbuild-style-plugin
- source maps
- full build css (applied cssmodules, nesting, cssvariables)
- typescript declarations
- able to import only certain components
- api surface should not change
@mhsdesign mhsdesign marked this pull request as ready for review February 15, 2023 08:37
@mhsdesign
Copy link
Member Author

Is now completely ready for review ;)

See https://github.com/neos/neos-ui/blob/d8ab716a5753a00bec648cfd480bd0aa3e347abc/packages/react-ui-components/README.md for a new documentation of the react-ui-components ;)

… file separately

the advantage is we dont have so many empty chunks and less files to publish.
Also previously with importing from the unstyled compontents would still include css as the chunking wasnt optimal.
mhsdesign and others added 2 commits February 22, 2023 13:06
TASK: Analyze bundle size and health `node esbuild.js --production --analyze`
Copy link
Member

@markusguenther markusguenther left a comment

Choose a reason for hiding this comment

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

Looks good to me, if we have bugs left, we need to fix them anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.3 Feature Label to mark the change as feature
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

2 participants