nystudio107 / rollup-plugin-critical

Vite.js & Rollup plugin for generating critical CSS
MIT License
91 stars 11 forks source link

Specifying source CSS files is not working #17

Closed martinhellwagner closed 10 months ago

martinhellwagner commented 10 months ago

Describe the bug

We're running a project with three different CSS stylings. This is why just generating Critical CSS without specifying CSS source files leads to wrong styling. However, when specifying a CSS source file, the Critical CSS files get created but are empty.

This is the code we're using:

    critical({
      criticalUrl: 'https://www.karriere.at',
      criticalBase: path.resolve('./templates/b2c/'),
      criticalPages: [
        { uri: '/agb', template: 'entry/pagesB2C/cpDefault' },
        { uri: '/blog', template: 'entry/homeBlog/homeBlog' },
        { uri: '/c/a/entscheidung', template: 'entry/blog/default' },
        { uri: '/ueber-uns', template: 'entry/pagesB2C/lpDefault' },
      ],
      criticalConfig: {
        width: 1680,
        height: 1200,
        css: [
         './assets/css/app.css',
        ],
        ignore: {
          rule: [/o-footer/],
        },
        rebase: asset => (asset.url.includes('https://content.karriere.at') || asset.url.includes('https://kcdn.at')) ? `${asset.url}` : `https://content.karriere.at${asset.url}`,
      },

To reproduce

Steps to reproduce the behaviour: Create Critical CSS with above code. If the css config is set, empty Critical CSS files are created. If the css config is not set, wrong Critical CSS files are created.

Expected behaviour

Critical CSS files are generated and have the correct content.

Versions

martinhellwagner commented 10 months ago

Probably import as well: this is how we are using the three different imports in Vite. However, for each Critical CSS task, we'd like to use one import.

    rollupOptions: {
      input: {
        app: './assets/js/app.ts',
        app_b2c: './assets/js-b2c/app.js',
        app_hr: './assets/js-hr/app.js',
      },
      output: {
        sourcemap: true,
      },
    },
martinhellwagner commented 10 months ago

Not sure if the css option should point to a source or dist file, but pointing to the dist file is kind of difficult since it has a changing hash each time the build is created – and all styles are merged together into one file (e.g. dist/assets/app-7e6383bb.css).

Would be great if I could specify a set of source CSS files that are used for the Critical CSS, even though I'm using multiple input files.

martinhellwagner commented 10 months ago

@khalwat

Any thoughts or ideas on that topic? It'd be great to have three different Critical CSS jobs with three different sources. 💪🏼

khalwat commented 10 months ago

Not offhand, no. It's not a scenario I've encountered, but if you find a change/fix, PRs are welcome.

martinhellwagner commented 10 months ago

@khalwat

Got it, I‘ll look into it. Just to confirm, should the css parameter point to the src or dist file?

khalwat commented 10 months ago

I believe it should point to the final dist result... but it's not something I use because normally it takes the CSS directly from Vite, so it knows what to use as part of the project.

martinhellwagner commented 10 months ago

Okay, so I somehow need to find a way to use the three inputs as separate CSS „sources“? 🤔 Is it possible to have multiple Vite config files or exports?

martinhellwagner commented 10 months ago

@khalwat

I've now opted for the following solution:

"scripts": {
    "build": "vite build --config vite.config.critical.b2c.js && vite build --config vite.config.critical.hr.js && vite build"
  }

During the build, first the Critical CSS for the B2C styles (Vite inputs 1 & 2) and then the Critical CSS for the HR styles (Vite input 3) get built, using only the critical plugin in a Vite config file. The Critical CSS files are pushed directly into the directories where their respective Twig files are located.

After that, the "normal" Vite build (without the critical plugin) is running through for all three inputs – leaving the correct files in the dist folder.

It's not a pretty solution, but it works for now.