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 compiler does not throw an error when expected #10188

Open
Elijbet opened this issue Aug 29, 2024 · 3 comments
Open

Sass compiler does not throw an error when expected #10188

Elijbet opened this issue Aug 29, 2024 · 3 comments
Assignees
Labels
3 - installed Issues that have been merged to master branch and are ready for final confirmation. estimate - 2 Small fix or update, may require updates to tests. p - low Issue is non core or affecting less that 10% of people using the library tooling Issues relating to build system fixes or improvements.

Comments

@Elijbet
Copy link
Contributor

Elijbet commented Aug 29, 2024

Priority impact

p - low

Summary

Sass names refactor in the loader revealed that Sass compiler did not throw an error when it was referencing a function that does not exist.

Desired Outcome

Sass compiler should throw an error when referencing a function that does not exist.

Resources

No response

@Elijbet Elijbet added tooling Issues relating to build system fixes or improvements. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Aug 29, 2024
@jcfranco jcfranco self-assigned this Aug 30, 2024
@jcfranco jcfranco added 2 - in development Issues that are actively being worked on. estimate - 2 Small fix or update, may require updates to tests. p - low Issue is non core or affecting less that 10% of people using the library and removed 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Aug 30, 2024
@jcfranco
Copy link
Member

This is a known limitation for unknown Sass functions:

⚠️ Heads up!

Because any unknown function will be compiled to CSS, it’s easy to miss when you typo a function name. Consider running a CSS linter on your stylesheet’s output to be notified when this happens!

Stylelint's function-no-unknown rule can help with this. Local testing looks good, but we need to specify which functions to ignore, as it doesn't do this automatically. Currently, we only have 2 custom sass functions, so we can add coverage for this easily.

Keeping track of custom functions will be a manual process for now, but we may be able to automate this.

jcfranco added a commit that referenced this issue Sep 4, 2024
**Related Issue:** #10188 

## Summary

The Sass compiler does not error on unknown functions, but we can use
Stylelint's
[`function-no-unknown`](https://github.com/stylelint-scss/stylelint-scss/tree/master/src/rules/function-no-unknown)
to ensure function names are properly referenced.
jcfranco added a commit that referenced this issue Sep 16, 2024
…namic options (#10311)

**Related Issue:** N/A

## Summary

This change will give us more flexibility for sharing rule options and
for more dynamic rule updates (e.g., #10188).
jcfranco added a commit that referenced this issue Sep 16, 2024
…int (#10313)

**Related Issue:** #10188 

## Summary

This adds a script that tracks all custom Sass functions and updates the
metadata for Stylelint’s
[`function-no-unknown`](https://github.com/stylelint-scss/stylelint-scss/tree/master/src/rules/function-no-unknown)
rule, addressing workflow improvements as discussed in
#10188 (comment).

Our pre-commit hook will run this script if there is a staged Sass file
and will add any changes (if any).

**Note**: this depends on
#10311.

---------

Co-authored-by: Ben Elan <no-reply@benelan.dev>
@jcfranco jcfranco added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Sep 16, 2024
@jcfranco
Copy link
Member

Keeping track of custom functions will be a manual process for now, but we may be able to automate this.

This step has ben automated via a pre-commit hook.

@jcfranco jcfranco added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Sep 17, 2024
@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned jcfranco Sep 17, 2024
Copy link
Contributor

Installed and assigned for verification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - installed Issues that have been merged to master branch and are ready for final confirmation. estimate - 2 Small fix or update, may require updates to tests. p - low Issue is non core or affecting less that 10% of people using the library tooling Issues relating to build system fixes or improvements.
Projects
None yet
Development

No branches or pull requests

4 participants