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

@CssImport breaks if you add 'css-loader' to webpack #6276

Closed
Artur- opened this issue Aug 19, 2019 · 8 comments
Closed

@CssImport breaks if you add 'css-loader' to webpack #6276

Artur- opened this issue Aug 19, 2019 · 8 comments

Comments

@Artur-
Copy link
Member

Artur- commented Aug 19, 2019

If you add css-loader to your webpack config, Flow automatically starts using that for @CssImports and instead of CSS, creates style tags like this:

<style>exports = module.exports = require("../node_modules/css-loader/dist/runtime/api.js")(false);
// Module
exports.push([module.id, ":host {\n    background: red;\n    color: white;\n}", ""]);
</style>

Example project: https://github.com/Artur-/test-css-import-css-loader

@project-bot project-bot bot added this to Inbox - needs triage in OLD Vaadin Flow ongoing work (Vaadin 10+) Aug 19, 2019
@Artur-
Copy link
Member Author

Artur- commented Aug 19, 2019

Can be fixed by generating code like

import $css_0 from '!!raw-loader!Frontend/test.css';

instead of

import $css_0 from 'Frontend/test.css';

@pleku pleku added this to Needs triage in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) via automation Aug 23, 2019
@pleku pleku removed this from Inbox - needs triage in OLD Vaadin Flow ongoing work (Vaadin 10+) Aug 23, 2019
@ujoni ujoni moved this from Needs triage to P2 - Medium Priority in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Aug 27, 2019
@karlitos
Copy link

karlitos commented Jan 9, 2020

Hello, I just stumbled upon this issue when trying to use Webpack css-loader. So far I could do

@CssImport("./styles/global.css")
@CssImport("bootstrap/dist/css/bootstrap.min.css")

to be able to use bootstrap styling. After installing the css-loader it stopped working - now I know why.

Anyhow, my intention was to use ES6 imports in a index.js to import all necessary js libraries and stylesheets and than having only one @javascrip annotation including this file. This is why I installed the css-loader in first place.

I do not not get the workaround and how can it be used with css files installed with npm

@Artur-
Copy link
Member Author

Artur- commented Jan 9, 2020

The fix code above is a suggestion for the code generated by Flow, it is not a workaround for a project

@jouni
Copy link
Member

jouni commented Aug 6, 2020

@karlitos: For your project’s workaround, I suppose you could make the css-loader glob more strict so that it only matches the style sheets that are loaded by the index.js

For example (replace foobar with whatever you like):

webpack.config.js

...
        test: /\.foobar.css$/i,
        use: ['css-loader']
....

@Haprog
Copy link
Contributor

Haprog commented Aug 11, 2020

Related to issue vaadin-learning-center/crm-tutorial-typescript#8 we're thinking about making Flow by default use a loader chain of ['lit-css-loader', 'extract-loader', 'css-loader'] for CSS files (instead of the current ['raw-loader']). This will give us nice UX for importing styles to LitElement based views (since it gives a CssResult object and it also works for @CssImport since CssResult has a toString() method. In this case the benefit of css-loader is mainly to get resolving of @import in CSS files to work (otherwise we could use just lit-css-loader by itself).

But I'm wondering what was the original use case behind this issue, as in can we get this issue resolved by doing this? Is it about getting @import rules to work, or is it about wanting to get a JS module instead of a CSS string when doing import styles from 'test.css'; for your own purposes?

If the user wants to customize it further in a way that the JS import of a CSS file doesn't return a plain CSS string I think it would make sense to recommend overriding the CSS loader in project config only for specific directories/files in that case.

@Artur-
Copy link
Member Author

Artur- commented Aug 12, 2020

I think my original case was using monaco-editor that says in the installation instructions at https://github.com/microsoft/monaco-editor/blob/master/docs/integrate-esm.md that you should use

  module: {
    rules: [{
      test: /\.css$/,
      use: ['style-loader', 'css-loader']
    }, {
      test: /\.ttf$/,
      use: ['file-loader']
    }]
  }

@Haprog
Copy link
Contributor

Haprog commented Aug 12, 2020

@Artur- Thanks for the context. In that case I would guess that monaco-editor itself depends on style-loader being used for its own styles and you'd probably be able to limit the test: /\.css$/, part to some directory/directories where the monaco-editor styles live and only use ['style-loader', 'css-loader'] chain in that context and not for all CSS files.

@Haprog
Copy link
Contributor

Haprog commented Aug 12, 2020

I think Flow can assume a specific format for webpack CSS loader output. If the user modifies the loader configuration they should make sure that the output of their loader configuration (for any CSS files imported using Flow features like @CssImport() in Java) is compatible. The CSS loader should return a plain CSS string (or something with a toString() method returning a plain CSS string). Or if/when we change to use lit-css-loader by default, the output should be a CSSResult object if they use our (future) recommended way of loading styles into TS/Lit views.

It should be ok to modify global CSS loader config as long as the output format stays compatible.

If the user wants or needs specific CSS files to be loaded differently, they should make sure their loader config overrides only affect those files to not break @CssImport(). And if they want to use our (future) recommended way of importing styles to TS/Lit views, then those CSS files also need to be loaded in a compatible way.

Related: #8843

@Artur- Artur- closed this as completed Nov 22, 2022
OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from New P2 to Closed Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants