Modernize the clay-css package build system

[matuzalemsteles]

Well, historically the build system of the clay-css package is quite old, probably built on the first version of Lexicon CSS, using gulp and many other dependencies to compile SCSS, build SVGs, and test site.

Due to not having many changes over time, the dependencies ended up being deprecated some of them, as well as many old ones (#3520) which sometimes prevents us from building the clay-package using node.js 12 (#2053) and we are still using node 10, ideally, we want to use the current LTS version which is Node 14 and to avoid build issues for the package as a whole and simplify the maintenance of this package.

The goal is to build scripts to compile SASS (with dart-sass), build the SVG... in Node.js vanilla without using dependencies like lodash, gulp, and other plugins only if necessary, one of the goals is also to decrease the number of dependencies and make it simple.

An important thing is also to maintain the test site, @pat270 uses this very often to do smoke tests, instead of translating this to a simpler and more modern version in Node.js, let's create a proof of concept based on Gatsby, using the very simple version and without many plugins to allow the build to be fast, unlike clayui.com, this is just for smoke tests we keep and we will just translate to a very simplified version in Gatsby. clayui.com build is very slow because we have many plugins to compile mdx, md, so it gets slow.

[julien]

Hey @matuzalemsteles, I totally agree that we should use dart-sass and make it possible to build clay with the latest version of node LTS.

On the other hand, I'm not really in favor of having yet another site (the test site): ideally I'd like having one site with the docs and the "playground" ... I suggest working on making clayui.com to make it faster: if Gatsby is too slow maybe we could look for of an alternative and talk about it together.

[matuzalemsteles]

On the other hand, I'm not really in favor of having yet another site (the test site): ideally I'd like having one site with the docs and the "playground" ... I suggest working on making clayui.com to make it faster: if Gatsby is too slow maybe we could look for of an alternative and talk about it together.

Yes, we've tried that in the past to put together everything that was done on @pat270's smoke test site and put it on clayui.com, I remember we started to send most of the documents but not all because in fact a lot of things are just tests that @pat270 uses, for the purpose of @pat270 the current site he maintain is fast because it generates documents from HTML, so this is very fast, unlike Gatsby or another static generator that handles plugins, files like markdown or mdx that mixes markdown with React, that for clayui.com works fine but for @pat270 might it looked bad.

The idea of creating a website is actually just bringing what already exists into a simple structure, for example, https://github.com/gatsbyjs/gatsby-starter-default (This is to be used locally and not for publishing purposes), starting from this simple example with zero configuration just to get up these contents and it will be much faster. I remember one of the things also is that smoke tests shouldn't go into production, so that was one of the problems, I even made some suggestions to prevent the content from being published in production, we had an initial conversation about that in #1111, #1212. We can raise this question again, @pat270 could you describe a little more?

[pat270]

Everything @matuzalemsteles said is valid. I can work with the current clayui.com. The main reason why I'm still working with the test site is that it's easy to toss up on github pages and keep the CSS documentation versioned. clayui.com is always the latest.

I'm good with one site, probably easier if I was forced to use it.

[julien]

Thanks for the feedback guys

[julien]

Hey @pat270 and @matuzalemsteles just out of curiosity, I did a test replacing node-sass with sass, basically replacing this line as a first step.

The good news is that it works, but a lot of warnings are printed. I first thought I'd leave them here, but I think that's going to add a lot of "junk" to this conversation, and those warnings are mostly repeated, they concern "division", i.e. DEPRECATION WARNING: Using / for division is deprecated and will be removed in Dart Sass 2.0.0.

Maybe we could do this in several steps, for example
1st step: Replace node-sass with sass
2nd step: Get rid of those warnings

Let me know what you think when you get a chance. Thanks!

[pat270]

@julien good plan. We will need to add a fallback for older versions of Sass that don't have the div() function.

[julien]

As a first step, I think we could replace node-sass with sass in the clay-css package, but that still doesn't mean we can start using node latest (v16.9.1 at this moment), because we also depend on gulp and lots of related plugins, and it seems that there are some incompatibilities. However it can work with node lts (v14.17.6 as of writing this), I've pushed a branch (to upstream so that everyone can collaborate on this) here which only replaces node-sass in the clay-css package.

The goal is to build scripts to compile SASS (with dart-sass), build the SVG... in Node.js vanilla without using dependencies like lodash, gulp, and other plugins only if necessary, one of the goals is also to decrease the number of dependencies and make it simple.

Compiling sass with dart-sass doesn't seem to be the only issue, but the other tasks would need more work and I think that's what we'd need to analyze and prioritize. Because there are lots of other issues. For example,
I decided to go a bit further and tried to replace node-sass in all packages where it's listed as a dependency, and that didn't work, because gatsby-plugin-sass@2.3.12 (used by clayui.com) depends on node-sass too.
This also means that at some point we'll need to update clayui.com and clay-charts which also use node-sass.

An important thing is also to maintain the test site, @pat270 uses this very often to do smoke tests, instead of translating this to a simpler and more modern version in Node.js, let's create a proof of concept based on Gatsby, using the very simple version and without many plugins to allow the build to be fast, unlike clayui.com, this is just for smoke tests we keep and we will just translate to a very simplified version in Gatsby. clayui.com build is very slow because we have many plugins to compile mdx, md, so it gets slow.

I don't have a strong opinion about Gatsby, but are we sure we need to use that for the test site? Every project that I've worked on using Gatsby is not "simple", and I understood that simplicity is what we are aiming for.

In any case, if we can do some progress on this (like building the svg without gulp) let me know.

[pat270]

@julien I'm not familiar with the release process anymore. If we solve building the SVG (might already be solved), we can scrap the test site from Clay. I will probably keep a copy locally so I can easily toss up demos. The other thing I saw as @bryceosterhaus was creating an icons module? Sounds like we won't need to handle SVG's in Clay.

[bryceosterhaus]

@pat270 we will likely need an icons build process on the clay side but only for the case of a dev environment and testing. Hopefully with the new icons stuff I am working on, we no longer will use the build icons.svg file that clay produces.

[julien]

@pat270, @bryceosterhaus that's good news ... I started looking at the gulp tasks yesterday and thought we might still need all of it. Concerning building the SVG, I saw two different things:

1. Generating the "icons.svg" file which contains all icons
2. Generating css classes that correspond to the svg: what I mean is generating this

For the first task, I found the merge-svg-files package which allows concatenating multiple SVGs into one, I'm not exactly sure the output is the same and I'm not saying we have to use it, but it might be worth a look (it generates a file call "sprites.svg" but that might be configurable or we could do something to rename the file once created).

In any case, even if that package doesn't work for us (or if we simply don't want to use it), I think creating a (node) script to handle those two tasks is the next thing have a look at, and it seems like there's less "work" than what I had initially thought so that's good news.

