-
Notifications
You must be signed in to change notification settings - Fork 12k
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
feat(build): add style paths #4003
Conversation
e8a741d
to
4476f61
Compare
const includePaths = [ | ||
path.resolve(appRoot, './style-paths/') | ||
]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover test code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, my bad
disable: !extractCss | ||
}), | ||
new webpack.LoaderOptionsPlugin({ | ||
test: /\.(css|scss|sass|less|styl)$/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test technically isn't needed and It adds an additional check per resource request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
ce516be
to
bdd4b36
Compare
&& appConfig.stylePreprocessorOptions.includePaths.length > 0 | ||
) { | ||
appConfig.stylePreprocessorOptions.includePaths.forEach((includePath: string) => | ||
includePaths.push(path.resolve(appRoot, includePath))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using appRoot
requires me to specify my includePaths
relative to the current appRoot
. I guess that's not bad. Here's a sample configuration I have to do and it works 👍 :
"stylePreprocessorOptions": {
"includePaths": [
"../../node_modules",
"../src"
]
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah in general anything that's within app config is always relative to appRoot
. This way it's consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing major. LGTM in general.
} | ||
|
||
// supress empty .js files in css only entry points | ||
if (extractCss) { extraPlugins.push(new SuppressExtractedTextChunksWebpackPlugin()); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split on a separate line please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
{ test: /\.html$/, loader: 'raw-loader' }, | ||
|
||
{ test: /\.(eot|svg)$/, loader: `file-loader?name=[name]${hashFormat.file}.[ext]` }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those new loaders are not part of the issue..?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They aren't new, they were just moved up from a few lines below.
28c1a23
to
c181c0c
Compare
Add `paths/includePaths` functionality for `sass` and `stylus`. Similar functionality for less is blocked by webpack-contrib/less-loader#75. To add paths, use the new entry in `angular-cli.json` app object: ``` "stylePreprocessorOptions": { "includePaths": [ "style-paths" ] }, ``` Fix angular#1791
c181c0c
to
f1cd870
Compare
Add `paths/includePaths` functionality for `sass` and `stylus`. Similar functionality for less is blocked by webpack-contrib/less-loader#75. To add paths, use the new entry in `angular-cli.json` app object: ``` "stylePreprocessorOptions": { "includePaths": [ "style-paths" ] }, ``` Fix angular#1791
This feature was introduce in angular#4003 but never documented.
This feature was introduce in #4003 but never documented.
This feature was introduce in angular#4003 but never documented.
Any workaround for less? |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Add
paths/includePaths
functionality forsass
andstylus
.Similar functionality for less is blocked by webpack-contrib/less-loader#75.
To add paths, use the new entry in
angular-cli.json
app object:Fix #1791