cli icon indicating copy to clipboard operation
cli copied to clipboard

[BUG] with webpack, inlineView deps (<require from="./style.css"></require>) doesn't work

Open 3cp opened this issue 7 years ago • 11 comments

I'm submitting a bug report

  • Library Version: 0.34.0

Please tell us about your environment:

  • Operating System: all

  • Node Version: all

  • NPM Version: all

  • Browser: all

  • Language: all

  • Loader/bundler: Webpack

Current behavior:

aurelia-webpack-plugin did its job to bring in the inlineView required "./style.css". But our cli webpack.config.js prevented it from working properly.

It's caused by:

// CSS required in JS/TS files should use the style-loader that auto-injects it into the website
// only when the issuer is a .js/.ts file, so the loaders are not applied inside html templates
{
  test: /\.css$/i,
  issuer: [{ not: [{ test: /\.html$/i }] }],
  use: extractCss ? [{
      loader: MiniCssExtractPlugin.loader
    },
    'css-loader'
  ] : ['style-loader', ...cssRules]
},
{
  test: /\.css$/i,
  issuer: [{ test: /\.html$/i }],
  // CSS required in templates cannot be extracted safely
  // because Aurelia would try to require it again in runtime
  use: cssRules
},

The "./style.css" dep loaded from inlineView is NOT from issuer html, so it ended up with style-loader instead of packed as a module.

  • What is the expected behavior?

The dep loaded by inlineView should be treated same as dep loaded by html file.

  • What is the motivation / use case for changing the behavior?

Need to update the rules to avoid this regression. But I don't know webpack enough to fix this.

cc @jods4 this bug prevents aurelia/webpack-plugin#107 from working.

3cp avatar Aug 15 '18 00:08 3cp

@huochunpeng's analysis is correct. This is a problem with the way loaders are configured.

The issuer tests in config ensure .css dependencies from a .ts file are loaded with style-loader.

This is adequate for import "main.css" pattern. Not standard JS but quite common.

It is not adequate for @inlineView("<require from='main.css'>") because this one would be loaded by aurelia-loader.

It is hard to come up with a config that caters for all needs. If you use import "main.css" you typically don't use <require from='main.css'> and vice-versa.

One possible solution is to never include style-loader by default and add it explicitely in import. import from "style!main.css".

jods4 avatar Aug 15 '18 09:08 jods4

The solution you proposed is a breaking change for users' source code. We can do the other way around to avoid breaking users' code.

Since all dynamic loading of css files are traced and ONLY traced by aurelia-webpack-plugin,

  1. from a html template
  2. from an inlineView template
  3. from PLATFORM.moduleName, like @noView([PLATFORM.moduleName('./style.css')]);

When aurelia-webpack-plugin do addDependency('./style.css'), it can add prefix like "css!./style.css". Then they will all be loaded by css-loader. (Tell me if webpack.config.js needs a update).

The non-standard import "./style.css"; is traced by standard webpack tracer, they will end up as module "./style.css" without prefix, which you can enforce style-loader on them.

@jods4 is this feasible in webpack?

3cp avatar Aug 15 '18 23:08 3cp

@huochunpeng That's quite hard to do. Not everyone uses css!. I use css!less!, others use css!sass! and yet other users can pipe more loaders to do additional transforms.

Webpack is very flexible and the philosophy of webpack-plugin is to embrace this flexibility and let users configure their build, with some reasonable defaults when it comes to Aurelia.

I am not familiar with our CLI but it seems to me that this issue lies into the webpack configuration not the plugin.

I suppose that would be a change in our templates for new projects, wouldn't it? If it's a change in new templates then it can't be a breaking change?

jods4 avatar Aug 15 '18 23:08 jods4

Yes, it's a cli issue. I also sensed adding prefix by webpack-plugin is a violation of webpack design.

But is there any way for webpack-plugin to give a hint (other than just issuer) for later webpack config file that a css file is for dynamic loading?

Maybe, something like: this dep is traced by plugin "AureliaPlugin", not standard tracer.

3cp avatar Aug 15 '18 23:08 3cp

An update to cli template is unavoidable since the current one has bug in supporting basic aurelia convention. In that sense, it's not a breaking change but a hot fix.

3cp avatar Aug 16 '18 00:08 3cp

Any update on this? Is there a work-around to get .scss files working with '@inlineView'? I think I managed to get it working by changing:

     {
        test: /\.scss$/,
        use: ['style-loader', 'css-loader', 'sass-loader'],
        issuer: /\.[tj]s$/i
      },
      {
        test: /\.scss$/,
        use: ['css-loader', 'sass-loader'],
        issuer: /\.html?$/i
      },

to this:

      {
        test: /\.scss$/,
        use: ['css-loader', 'sass-loader'],
      },
      // {
      //   test: /\.scss$/,
      //   use: ['css-loader', 'sass-loader'],
      //   issuer: /\.html?$/i
      // },

but not sure what other things that might break later. I only have .scss files of my own, but may end up with other 3rd party or external css at some point. It probably causes me no real pain to not use '@inlineView' - at least for now.

stevies avatar Sep 15 '18 15:09 stevies

@stevies I am not sure there has to be a "work-around" here.

You just need a webpack config that matches your usage. If you don't import "x.scss" in your JS files, then it makes perfect sense to use the simplified rule you're using. That's what I do in my own project.

When it comes to 3rd party, you can always filter node_modules out from your loader config.

jods4 avatar Sep 15 '18 20:09 jods4

Thanks, I'll keep it simple for now.

stevies avatar Sep 15 '18 20:09 stevies

Can be closed?

Alexander-Taran avatar Oct 14 '18 16:10 Alexander-Taran

No objections from me.

stevies avatar Oct 14 '18 19:10 stevies

Still a cli wepack config issue with no solution.

3cp avatar Oct 14 '18 21:10 3cp