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

Switch to biome from eslint #672

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .eslintignore

This file was deleted.

81 changes: 0 additions & 81 deletions .eslintrc.json

This file was deleted.

136 changes: 136 additions & 0 deletions biome.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
{
"$schema": "https://biomejs.dev/schemas/1.8.3/schema.json",
Copy link
Member

Choose a reason for hiding this comment

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

This is nice to have, but every time dependatbot updates the biome version I feel someone needs to come and manually update this.

From the docs we can also do:

{
  "$schema": "./node_modules/@biomejs/biome/configuration_schema.json"
}

Copy link
Author

Choose a reason for hiding this comment

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

yes, not sure if we want too

"organizeImports": { "enabled": true },
"linter": {
"enabled": true,
"rules": {
"recommended": false,
"a11y": {
"noAccessKey": "error",
"noAriaUnsupportedElements": "error",
"noAutofocus": "error",
"noBlankTarget": "error",
"noDistractingElements": "error",
"noHeaderScope": "error",
"noInteractiveElementToNoninteractiveRole": "error",
"noNoninteractiveElementToInteractiveRole": "error",
"noNoninteractiveTabindex": "error",
"noPositiveTabindex": "error",
"noRedundantAlt": "error",
"noRedundantRoles": "error",
"useAltText": "error",
"useAnchorContent": "error",
"useAriaActivedescendantWithTabindex": "error",
"useAriaPropsForRole": "error",
"useHeadingContent": "error",
"useHtmlLang": "error",
"useIframeTitle": "error",
"useKeyWithClickEvents": "error",
"useKeyWithMouseEvents": "error",
"useMediaCaption": "error",
"useValidAnchor": "off",
"useValidAriaProps": "error",
"useValidAriaRole": "error",
"useValidAriaValues": "error"
},
"complexity": {
"noExtraBooleanCast": "error",
"noMultipleSpacesInRegularExpressionLiterals": "error",
"noUselessCatch": "error",
"noUselessConstructor": "error",
"noUselessFragments": "error",
"noUselessLoneBlockStatements": "error",
"noUselessRename": "error",
"noUselessTernary": "error",
"noVoid": "error",
"noWith": "error",
"useLiteralKeys": "error",
"useRegexLiterals": "error"
},
"correctness": {
"noChildrenProp": "error",
"noConstAssign": "error",
"noConstantCondition": "error",
"noEmptyCharacterClassInRegex": "error",
"noEmptyPattern": "error",
"noGlobalObjectCalls": "error",
"noInnerDeclarations": "error",
"noInvalidConstructorSuper": "error",
"noInvalidUseBeforeDeclaration": "error",
"noNewSymbol": "error",
"noNonoctalDecimalEscape": "error",
"noPrecisionLoss": "error",
"noSelfAssign": "error",
"noSetterReturn": "error",
"noSwitchDeclarations": "error",
"noUndeclaredVariables": "error",
"noUnreachable": "error",
"noUnreachableSuper": "error",
"noUnsafeFinally": "error",
"noUnsafeOptionalChaining": "error",
"noUnusedLabels": "error",
"noUnusedVariables": "error",
"useArrayLiterals": "error",
"useExhaustiveDependencies": "error",
"useHookAtTopLevel": "error",
"useIsNan": "error",
"useJsxKeyInIterable": "error",
"useValidForDirection": "error",
"useYield": "error"
},
"security": {
"noDangerouslySetInnerHtmlWithChildren": "error",
"noGlobalEval": "error"
},
"style": {
"noCommaOperator": "error",
"noImplicitBoolean": "error",
"noVar": "error",
"useBlockStatements": "off",
"useConst": "error",
"useFragmentSyntax": "error",
"useSingleVarDeclarator": "error"
},
"suspicious": {
"noAssignInExpressions": "error",
"noAsyncPromiseExecutor": "error",
"noCatchAssign": "error",
"noClassAssign": "error",
"noCommentText": "error",
"noCompareNegZero": "error",
"noConfusingLabels": "error",
"noConsoleLog": "off",
"noControlCharactersInRegex": "error",
"noDebugger": "error",
"noDoubleEquals": "error",
"noDuplicateCase": "error",
"noDuplicateClassMembers": "error",
"noDuplicateJsxProps": "error",
"noDuplicateObjectKeys": "error",
"noDuplicateParameters": "error",
"noEmptyBlockStatements": "error",
"noFallthroughSwitchClause": "error",
"noFunctionAssign": "error",
"noGlobalAssign": "error",
"noImportAssign": "error",
"noMisleadingCharacterClass": "error",
"noPrototypeBuiltins": "error",
"noRedeclare": "error",
"noSelfCompare": "error",
"noShadowRestrictedNames": "error",
"noUnsafeNegation": "error",
"useDefaultSwitchClauseLast": "error",
"useGetterReturn": "error",
"useValidTypeof": "error"
}
},
"ignore": ["node_modules/*", "pkg/lib/*", "bots/*"]
Copy link
Member

@jelly jelly Jul 23, 2024

Choose a reason for hiding this comment

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

pkg/lib contains shared component which should be linted I'd say but this was already in ignore. So leave it be, was more a surprise for me :)

Copy link
Member

Choose a reason for hiding this comment

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

We can also enable git integration:

{
  "vcs": {
    "enabled": true,
    "clientKind": "git"
  }
}

That way we don't need to do any "ignore" in linter or formatter.

Copy link
Member

Choose a reason for hiding this comment

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

node_modules are a submodule, wouldn't that be checked then?

Copy link
Author

Choose a reason for hiding this comment

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

we would any ways. bots has a bunch of broken symlinks that cause biome to spit out internal fs errors

Copy link
Member

Choose a reason for hiding this comment

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

node_modules are a submodule, wouldn't that be checked then?

I believe Biome ignores node_modules even without git integration. It's for the other files/folders in that are ignored in git.

we would any ways. bots has a bunch of broken symlinks that cause biome to spit out internal fs errors

Interesting. I think /bots is already in .gitignore.

},
"javascript": {
"globals": ["require", "module", "document", "navigator", "window"]
Copy link
Member

Choose a reason for hiding this comment

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

Does biome not support setting the env, we developer for a browser so the linter should know about it like ESLint does?

Copy link
Member

Choose a reason for hiding this comment

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

  ✖ Use Number.parseInt instead of the equivalent global.

    30 │             method,
    31 │             address: u.hostname,
  > 32 │             port: u.port ? parseInt(u.port) : 80,
       │                            ^^^^^^^^
    33 │             path: u.pathname + u.hash,
    34 │         };

This is maybe something we should fix...

Copy link
Member

Choose a reason for hiding this comment

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

I think this is unnecessary.

Copy link
Author

Choose a reason for hiding this comment

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

this was imported from the eslint config

},
"overrides": [{ "include": ["**/*.ts", "**/*.tsx"] }],
Copy link
Member