[matuzalemsteles]

@julien answering some of the things you brought up, arriving a little late here, forgive me 😅

Compiling sass with dart-sass doesn't seem to be the only issue, but the other tasks would need more work and I think that's what we'd need to analyze and prioritize. Because there are lots of other issues. For example [...]

Yeah, for sure there are still a lot of problems that we'll have to tackle one at a time, not that we need to change everything now but we can move slowly until we complete the objective here.

I even commented to Patrick in some PR that we also need to update Gatsby and its dependencies. For clay-charts we still have to see how we're going with this package, we always come back to this issue with it if we really support it or just recommend using a library, I had a PoC to support it better but without much progress #3745 about the decisions.

I don't have a strong opinion about Gatsby, but are we sure we need to use that for the test site? Every project that I've worked on using Gatsby is not "simple", and I understood that simplicity is what we are aiming for.

Yeah, I see we can have Gatsby just to serve markdown content and the simple site without a lot of plugins or extra configs that like we have on clayui.com, but if that increases we can explore other possibilities too, I say Gastby because it will deliver a good experience for development.

For the first task, I found the merge-svg-files package which allows concatenating multiple SVGs into one, I'm not exactly sure the output is the same and I'm not saying we have to use it, but it might be worth a look (it generates a file call "sprites.svg" but that might be configurable or we could do something to rename the file once created). [...]

