feat(sass resolve paths)

[ValeryVS]

Change webpack config to implement sassLoader's includePaths support.

Fix #1791

[grizzm0]

What's your use case for this?

[ValeryVS]

@grizzm0
For exampe, I store global sass variables in src/styles/_variables.sass and import them in my components sass files.

src/
    styles/
        _variables.sass
        main.sass

// angular-cli.json
{
  "apps": [
    {
      "styles": [
        "styles/main.sass"
      ],
      "sassPaths": [
        "styles"
      ],
    }
  ]
}

// app.component.sass
@import 'variables'

:host
    color: $text-color

[grizzm0]

Both less and stylus has support for paths aswell. If you're going to implement it you should create an implementation that works across all three.

[clydin]

As @grizzm0 indicated there are similar options for less and stylus, generalizing the setting would probably be a good idea. Having a sass specific sub-option at the app level seems like a bad precedent to set.

Another option is to allow node-sass settings (as well as less and stylus) to be set via a more comprehensive addition to the config file. However, this may extended into the future add-on support. (which this PR may as well).

@filipesilva, out of curiosity, is there a plan or initial design concept for the add-ons feature?

[ValeryVS]

As far as I can see, less-loader use webpack's resolve.modules.
https://github.com/webpack/less-loader
And stylus-loader use own resolver by default, but can fallback to webpack's resolver.
https://github.com/shama/stylus-loader

Probably, resolve.modules or even resolve.aliases can be used to implement this for sass/less/stylus.

[ValeryVS]

Acualy, I like resolve.alias.
In this case, we can import our applications styles with

@import '~@styles/variables'

But, we, probably, should care about conflicts with tsconfig's paths option.

[clydin]

@ValeryVS, I experimented with alias as well. It's somewhat awkward as it can conflict with ts (as you said) and the stylesheets are now specific to webpack and its configuration.

The less and stylus loaders both seem to support a paths option. At least from a cursory look at the source code.

Overall, stylesheet configuration is extremely basic right now. A more detailed plan to allow configuration of the available pre-processors as well as postcss would probably be a good long-term goal. Not that this PR is the place for that. But include paths is a good first step, as this is the missing analog to tsconfig's path support.

[filipesilva]

So this functionality... using alias is suboptimal, includePaths in general is better. An implementation that offers includePathsshould offer parity across supported engines as well.

We're wary of adding new properties to angular-cli.json but this one seems to need to be there. Name should be generic, perhaps cssPreprocessorIncludePaths? It's kinda long but just cssIncludePaths implies that css imports also support it, but afaik css-loader doesn't.

Tests would also be required to make sure that it works, https://github.com/angular/angular-cli/tree/master/tests/e2e/tests/build/styles is the best spot to add them.

@clydin nothing to share yet, I'm sorry. It's post 1.0 still.

[ValeryVS]

@filipesilva Thanks for reply and instructions. I will check less and stylus loaders for paths support, add tests and update request.

[frederikschubert]

This is blocked until webpack-contrib/less-loader#75 is merged.

@filipesilva sass-loader and stylus-loader support includePaths/paths. Would it be possible to support these two and add support for less when the above PR is merged? I implemented an e2e test for sass based on this PR like you suggested and can also add an e2e test for stylus.

[gjuchault]

What if we create another pull-request that is not precompiler but that is not waiting for less-loader to merge a 8months old ?

This is actually very blocking: s(a|s)css import paths are just mandatory to use

[ValeryVS]

I think we should make sass (and stylus) paths support. And then wait for less-loader update.
There is paths option in less cli, so, somtime they will implement this in loader.

[frederikschubert]

@filipesilva What do you think? This feature is the only reason why my company has to use its own workflow instead of the angular-cli. So it would be great to get this in!

[filipesilva]

I think it would be good for such support to go in, but am concerned about the robustness of such a system.

Besides being well tested, we'd have to adequately (and dynamically) configure each loader that makes use use of include paths. So that's the reason why this feature needs to be well planned.

I'm going to bring it up for direct discussion though in hope we can prioritise it.

[admosity]

I've provided a use case in: #3683

I need to use a sass library that in itself relies on another third party library (in this case foundation-sites). This library is blackbox to me (private npm package) and is mandated to be used by my client :/

Also to illustrate the usage with the top level import with component sass:

[kyranjamie]

Hugely valuable feature. Another use case is utilising styles from material-components or other libs using npm scoped modules

[zackarychapple]

I think this is critical for people using bootstrap as well as a base in our component libraries in enterprises. Referencing node_modules directly "works" in the component lib but breaks when we pull the lib in and the cli tries to resolve the scss files for the component library.

[fxck]

I've encountered this as well, using AotPlugin rather than the whole cli, it's another one of those deal breakers that makes @ngtools/webpack currently unusable in bigger apps.

Is there any general consensus on this PR?

[admosity]

I have an up to date implementation in #3724 that supports sass and stylus. I think the issue we're up against is that there's a one year old PR for the webpack less loader preventing support for LESS: webpack-contrib/less-loader/pull/75

I'd agree on supporting less later though...

[filipesilva]

Superseded by #4003

[angular-automatic-lock-bot]

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