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 stylelint plugin & no-color-variables-in-rgba rule #21269

Merged
merged 7 commits into from
Jan 13, 2023

Conversation

josephkmh
Copy link
Contributor

What

Recently we merged a PR that makes using our SASS color variables incompatible with the rgba() function. Unfortunately there is nothing blocking a developer from using this syntax, it just silently fails. This PR introduces a stylelint plugin & rule, no-color-variables-in-rgba, which checks our source (S)CSS files for incompatible usage.

For example, if you write some CSS like this:

@use "scss/colors";

.myClass {
  background-color: rgba(colors.$blue, 50%);

Our pre-commit hook, which runs stylelint, will fail with this error message:

image

Considerations

The rule checks using a regex /rgba\([^)]*colors\.\$/ that looks specifically for rgba(colors.$ to detect if the rule is violated. If we change the name of our scss/colors import for whatever reason, this could break.

@josephkmh josephkmh added the area/frontend Related to the Airbyte webapp label Jan 11, 2023
Comment on lines +4 to +6
const rules = {
"no-color-variables-in-rgba": require("./no-color-variables-in-rgba"),
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we want to add more stylelint plugins later, we can put them in here

@josephkmh
Copy link
Contributor Author

Tested that this works on CI during the frontend build step as well: https://github.com/airbytehq/airbyte/actions/runs/3895729403/jobs/6651434662#step:8:1853

Copy link
Collaborator

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Tested locally, code LGTM. Just left a comment about potential change in wording, but non blocking for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants