Skip to content

Commit

Permalink
🪟 🔧 Add stylelint plugin & no-color-variables-in-rgba rule (#21269)
Browse files Browse the repository at this point in the history
  • Loading branch information
josephkmh authored and jbfbell committed Jan 13, 2023
1 parent 8b65da1 commit 1441218
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 25 deletions.
6 changes: 4 additions & 2 deletions airbyte-webapp/.stylelintrc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
"scss/dollar-variable-pattern": null,
"scss/percent-placeholder-pattern": null,
"value-keyword-case": null,
"color-no-hex": true
"color-no-hex": true,
"airbyte/no-color-variables-in-rgba": true
},
"ignoreFiles": ["**/build/**", "**/dist/**"]
"ignoreFiles": ["**/build/**", "**/dist/**"],
"plugins": ["./packages/stylelint-plugin/index.js"]
}
70 changes: 47 additions & 23 deletions airbyte-webapp/STYLEGUIDE.md
Original file line number Diff line number Diff line change
@@ -1,48 +1,72 @@
# Frontend Style Guide

This serves as a living document regarding conventions we have agreed upon as a frontend team. In general, the aim of these decisions and discussions is to both (a) increase the readability and consistency of our code and (b) decrease day to day decision-making so we can spend more time writing better code.
This serves as a living document regarding conventions we have agreed upon as a frontend team. In general, the aim of these decisions and discussions is to both (a) increase the readability and consistency of our code and (b) decrease day to day decision-making so we can spend more time writing better code.

## General Code Style and Formatting

* Where possible, we rely on automated systems to maintain consistency in code style
* We use eslint, Prettier, and VSCode settings to automate these choices. The configuration files for these are checked into our repository, so no individual setup should be required beyond ensuring your VSCode settings include:
- Where possible, we rely on automated systems to maintain consistency in code style
- We use eslint, Prettier, and VSCode settings to automate these choices. The configuration files for these are checked into our repository, so no individual setup should be required beyond ensuring your VSCode settings include:

```
"editor.codeActionsOnSave": {
"source.fixAll.eslint": true,
}
```

* Don’t use single-character names. Using meaningful name for function parameters is a way of making the code self-documented and we always should do it. Example:
* .filter(([key, value]) => isDefined(value.default) ✅
* .filter(([k, v]) => isDefined(v.default) ❌

- Don’t use single-character names. Using meaningful name for function parameters is a way of making the code self-documented and we always should do it. Example:
- .filter(([key, value]) => isDefined(value.default) ✅
- .filter(([k, v]) => isDefined(v.default) ❌

## Exporting

* Export at declaration, not at the bottom. For example:
* export const myVar ✅
* const myVar; export { myVar }; ❌

- Export at declaration, not at the bottom. For example:
- export const myVar ✅
- const myVar; export { myVar }; ❌

## Component Props
* Use explicit, verbose naming
* ie: `interface ConnectionFormProps` not `interface iProps`

- Use explicit, verbose naming
- ie: `interface ConnectionFormProps` not `interface iProps`

## Testing

* Test files should be store alongside the files/features they are testing
* Use the prop `data-testid` instead of `data-id`

- Test files should be store alongside the files/features they are testing
- Use the prop `data-testid` instead of `data-id`

## Types

* For component props, prefer type unions over enums:
* `type SomeType = “some” | “type”;`
* `enum SomeEnum = { SOME: “some”, TYPE: “type” };`
* Exceptions may include:
* Generated using enums from the API
* When the value on an enum is cleaner than the string
* In this case use `const enum` instead
- For component props, prefer type unions over enums:
- `type SomeType = “some” | “type”;`
- `enum SomeEnum = { SOME: “some”, TYPE: “type” };`
- Exceptions may include:
- Generated using enums from the API
- When the value on an enum is cleaner than the string
- In this case use `const enum` instead

## Styling

### Color variables cannot be used inside of rgba() functions

Our SCSS color variables compile to `rgb(X, Y, Z)`, which is an invalid value in the CSS `rgba()` function. A custom stylelint rule, `airbyte/no-color-variables-in-rgba`, enforces this rule.

❌ Incorrect

```scss
@use "scss/colors";

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

✅ Correct - define a color variable with transparency and use it directly

```scss
@use "scss/colors";

.myClass {
background-color: colors.$blue-transparent;
}
```

> Historical context: previously there was some usage of color variables inside `rgba()` functions. In these cases, _SASS_ was actually compiling this into CSS for us, because it knew to convert `rgba(rgb(255, 0, 0), 50%)` into `rgb(255, 0, 0, 50%)`. Now that we use CSS Custom Properties for colors, SASS cannot know the value of the color variable at build time. So it outputs `rgba(var(--blue), 50%)` which will not work as expected.
12 changes: 12 additions & 0 deletions airbyte-webapp/packages/stylelint-plugin/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// eslint-disable-next-line @typescript-eslint/no-var-requires
const stylelint = require("stylelint");

const rules = {
"no-color-variables-in-rgba": require("./no-color-variables-in-rgba"),
};

const rulesPlugins = Object.keys(rules).map((ruleName) =>
stylelint.createPlugin(`airbyte/${ruleName}`, rules[ruleName])
);

module.exports = rulesPlugins;
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// eslint-disable-next-line @typescript-eslint/no-var-requires
const stylelint = require("stylelint");
const { ruleMessages } = stylelint.utils;
const ruleName = "airbyte/no-color-variables-in-rgba";
const messages = ruleMessages(ruleName, {
variableFoundInRgba: () =>
`A color variable can't be used within an rgba() function. Explanation: ${LINK_TO_STYLEGUIDE}`,
});
const LINK_TO_STYLEGUIDE =
"https://github.com/airbytehq/airbyte/blob/master/airbyte-webapp/STYLEGUIDE.md#color-variables-cannot-be-used-inside-of-rgba-functions";

module.exports.ruleName = ruleName;
module.exports.messages = messages;

/**
* This stylelint rule checks if a color variable is used inside of rgba(), which we do not currently support.
* There are no options passed to the rule, as long as it is enabled in .stylelintrc it will be enforced.
*/
module.exports = (enabled) => {
return function lint(postcssRoot, postcssResult) {
if (!enabled) {
return () => null;
}
postcssRoot.walkDecls((decl) => {
// Check each value to see if it contains a string like "rgba(colors.$" or "rgba( ... colors.$"
const hasVariableInRgba = /rgba\([^)]*colors\.\$/.test(decl.value);
if (hasVariableInRgba) {
stylelint.utils.report({
ruleName,
result: postcssResult,
message: messages.variableFoundInRgba(),
node: decl,
word: "blue",
});
}
});
};
};

0 comments on commit 1441218

Please sign in to comment.