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

Mixins: Inconsistent errors when not passing required params #79

Open
dwighthouse opened this issue May 7, 2019 · 0 comments
Open

Mixins: Inconsistent errors when not passing required params #79

dwighthouse opened this issue May 7, 2019 · 0 comments

Comments

@dwighthouse
Copy link

dwighthouse commented May 7, 2019

Related to: #76

There are two ways to avoid passing a required param to a mixin: empty params (@include some-mixin();) and no params (@include some-mixin;). However, they behave differently.

Empty Params Example:

/* Mixin with no defaults */
@mixin heading-text($font-size) {
    font-size: $font-size;
}

/* Using empty param form results in empty value instead of error */
h1 {
    @include heading-text();
}

Output:

/* Mixin with no defaults */
/* Using empty param form results in empty value instead of error */
h1 {
    font-size: ; /* <= ERROR */
}

This is invalid CSS.


No Params Example:

/* Mixin with no defaults */
@mixin heading-text($font-size) {
    font-size: $font-size;
}

/* Using no param form results in PreCSS throwing */
h1 {
    @include heading-text;
}

Output (throws):

Message:
    precss: /.../main.css:3:5: Could not resolve the variable "$font-size" within "$font-size"

  1 | /* Mixin with no defaults */
  2 | @mixin heading-text($font-size) {
> 3 |     font-size: $font-size;
    |     ^
  4 | }
  5 | 

This is confusing because the issue is at the site of the @include, not at the @mixin. ("What do you mean you can't resolve the variable?! It's RIGHT THERE!")


I do not know what the correct answer should be, only that it should be consistent. I see two reasonable options:

  1. Output the CSS without the result of the mixin's failed operation (and maybe a warning about what happened).
    h1 {}
  2. Throw with a more specific error, like:
    Message:
        precss: /.../main.css:8:5: Included mixin without required variable "$font-size"
    
      6 | /* Using empty param form results in empty value instead of error */
      7 | h1 {
    > 8 |     @include heading-text;
        |     ^
      9 | }
    
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

No branches or pull requests

1 participant