jleeson / rollup-plugin-import-css

A Rollup plugin to import CSS into JavaScript
MIT License
50 stars 6 forks source link

stop working from release 3.5.2 #27

Closed mdellanave closed 1 month ago

mdellanave commented 1 month ago

Hey, I'm using this plugin to rollup ckeditor files.

Since the release of the 3.5.2 version the plugin do not export the css as in the previous version.

my rollup dep

  "@rollup/plugin-commonjs": "^28.0.0",
  "@rollup/plugin-node-resolve": "^15.3.0",
  "@rollup/plugin-terser": "^0.4.4",
  "rollup": "^4.22.4",
  "rollup-plugin-import-css": "3.5.2",

my main.js contains this

 import 'ckeditor5/ckeditor5.css' assert { type: 'css' };

this is my rollup task

const bundle = await rollup({
    input: './rollup/ckeditor/main.js',
    plugins: [
      css({
        minify: true,
        output: 'custom-ckeditor.min.css',
      }),
      commonjs(),
      resolve(),
      terser(),
    ],
  });

  await bundle.write({
    file: './rollup/ckeditor/dist/custom-ckeditor.min.js',
    format: 'es',
  });

As I said, if I run it with 3.5.1 version the file custom-ckeditor.min.css is going to be created, but not with the latest version 3.5.2 published on npm.

Am I missing something?

jleeson commented 1 month ago

Hi @mdellanave, it looks like you are mixing import types. Until the latest version, import assertions were not working in Rollup 4, so it was originally working due to ignoring the assert syntax which worked in this plugin with Rollup 3.

When using assert { type: "css" } it is converting the css into a css module for you to import as a variable. ex: import styles from "./styles.css" assert { type: "css" } In order to avoid duplicate css in an application, we only output css that was not imported as a variable.

It sounds like you are intending to only have the css imported so it gets included in a bundle rather than a css module. So everything should work if you change your import to be: import 'ckeditor5/ckeditor5.css';

Let me know if that works or not.

rez1dent3 commented 1 month ago

Hello. I have a similar problem. I reversed the update and the problem went away.

Revert: https://github.com/bavix/uuid-ui/pull/198

jleeson commented 1 month ago

@rez1dent3, are you utilizing the assertion syntax as well or are you using plain imports like import "./styles.css";?

rez1dent3 commented 1 month ago

Yes, but I tried using with { type: 'css' } (assert deprecated) too.

jleeson commented 1 month ago

when using the assert (or with) syntax, the css being imported that way, will not be included in the css bundle, instead it is included in the js bundle as such: ex: const sheet = new CSSStyleSheet();sheet.replaceSync(".red{color: red;}");

Are the imports that look like import "./styles.css"; not being included in the css bundle?

jleeson commented 1 month ago

Here is an example I currently have working:

rollup.config.mjs

import css from "rollup-plugin-import-css";

export default {
    input: "src/index.js",
    output: [
        { file: "dist/index.js", format: "esm" },
    ],
    plugins: [
        css({ minify: true, output: "bundle.css" })
    ],
};

src/index.js

import "./index.css";

src/index.css

.red {
    color: red;
}

This correctly outputs the bundle.css file, if i do any other kind of css import, it will not be included in the bundle.

rez1dent3 commented 1 month ago

I updated the plugin @babel/plugin-syntax-import-attributes to version 7.25.6 and it worked. Hmm.. Thanks for the help.

jleeson commented 1 month ago

Got it, sounds like it may have been a conflict with the import attributes plugin and rollups native support for it then.

rez1dent3 commented 1 month ago

Yes, it is possible. Thanks for the quick reply.

rez1dent3 commented 1 month ago

When I said that the problem was in the plugin @babel/plugin-syntax-import-attributes, I simply forgot to update your package. I checked on version 3.5.1 :there should be a picture of harold here:

Revert revert: https://github.com/bavix/uuid-ui/pull/203

My imports:

import '@creativebulma/bulma-tooltip/dist/bulma-tooltip.css'
import 'bulma/css/bulma.css'
import './app.css'
//...
import "@theme-toggles/react/css/Expand.css"

diff: https://www.diffchecker.com/ck0onuCe/

Was / became:

Screenshot 2024-09-25 at 23 21 36
rez1dent3 commented 1 month ago

I think the problem is here: https://github.com/jleeson/rollup-plugin-import-css/commit/7b40eee1af771f2562b3e4fb766cfe443af80953

jleeson commented 1 month ago

You are right @rez1dent3, this is a result of the css minifier changes, this is unrelated to this issue then, the css is being bundled where this issue was opened regarding the css not being bundled in the output. Would you mind opening another issue regarding the css minification changes?

jleeson commented 1 month ago

Just to reference this again since the discussion here went into a different issue, In order to resolve the original problem, please remove the assert syntax and use a plain import like import 'ckeditor5/ckeditor5.css'; and it should work properly.

Hi @mdellanave, it looks like you are mixing import types. Until the latest version, import assertions were not working in Rollup 4, so it was originally working due to ignoring the assert syntax which worked in this plugin with Rollup 3.

When using assert { type: "css" } it is converting the css into a css module for you to import as a variable. ex: import styles from "./styles.css" assert { type: "css" } In order to avoid duplicate css in an application, we only output css that was not imported as a variable.

It sounds like you are intending to only have the css imported so it gets included in a bundle rather than a css module. So everything should work if you change your import to be: import 'ckeditor5/ckeditor5.css';

Let me know if that works or not.

mdellanave commented 1 month ago

Just installed the 3.5.3 versione and removed the assert syntax and it worked! Thank you @jleeson and thank you all guys.

mdellanave commented 1 month ago

Sorry, I have to say there's something still not working as expected. The file custom-ckeditor.min.css is now built but it contains error and it's not possible to include it in a sass processing pipe.

I think the problem is due to the way background-image urls are managed

The linter show me a problem here image

do you have some idea on how to solve it?

jleeson commented 1 month ago

@mdellanave, This is related to #28, there was a minification change introduced in order to support custom properties in attribute selectors and it seems to have broken many cases where a colon is present. I will be working on a fix for this today.

jleeson commented 1 month ago

Closing this issue since the original problem is resolved and the new problem is the same as #28