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

Sass compile errors #558

Closed
reinos opened this issue Apr 29, 2016 · 14 comments
Closed

Sass compile errors #558

reinos opened this issue Apr 29, 2016 · 14 comments
Labels
effort1: easy (hours) P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent type: bug/fix

Comments

@reinos
Copy link

reinos commented Apr 29, 2016

According this #551 (comment) Angular Cli is compiling all the files.

Currently i have this setup

sass/mixins/_pulls.sass (the mixin himself)

@mixin pull-left
    float: left !important

@mixin pull-right 
    float: right !important

sass/mixins/index.sass (the file that load all the mixins)

//this is the main entry point for all Mixins files
@import "_pulls"

sass/utils/pulls.sass (the file that call the pull mixin)

.pull-left
    @include pull-left()

.pull-right
    @include pull-right()

sass/utils/index.sass (the file that load all utils files)

//this is the main entry point for all utils files
@import '_pulls'

sass/_variables.sass (the variable file)
sass/index.sass ( the file that load the files together)

//this is the main entry point for all default sass files
@import "variables"
@import "mixins/index"
@import "utils/index"

Now the point is that Angular cli is compiling every file and not respect the import rule and create one file for it.

so the compiler is triggering an error on the file that call the mixin utils/_pulls.sass because the mixin file himself is compiled and not loaded into the _pull.sass file.

Is there something that i missed or does the sass compiler does not handle those structure. And how i can make this structure work.

@reinos
Copy link
Author

reinos commented Apr 29, 2016

Update:
To let the import works, you have to include the needed mixin files into your file where you`re working in.

For example:
in the mixin folder i have _pull.sass with a mixin. When i need this mixin, i have to import this file in the file where i need it.

Because the sass (perhaps also less) compiles every file, the mixin is not in the scope. Therefore you need to load it per file when needed. Also for the variable folder.

@nareshbhatia
Copy link

I don't understand why #551 is closed. This and #551 are real issues with Sass support. _*.sass or _*.scss files should never be compiled by the Sass compiler directly - they should be excluded from compiles. These files are only meant to be imported by main sass files (files without a leading underscore).

Also I have a related question - what is the correct place to specify sassOptions (see ember-cli-sass for example). Should I modify angular-cli-build.js or angular-cli.json? How?

TIA.

@steveblue
Copy link

@reinos including mixins in the same file is not an acceptable solution IMHO, we need the ability to leverage SASS imports from other files. The compiler should respect files prepended with underscore and not compile them. That is the nature of SASS. I am also disappointed I cannot easily use PostCSS in the CLI, which handles vendor prefixing in the most elegant way possible.

@reinos
Copy link
Author

reinos commented May 9, 2016

@steveblue agree, but thats for now my solution...

@hpop
Copy link

hpop commented May 15, 2016

Just for reference: Even the SASS docs are saying that files with an underscore will not be compiled: http://sass-lang.com/guide#topic-4

killface added a commit to killface/angular-cli that referenced this issue May 16, 2016
…in file patterns

Allow passing in cacheInclude and cacheExclude as options to the SASSPlugin, primarily so that filenames beginning with an underscore can be ignored during SASS/SCSS compilation, which is generally accepted to be a standard in SASS (http://sass-lang.com/guide#topic-4). These config values can be set in angular-cli-build.js thus:
    "sassCompiler": {
        "cacheExclude": [/\/_[^\/]+$/]
    }

This arguably closes issue angular#558 (angular#558)
@steveblue
Copy link

Sass in the public folder does not compile to css in the dist folder, the files are just being straight copied and now these .scss in the dist. This issue has been reported by others. These minor bugs are prohibiting us from adopting the cli at the moment.

@filipesilva filipesilva added type: bug/fix effort1: easy (hours) P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Jun 4, 2016
@Madd
Copy link
Contributor

Madd commented Jun 18, 2016

I stumbled across this problem myself. So my suggestion is to add an option and a regex in angular-cli/lib/broccoli/angular-broccoli-sass.js:

constructor(inputNodes, options) {
  super(inputNodes, {});

  options = options || {};
  Plugin.call(this, inputNodes, {
    cacheInclude: options.cacheInclude || [/\.scss$/, /\.sass$/],
    cacheExclude: options.cacheExclude || undefined,
    compilePartials: options.compilePartials || true
  });
  this.options = options;
}

build() {
  this.listFiles().forEach(fileName => {
    // We skip compiling partials if options.compilePartials is false
    if (this.options.compilePartials ||
      !new RegExp(/^_+.*.s[ac]ss$/i).test(path.basename(fileName))) {

      // Normalize is necessary for changing `\`s into `/`s on windows.
      this.compile(path.normalize(fileName),
                   path.normalize(this.inputPaths[0]),
                   path.normalize(this.outputPath));
    }
  });
}

This way the partials are not compiled into separate .css files and does not require you to import variables and mixins in every file. And the best thing is that they are watched, unlike the cacheExclude option, so you can enjoy the live reload while building cool stuff. :)

@filipesilva
Copy link
Contributor

@Madd would you be interested in making a PR that adds this functionality?

@Madd
Copy link
Contributor

Madd commented Jun 19, 2016

@filipesilva I can do that. Will go through CONTRIBUTING.md and make a PR as soon as I can.

@filipesilva
Copy link
Contributor

Basically make the proposed change, add a test on e2e_workflow.ts that confirms this, and use as commit message something like fix(sass): don't compile partials.

I think that's all that matters. If you have trouble running the tests in your machine, Travis will run them anyway.

@Madd
Copy link
Contributor

Madd commented Jun 20, 2016

Tests passed (edit: locally) and PR submitted #1150.

@lexusburn
Copy link

I think Less compiling need the same functionality (excluding of partials while compiling).

@filipesilva
Copy link
Contributor

Fixed via #1150

@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 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
effort1: easy (hours) P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent type: bug/fix
Projects
None yet
Development

No branches or pull requests

7 participants