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

Add scss/function-no-unknown rule #591

Merged
merged 11 commits into from Mar 15, 2022
Merged

Add scss/function-no-unknown rule #591

merged 11 commits into from Mar 15, 2022

Conversation

ybiquitous
Copy link
Contributor

export const ALL_FUNCTIONS = Object.freeze([
...GLOBAL_FUNCTIONS,
...COLOR_FUNCTIONS
]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[ask] Is this a smart way to get all Sass functions?

Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure if there is a better way than to manually list them like you have done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will keep as-is if there is no objection.

code: "a { color: adjust-color(#6b717f, $red: 15); }"
},
{
code: "a { color: color.adjust(#6b717f, $red: 15); }"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[ask] This case fails now with the following message:

Unexpected unknown function "color.adjust" (scss/function-no-unknown)

If supporting a {module}.{func} notation, is there a smart way? I'm concerned about the following case:

@use "sass:color" as c;

a { color: c.adjust(#6b717f, $red: 15); }

Ref: https://sass-lang.com/documentation/at-rules/use

Copy link
Collaborator

Choose a reason for hiding this comment

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

One option is simply not to support @use "sass:color" as c if it gets too difficult to implement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, as the first implementation, not supporting as sounds good. 👍🏼

@ybiquitous
Copy link
Contributor Author

I'm not familiar with Sass, so I have some questions about the implementation.

@ybiquitous
Copy link
Contributor Author

stylelint/stylelint#5916 (maybe stylelint@14.5.1) should fix the following error:

TypeError [ERR_INVALID_URL_SCHEME]: The URL must be of scheme file

@ybiquitous ybiquitous marked this pull request as ready for review February 14, 2022 17:56
@ybiquitous
Copy link
Contributor Author

I've bumped stylelint to 14.5.1. Now the CI is all green!

@kristerkari
Copy link
Collaborator

@ybiquitous Does this rule now depend on stylelint version 14.5.1 or newer or is there backwards compatibility?

@ybiquitous
Copy link
Contributor Author

@kristerkari

Does this rule now depend on stylelint version 14.5.1 or newer

Yes. The new rule requires stylelint@14.5.1 or newer.

is there backwards compatibility?

Yes. Unless users don't enable the new rule, their stylelint analysis should not change.

Copy link
Collaborator

@kristerkari kristerkari left a comment

Choose a reason for hiding this comment

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

Looks good now! 👍

@kristerkari kristerkari merged commit 03af755 into stylelint-scss:master Mar 15, 2022
@ybiquitous ybiquitous deleted the function-no-unknown branch March 15, 2022 06:31
@kristerkari
Copy link
Collaborator

Thanks @ybiquitous ! Released now in https://github.com/stylelint-scss/stylelint-scss/releases/tag/v4.2.0

@ybiquitous
Copy link
Contributor Author

@kristerkari Thank you! 😊

@chalkygames123
Copy link
Contributor

@kristerkari Hi, any plan to include this rule to stylelint-config-recommended-scss?

@ybiquitous
Copy link
Contributor Author

@chalkygames123 Here is a reason not to include this rule to stylelint-config-recommended-scss:
stylelint-scss/stylelint-config-recommended-scss#92 (comment)

@chalkygames123
Copy link
Contributor

I see. Thanks for letting me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

New stylelint function-no-unknown rule not working well with SCSS Feature request: scss/function-no-unknown
3 participants