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

feat(build): add style paths #4003

Merged
merged 1 commit into from
Jan 19, 2017
Merged

Conversation

filipesilva
Copy link
Contributor

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

const includePaths = [
path.resolve(appRoot, './style-paths/')
];

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover test code?

Copy link
Contributor Author

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)$/,
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

&& appConfig.stylePreprocessorOptions.includePaths.length > 0
) {
appConfig.stylePreprocessorOptions.includePaths.forEach((includePath: string) =>
includePaths.push(path.resolve(appRoot, includePath)));
Copy link
Contributor

@admosity admosity Jan 17, 2017

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"
        ]
      }

Copy link
Contributor Author

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.

Copy link
Contributor

@hansl hansl left a 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()); }
Copy link
Contributor

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.

Copy link
Contributor Author

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]` },
Copy link
Contributor

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

Copy link
Contributor Author

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.

@filipesilva filipesilva force-pushed the style-paths branch 2 times, most recently from 28c1a23 to c181c0c Compare January 19, 2017 00:40
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
@filipesilva filipesilva merged commit e5ef996 into angular:master Jan 19, 2017
@filipesilva filipesilva deleted the style-paths branch January 19, 2017 12:08
MRHarrison pushed a commit to MRHarrison/angular-cli that referenced this pull request Feb 9, 2017
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
@Brocco Brocco mentioned this pull request Mar 6, 2017
38 tasks
filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Mar 10, 2017
This feature was introduce in angular#4003 but never documented.
hansl pushed a commit that referenced this pull request Mar 13, 2017
This feature was introduce in #4003 but never documented.
asnowwolf pushed a commit to asnowwolf/angular-cli that referenced this pull request Apr 12, 2017
This feature was introduce in angular#4003 but never documented.
@ghost
Copy link

ghost commented May 28, 2017

Any workaround for less?

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Passing additional options to node-sass
5 participants