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

refactor: manage loader resolution with webpack #287

Closed
wants to merge 2 commits into from
Closed

refactor: manage loader resolution with webpack #287

wants to merge 2 commits into from

Conversation

chrislloyd
Copy link

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

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

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

delijah commented Oct 11, 2016

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

@sokra
Copy link
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
Member

Fixes #286

@michael-ciniawsky
Copy link
Member

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