feat: create ESLint plugin for Griffel #99

layershifter · 2022-04-21T14:19:05Z

This PR adds @griffel/eslint-plugin with a single ESLint rule to enforce usage of CSS longhands.

Fixes #76.

Rule Details

Griffel does not support usage of CSS shorthands, microsoft/griffel#30.
This is enforced by typings, but there are cases when they don't work as expected microsoft/griffel#78.

Examples of incorrect code for this rule:

import { makeStyles } from '@griffel/react';

export const useStyles = makeStyles({
  root: {
    background: 'red',
    padding: '10px 20px',
  },
});

Examples of correct code for this rule:

import { makeStyles } from '@griffel/react';

export const useStyles = makeStyles({
  root: {
    backgroundColor: 'red',
    ...shorthands.padding('10px', '20px'),
  },
});

github-actions · 2022-04-21T14:20:52Z

📊 Bundle size report

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`core` makeStyles + mergeClasses (build time)	`1.8 kB` `861 B`
`core` makeStyles + mergeClasses (runtime)	`20.029 kB` `7.479 kB`
`react` makeStaticStyles (runtime)	`8.247 kB` `3.595 kB`
`react` makeStyles + mergeClasses (runtime)	`21.054 kB` `7.911 kB`
`react` makeStyles + mergeClasses (build time)	`2.811 kB` `1.275 kB`

_{🤖 This report was generated against 09eb2760eabcb6abc3d21b7333d0ec14f6c4b5d2}

layershifter · 2022-04-21T14:22:33Z

packages/eslint-plugin/src/rules/no-shorthands.ts

+// This collection is a map simply for faster access when checking if a CSS property is unsupported
+// @griffel/core has the same definition, but ESLint plugin should not depend on it
+const UNSUPPORTED_CSS_PROPERTIES: Record<keyof CSS.StandardShorthandProperties, true> = {


It's nonsense to have there dependency on @griffel/core, so unfortunately - copy-paste.

layershifter · 2022-04-21T14:26:15Z

packages/eslint-plugin/src/utils/helpers.ts

+export const isIdentifier: IsHelper<AST_NODE_TYPES.Identifier> = ASTUtils.isIdentifier;
+export const isObjectExpression: IsHelper<AST_NODE_TYPES.ObjectExpression> = ASTUtils.isNodeOfType(
+  AST_NODE_TYPES.ObjectExpression,
+);
+export const isProperty: IsHelper<AST_NODE_TYPES.Property> = ASTUtils.isNodeOfType(AST_NODE_TYPES.Property);


Had to type this manually otherwise TypeScript goes wild:

ling1726 · 2022-04-22T06:24:34Z

packages/core/src/index.ts

@@ -65,4 +65,5 @@ export type {
  GriffelRenderer,
 } from './types';

+// Private exports, are used by devtools


maybe prefix with @internal and then strip on build and use stripInternal in the tsconfig ?

you'll be able to use it with aliases in the monorepo but the types will be stripped on build

Didn't know about stripInternal, it's a good idea. I will try it in another PR.

ling1726 · 2022-04-22T06:32:02Z

packages/eslint-plugin/src/rules/no-shorthands.ts

+    if (isProperty(propertyNode)) {
+      if (isIdentifier(propertyNode.key) && !isRoot) {
+        if (Object.prototype.hasOwnProperty.call(UNSUPPORTED_CSS_PROPERTIES, propertyNode.key.name)) {


nit: these nested if blocks can just be reduced to a single one right ?

Yes, but IMO it's more readable

ling1726 · 2022-04-22T09:06:39Z

packages/eslint-plugin/src/utils/helpers.ts

@@ -0,0 +1,11 @@
+import { AST_NODE_TYPES, ASTUtils, TSESTree } from '@typescript-eslint/utils';


is it necessary to use typescript-eslint to create the lint rule ?

AFAIK there are no similar utils in ESLint itself. I was using eslint-plugin-testing-library as a reference.

layershifter commented Apr 21, 2022

View reviewed changes

layershifter force-pushed the feat/eslint-plugin branch from e9934fd to e539a67 Compare April 21, 2022 14:25

layershifter commented Apr 21, 2022

View reviewed changes

feat: create ESLint plugin for Griffel

c9c2fb7

layershifter force-pushed the feat/eslint-plugin branch from e539a67 to c9c2fb7 Compare April 21, 2022 14:39

layershifter marked this pull request as ready for review April 21, 2022 14:46

layershifter requested a review from a team as a code owner April 21, 2022 14:46

ling1726 reviewed Apr 22, 2022

View reviewed changes

add eslint to peerDependencies

576062a

ling1726 approved these changes Apr 22, 2022

View reviewed changes

layershifter merged commit 2d46633 into microsoft:main Apr 22, 2022

layershifter deleted the feat/eslint-plugin branch April 22, 2022 10:55

layershifter mentioned this pull request May 12, 2022

chore: enable ESLint plugin for Griffel microsoft/fluentui#22961

Merged

layershifter mentioned this pull request Jun 1, 2022

Styles that need shorthands are not being errored when they are inside of selectors #78

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create ESLint plugin for Griffel #99

feat: create ESLint plugin for Griffel #99

layershifter commented Apr 21, 2022 •

edited

Loading

github-actions bot commented Apr 21, 2022 •

edited

Loading

layershifter Apr 21, 2022

layershifter Apr 21, 2022

ling1726 Apr 22, 2022

layershifter Apr 22, 2022

ling1726 Apr 22, 2022

layershifter Apr 22, 2022

ling1726 Apr 22, 2022

layershifter Apr 22, 2022 •

edited

Loading

		@@ -0,0 +1,11 @@
		import { AST_NODE_TYPES, ASTUtils, TSESTree } from '@typescript-eslint/utils';

feat: create ESLint plugin for Griffel #99

feat: create ESLint plugin for Griffel #99

Conversation

layershifter commented Apr 21, 2022 • edited Loading

Rule Details

github-actions bot commented Apr 21, 2022 • edited Loading

📊 Bundle size report

layershifter Apr 21, 2022

Choose a reason for hiding this comment

layershifter Apr 21, 2022

Choose a reason for hiding this comment

ling1726 Apr 22, 2022

Choose a reason for hiding this comment

layershifter Apr 22, 2022

Choose a reason for hiding this comment

ling1726 Apr 22, 2022

Choose a reason for hiding this comment

layershifter Apr 22, 2022

Choose a reason for hiding this comment

ling1726 Apr 22, 2022

Choose a reason for hiding this comment

layershifter Apr 22, 2022 • edited Loading

Choose a reason for hiding this comment

layershifter commented Apr 21, 2022 •

edited

Loading

github-actions bot commented Apr 21, 2022 •

edited

Loading

layershifter Apr 22, 2022 •

edited

Loading