dirkolbrich / hugo-tailwindcss-starter-theme

Starter files for a Hugo theme with Tailwindcss
MIT License
401 stars 55 forks source link

Debloat and improve tailwindcss configuration #21

Closed diegosanchezp closed 3 years ago

diegosanchezp commented 4 years ago

I've started to notice that your current configuration of purgeCSS will produce bugs on production, I've made my portfolio using this starter template and started to notice that some of the CSS classes that i've made were removed when building, which was really annoying not seeing the changes I've done in development.

Here are the changes the I've made to fix the problem

  1. As a first point the dependency @fullhuman/postcss-purgecss is not required, tailwindcss already has it, the documentation clearly says

PurgeCSS (the library we use under the hood)

  1. Based on the first point you could introduce these changes 2.1 Provide an array of paths to all of your template files using the purge field in tailwind.config.js
    
    const themeDir = __dirname + '/../../';

module.exports = { // === Removing unused css === //

// 1. Provide an array of paths to all of your template files using the purge option purge: [ themeDir + 'layouts//*.html', themeDir + 'content/*/.html', 'layouts//*.html', 'content//*.html', 'exampleSite/layouts/*/.html', 'exampleSite/content//*.html', ],

// ...


2.2 Remove the purgecss config on `postcss.config.js`

3. Line 36 on `postcss.config.js` it's unnecessary, the [docs](https://tailwindcss.com/docs/controlling-file-size#removing-unused-css) say 
> Now whenever you compile your CSS with NODE_ENV set to production, Tailwind will automatically purge unused styles from your CSS.

This can be done easily using the `env` bash command which will set enviroment variables, as an example

```bash
env MODE=\"production\" NODE_ENV=\"production\" hugo
  1. And as a last point and most import one, to not remove css clases that someone else writes , the documentation states

Finally, we recommend only applying PurgeCSS to Tailwind's utility classes, and not to base styles or component classes

Which is done by adding purgeCSS ignore comment rules on assets/css/styles.css between line 1 and 6

/* purgecss start ignore */
/* Tailwind base - put variables under: tailwind.config.js */
@import "node_modules/tailwindcss/base";
/* Tailwind component classes registered by plugins*/
@import "node_modules/tailwindcss/components";
/* Site Specific */
@import "assets/css/site";
/* purgecss end ignore */
/* Tailwind's utility classes - generated based on config file */
@import "node_modules/tailwindcss/utilities";

Without this, there will be removed classes

As an example if you use gohugo's auto generated structures such as code fences which include the class .highlight and you reference this class on an external stylesheet PurgeCSS will remove it on since it does not knows about it.

I guess then you could easily make changes to the configuration. To fix these problems.

You have my portfolio repository as an example. Here are the changes that i've made to do the fix.

dirkolbrich commented 4 years ago

Hey, thanks for your proposed changes.

At first: can you provide an reproducible example or repo, which uses the exact TailwindCSS and PurgeCSS configuration used by this starter package (using Hugo’s internal PostCSS setup, not relying on NPM or NODE_ENV) and shows the same problem of removed css classes you encounter? This would help in verifying if the configuration of this package causes the removal of used CSS classes.

The TailwindCSS documentation describes a setup, which uses the NPM environment (via NODE_ENV === 'production') to build and preprocess assets. One of the purposes of this starter theme is actually to take NPM as much as possible out of the process and rely on Hugo Pipes and the build-in PostCSS configuration. The only use of NPM is to download the needed packages from the registry.

My impression of your approach seems to be very NPM centric and uses the preprocessor configuration of a typical Node project, but ignores the settings needed for a Hugo project. I think that the problem you encounter stems from the fact that you moved the PurgeCSS process into the TailwindCSS config instead of using PurgeCSS via the PostCSS process setup used by Hugo’s Pipes.

To your points:

  1. @fullhuman/postcss-purgecss is a required dependency as the Hugo docs clearly says(emphasis by me):

Hugo Pipe’s PostCSS requires the postcss-cli JavaScript package to be installed in the environment (npm install -g postcss-cli) along with any PostCSS plugin(s) used (e.g., npm install -g autoprefixer).

  1. With this point you move the Hugo PostCSS configuration into the dependency TailwindCSS configuration.

I don't think this a good idea as it spreads configuration across different config files. With this package I tried to leave the Tailwind configuration as untouched a possible, at least the part which is not a core functionality to TailwindCSS. Purging CSS class names is such a functionality, which is better dealt from the main system. Hugo’s Pipes system (with PostCSS) should contain the configuration for all processes to handle files and the configuration to do what on which ENV setting.

  1. No, it is not unnecessary.

This line checks for the HUGO_ENVIRONMENT variable to decide if to purge CSS classes. As stated above, the intend is not to use NPM but instead to use Hugo’s build-in functionality.

Your approach introduces an unnecessary step in need to set a NODE_ENV variable. Hugo sets the HUGO_ENVIRONMENT variable automatically. Why introduce a new dependency onto a NODE_ENV variable and not make use of what is already there?

  1. You left out an important next paragraph of this quote from the TailwindCSS docs which states further (emphasis from me):

This will ensure you don't accidentally purge important base styles when working with frameworks like Next.js, Nuxt, vue-cli, create-react-app, and others that hide their base HTML template somewhere in your node_modules directory.

If you need to use these frameworks (which I doubt is useful when deciding to use a static page generator like Hugo), why not use Hugo's Pipes functionality to process js files?


The problem you stated initially with this issue says:

As an example if you use gohugo's auto generated structures such as code fences which include the class .highlight and you reference this class on an external stylesheet PurgeCSS will remove it on since it does not knows about it.

This problem is unrelated to using the purgecss start ignore comment rule within the Tailwindcss configuration file.

You rely on using the default PurgeCSS setup from within the TailwindCSS configuration. TailwindCSS builds its CSS classes and because you have set NODE_ENV it purges them automatically, before Hugo is finished building its structure. That’s why PurgeCSS does not see a use of certain classes, because those pages don’t exist jet.

The PurgeCSS command should only run when Hugo has build the complete site pages. That is why this package calls PurgeCSS from Hugo’s PostCSS process and uses Hugo's PostProcess functionality to delay any post processing after all pages are build.

diegosanchezp commented 4 years ago

Alright mate, here you have it !

To reproduce the changes, do the following steps

  1. Clone the repository git clone https://github.com/diegosanchezp/hugo-theme-tailwindcss-starter.git

Then

cd hugo-theme-tailwindcss-starter
git checkout error
  1. Install dependencies, i've used the yarn package manager, should work with npm too.

yarn install

Fun fact postcss-cli and autoprefixer are not needed globally installed. Why ? Because we will execute the hugo server process with npm run, thus knowing where postcss-cli and autoprefixer binaries are located. Hence less dependencies to install and less duplication YAY!

Here is my list of installed global dependencies

/media/diego/Windows/.npm-global/lib
├── n@6.6.0
├── npm@6.14.6
└── yarn@1.22.4

Nodejs version v14.5.0 Hugo Static Site Generator v0.73.0-428907CC/extended linux/amd64 BuildDate: 2020-06-23T16:40:09Z

  1. Serve the site

npm run server

After serving the site you should see the default template and a new sidenav that will list every section, example taken from Section Menu for Lazy Bloggers

This section menu have a template conditional that will mark the current section anchor with a class named active I've styled this on assets/css/site.css to make the current anchor appear blue.

So the main bug is class deletion in the bundled CSS code, this means that in the CSS bundle should be the class .active

  1. Build the site

npm run build

  1. Search for the css class .active in the bundled CSS code
grep -i "active" exampleSite/public/css/styles.min.<hash>.css 

You will see no output because grep didn't found that string

You might pretty print the file for easier search

If you inspect the css bundle again the class starter is not even present too, which you put originally !

The same thing will happen with code fences if you test it with one the posts in content/posts and try to style the .highlight class.

That should be it. Happy debugging !

Don't make nodejs your enemy try to integrate it with hugo as I did!

dirkolbrich commented 4 years ago

Hey, thanks for the basic issue example.

I tried your issue and rebuild it in a slightly different order. And I found the problem within your example, which comes from missing blank spaces. It’s always the little things ;-)

See branch verify-issue21 for the steps involved.

1st step

I added the lazy navigation menu as a partial like in your example. See commit 7e1a983.

Note that my list of global installed dependencies looks like this:

$ npm list -g -depth=0
/usr/local/lib
├── autoprefixer@9.8.6
├── npm@6.14.8
├── npm-check-updates@7.1.1
└── postcss-cli@7.1.1

The packages postcss-cli and autoprefixer are globally installed just as mentioned in the readme and the Hugo docs.

I start the server in development mode with:

hugo server -s exampleSite --themesDir=../.. --disableFastRender

Everything looks as expected. Testing the build with:

hugo -s exampleSite --themesDir=../..

And you are right. The CSS classes .starter and .active are not in /public/css/styles.min.{hash}.css. Why?

  1. The class .starter was only defined in the site.css file, but not used in the templates. Thus the purging process removed this class correctly. I will add it in the next step for verification.
  2. The extractor regex defined in posts.config.js, see the TailwindCSS docs as well, does not find the class .active in the sidenav template, because there is a blank space missing. You used the following line to add the menu condition:
class="sidebar-nav-item{{if or ($currentPage.IsMenuCurrent "main" .) ($currentPage.HasMenuCurrent "main" .) }} active{{end}}"

But instead it should read:

class="sidebar-nav-item {{if or ($currentPage.IsMenuCurrent "main" .) ($currentPage.HasMenuCurrent "main" .) }} active {{end}}"

You can test this for the broadMatches and innerMatches regex strings.

2nd step

Adding the .starter class and fixing the blank space typo, see commit 56270c7.

Testing the package in dev and build, everything works as expected. The classes are present and not purged from the css file.

3rd step

I replaced the Hugo commands with NPM scripts as in your example and run the server with npm run server. See commit 689154f.

Note that postcss-cli is still installed globally. Everything works, no change to the previous step.

4th step

Testing with cleaned global dependencies:

$ npm list -g -depth=0
/usr/local/lib
├── npm@6.14.8
└── npm-check-updates@7.1.1

(Testing this with the Hugo commands for dev and build not with the NPM scripts)

hugo -s exampleSite --themesDir=../..

This works too, which I kind of find weird as it contradicts the Hugo docs. I have no explanation for that, but OK.

Conclusion

I think I will add the NPM script for starting dev and build to the package. Thanks for the heads-up to this.

diegosanchezp commented 4 years ago

Hello again, good findings, thanks for the detailed explanation.

I think your conclusion is good.

What I would like to see then is to put in the README.md a side note that says something like.

If you execute the hugo process via npm scripts that is npm run dev and npm run build, you will not need globally installed postcss-cli and autoprefixer. Why? Beacuse hugo knows where the binaries are located.

I thinking to start an issue in the hugo repository to keep further discussion, maybe linking to this issue and see what the experts say.

I'll close this issue in two days if there is no further activity.