Choose a reason for hiding this comment

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

which files does it match?

Copy link
Author

Choose a reason for hiding this comment

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

this was also inherited from the eslint config

"formatter": {
Copy link
Member

Choose a reason for hiding this comment

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

Please use spaces:

{
  "formatter": {
    "indentStyle": "space"
  }
}

And possibly add "indentWidth": 4 as well(?) I personally prefer 2 spaces, but cockpit has been traditionally using 4 spaces for some reason. I'm neutral on this.

"ignore": ["node_modules/*", "pkg/lib/*", "bots/*"]
Copy link
Member

Choose a reason for hiding this comment

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

We won't need this with the git integration.

Copy link
Author

Choose a reason for hiding this comment

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

we would see my comment on bots

}
}
120 changes: 56 additions & 64 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,66 +1,58 @@
{
"name": "files",
"description": "Scaffolding for a cockpit module",
"type": "module",
"main": "index.js",
"repository": "git@github.com:cockpit-project/cockpit-files.git",
"author": "",
"license": "LGPL-2.1",
"scripts": {
"watch": "./build.js -w",
"build": "./build.js",
"eslint": "eslint --ext .js --ext .jsx --ext .ts --ext .tsx src/",
"eslint:fix": "eslint --fix --ext .js --ext .jsx --ext .ts --ext .tsx src/",
"stylelint": "stylelint src/*{.css,scss}",
"stylelint:fix": "stylelint --fix src/*{.css,scss}"
},
"devDependencies": {
"@types/react": "18.3.3",
"@types/react-dom": "18.3.0",
"@typescript-eslint/eslint-plugin": "7.16.1",
"argparse": "2.0.1",
"chrome-remote-interface": "0.33.0",
"esbuild": "0.23.0",
"esbuild-plugin-copy": "2.1.1",
"esbuild-plugin-replace": "1.4.0",
"esbuild-sass-plugin": "3.3.1",
"esbuild-wasm": "0.23.0",
"eslint": "8.57.0",
"eslint-config-standard": "17.1.0",
"eslint-config-standard-jsx": "11.0.0",
"eslint-config-standard-react": "13.0.0",
"eslint-plugin-import": "2.29.1",
"eslint-plugin-jsx-a11y": "6.9.0",
"eslint-plugin-node": "11.1.0",
"eslint-plugin-promise": "6.5.1",
"eslint-plugin-react": "7.35.0",
"eslint-plugin-react-hooks": "4.6.2",
"gettext-parser": "8.0.0",
"htmlparser": "1.7.7",
"jed": "1.1.1",
"language-map": "1.5.0",
"mime-db": "1.52.0",
"sass": "1.77.6",
"sizzle": "2.3.10",
"stylelint": "16.3.1",
"stylelint-config-recommended-scss": "14.0.0",
"stylelint-config-standard": "36.0.1",
"stylelint-config-standard-scss": "13.1.0",
"stylelint-formatter-pretty": "4.0.0",
"stylelint-use-logical-spec": "^5.0.1",
"tsx": "4.16.0",
"typescript": "5.5.3"
},
"dependencies": {
"@patternfly/patternfly": "5.3.1",
"@patternfly/react-core": "5.3.4",
"@patternfly/react-icons": "5.3.2",
"@patternfly/react-styles": "5.3.1",
"@patternfly/react-table": "5.3.4",
"@patternfly/react-tokens": "5.3.1",
"dequal": "2.0.3",
"react": "18.3.1",
"react-dom": "18.3.1",
"throttle-debounce": "5.0.0"
}
"name": "files",
"description": "Scaffolding for a cockpit module",
"type": "module",
"main": "index.js",
"repository": "git@github.com:cockpit-project/cockpit-files.git",
"author": "",
"license": "LGPL-2.1",
"scripts": {
"watch": "./build.js -w",
"build": "./build.js",
"lint": "biome lint src/*",
"lint:fix": "biome lint src/* --fix",
"format": "biome format src/* --fix",
"stylelint": "stylelint src/*{.css,scss}",
"stylelint:fix": "stylelint --fix src/*{.css,scss}"
},
"devDependencies": {
"@biomejs/biome": "1.8.3",
"@types/react": "18.3.3",
"@types/react-dom": "18.3.0",
"@typescript-eslint/eslint-plugin": "7.16.1",
"argparse": "2.0.1",
"chrome-remote-interface": "0.33.0",
"esbuild": "0.23.0",
"esbuild-plugin-copy": "2.1.1",
"esbuild-plugin-replace": "1.4.0",
"esbuild-sass-plugin": "3.3.1",
"esbuild-wasm": "0.23.0",
"gettext-parser": "8.0.0",
"htmlparser": "1.7.7",
"jed": "1.1.1",
"language-map": "1.5.0",
"mime-db": "1.52.0",
"sass": "1.77.6",
"sizzle": "2.3.10",
"stylelint": "16.3.1",
"stylelint-config-recommended-scss": "14.0.0",
"stylelint-config-standard": "36.0.1",
"stylelint-config-standard-scss": "13.1.0",
"stylelint-formatter-pretty": "4.0.0",
"stylelint-use-logical-spec": "^5.0.1",
"tsx": "4.16.0",
"typescript": "5.5.3"
},
"dependencies": {
"@patternfly/patternfly": "5.3.1",
"@patternfly/react-core": "5.3.4",
"@patternfly/react-icons": "5.3.2",
"@patternfly/react-styles": "5.3.1",
"@patternfly/react-table": "5.3.4",
"@patternfly/react-tokens": "5.3.1",
"dequal": "2.0.3",
"react": "18.3.1",
"react-dom": "18.3.1",
"throttle-debounce": "5.0.0"
}
}