I think that even if we don't use it because it's not very well maintained, we can check how it was done and we can see how to replicate it for our case since we follow a fixed behavior. I'm going to try to explore that on some day and see if I can add something here.

[bryceosterhaus]

@julien for the first icons task, I think a simple script of reading the file, stripping the tag and rendering in a should be simple enough. I can send over a POC if needed. For the second task... I am less familiar with that scss sheet and didn't really know it existed, but I suspect we can do something similar within that script. I'd probably stay away from using an external package if possible.

[julien]

@matuzalemsteles no problem about being late, and thanks for your feedback, I also think this is something that can be done one step at a time. Concerning the test site, we can discuss about it because @pat270 seemed to say that it wasn't a top priority (I do see it being useful for testing though).

@bryceosterhaus seems like we all agree, I just thought I'd mention that package because it exists but I also think having two simple scripts (or one script that handles both "tasks" depending on the arguments it receives) would be enough (and more simple to maintain or extend in the future), and as usual your help is more than welcome.

[julien]

For the first task, I quickly made this, which I'm leaving temporarily in a private gist, before pushing another commit (I'll probably end up using the "promise" versions of the fs module)

[julien]

Hey again ... The gist I shared changed a lot (even though I didn't update it) but at this point, it's a regular (node) script that accept three arguments, and either "builds" the svg, compiles sass files or generates the icon CSS classes.

I also wanted to discuss about more details, and I might be stating the obvious, but gulp does a lot under the hood and for some tasks, using node's builtin modules isn't going to be enough, and since we're going to do a "refactor", I'd like to talk about some details.

• Do we want to use external dependencies to handle IO operations like creating nested directories?
  I don't really have a strong opinion on that, but given where we got with gulp I'd really like to avoid the
  same result: being "stuck" with a dependency isn't fun. The alternative would be to write those functions
  ourselves, but it's obviously going to take a bit more time, it's just something to take into account and that
  I thought I'd share.

• Do we want to use both require('fs') and require('fs').promises modules?
  I personally would like to use the promise only version, unfortunately it doesn't have everything that
  is exposed in the "regular" fs module, (and I really don't get the point).
  I also feel like "requiring" the two modules might be a bit confusing, but maybe I'm wrong and that's why I'm
  asking for your feedback. I'd rather see a script that uses the same patterns, rather than having some
  function being async and others not, but that's just my opinion.

Anyway, I'll keep on working on this step by step, we should have an initial "draft" pull request.

[bryceosterhaus]

Do we want to use external dependencies to handle IO operations like creating nested directories?

I'd also try to stay away from external dependencies for this too. You could also take a look through https://github.com/liferay/liferay-frontend-projects for inspiration and just copy/paste some of our helpers.

Do we want to use both require('fs') and require('fs').promises modules?

I took a look through our other projects and it seems like we almost always use the non-promises module so I would say let's just use the non-promises one. For synchronous actions, you can just use the *Sync version of the fs method.

[matuzalemsteles]

Do we want to use external dependencies to handle IO operations like creating nested directories?

I think we can follow what Bryce put, see what already exists in frontend-projects that can help us, I think there aren't many but there's no problem if we need to implement it either.

Do we want to use both require('fs') and require('fs').promises modules?

We can still use promise only if we don't want to use the sync methods of fs, for some methods that don't have something relative in promises, we can use the Node.js utility that transforms methods with callbacks to asynchronous methods.

for example:

const util = require('util');
const fs = require('fs');

const stat = util.promisify(fs.stat);

Some APIs are not available in asynchronous because most of them are synchronous maybe it gives the impression that not all of them are available.

[julien]

Thanks for the feedback. I'll have a look at what's in frontend-projects, I'm pretty sure there are somethings we can pick from there. I know about util.promisify but I think there might be no need for that.