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

stop working from release 3.5.2 #27

Closed
mdellanave opened this issue Sep 25, 2024 · 17 comments
Closed

stop working from release 3.5.2 #27

mdellanave opened this issue Sep 25, 2024 · 17 comments

Comments

@mdellanave
Copy link

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
Copy link
Owner

jleeson commented Sep 25, 2024

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
Copy link

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

Revert: bavix/uuid-ui#198

@jleeson
Copy link
Owner

jleeson commented Sep 25, 2024

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

@rez1dent3
Copy link

rez1dent3 commented Sep 25, 2024

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

@jleeson
Copy link
Owner

jleeson commented Sep 25, 2024

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
Copy link
Owner

jleeson commented Sep 25, 2024

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
Copy link

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

@jleeson
Copy link
Owner

jleeson commented Sep 25, 2024

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

@rez1dent3
Copy link

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

@rez1dent3
Copy link

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: bavix/uuid-ui#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
Copy link

I think the problem is here:
7b40eee

@jleeson
Copy link
Owner

jleeson commented Sep 25, 2024

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
Copy link
Owner

jleeson commented Sep 25, 2024

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
Copy link
Author

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

@mdellanave
Copy link
Author

mdellanave commented Sep 26, 2024

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
Copy link
Owner

jleeson commented Sep 26, 2024

@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
Copy link
Owner

jleeson commented Sep 26, 2024

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

@jleeson jleeson closed this as completed Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants