Skip to content

refactor: manage loader resolution with webpack#287

Closed
chrislloyd wants to merge 2 commits intowebpack:masterfrom
chrislloyd:use-webpack-for-composed-dependencies
Closed

refactor: manage loader resolution with webpack#287
chrislloyd wants to merge 2 commits intowebpack:masterfrom
chrislloyd:use-webpack-for-composed-dependencies

Conversation

@chrislloyd
Copy link
Copy Markdown

Enables the behavior detailed in #286.

Instead of assuming that child-loaders are the same as the parent, use Webpack’s build in dependency resolution. The benefits of this is that it’s easier to treat the CSS dependency tree differently (i.e. extract
text for certain vendor libs). The tradeoff is that it gives people the ability to shoot themselves in the foot if they misconfigure their loaders. This is worthwhile IMHO as the error will fail fast.

It shouldn’t change behavior for existing users.

Instead of assuming that child-loaders are the same as the parent, use
Webpack’s build in dependency resolution. The benefits of this is that
it’s easier to treat the CSS dependency tree differently (i.e. extract
text for certain vendor libs). The tradeoff is that it gives people the
ability to shoot themselves in the foot if they misconfigure their
loaders. This is worthwhile IMHO as the error will fail fast.

It shouldn’t change behavior for existing users.
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 9, 2016

Current coverage is 98.50% (diff: 100%)

No coverage report found for master at b6acfec.

Powered by Codecov. Last update b6acfec...2c151b6

@geelen
Copy link
Copy Markdown

geelen commented Jun 24, 2016

So if you've only got one loader matching all your CSS files, this will have no impact, right?

@chrislloyd
Copy link
Copy Markdown
Author

That's correct.

On Thu, Jun 23, 2016 at 6:23 PM Glen Maddern notifications@github.com
wrote:

So if you've only got one loader matching all your CSS files, this will
have no impact, right?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#287 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAACzgYOJ8fgUVL0IubLb8URzz4gU-hiks5qOzF7gaJpZM4Ixi89
.

@delijah
Copy link
Copy Markdown

delijah commented Oct 11, 2016

Could we go forward with this pull request? @sokra ?

@sokra
Copy link
Copy Markdown
Member

sokra commented Nov 17, 2016

Currently this doesn't work for the common case where you match style-loader and css-loader to *.css.

But we will probably have a better experience with webpack 2 here.

For the webpack 2 and the next version of css-loader we can recommend this ruleset:

rules: [
  { test: /\.css$/, rules: [
    { issuer: /\.js$/, use: "style-loader" },
    { use: "css-loader" }
  ] },
  { test: /\.sass$/, rules: [
    { issuer: /\.js$/, use: "style-loader" },
    { use: ["css-loader", "sass-loader"] }
  ] }
]

With this rules you can even @import sass from css...

@michael-ciniawsky michael-ciniawsky self-assigned this Apr 18, 2017
@michael-ciniawsky michael-ciniawsky changed the title Manage loader resolution with Webpack. Fixes #286. refactor: manage loader resolution with webpack Apr 18, 2017
@michael-ciniawsky
Copy link
Copy Markdown
Contributor

Fixes #286

@michael-ciniawsky
Copy link
Copy Markdown
Contributor

Closed due to inactivity, feel free to reopen :)

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

Successfully merging this pull request may close these issues.

6 participants