Skip to content

Commit

Permalink
chore(lint): Split linting and formatting (#11246)
Browse files Browse the repository at this point in the history
This change splits out prettier from eslint. 

This does mean we don't have CI to enforce formatting since we only
currently run linting which is now separate. I intend to follow up and
add prettier to CI which should be straightforward.

**Changes**
1. Remove prettier eslint plugin(s)
2. Added `reportUnusedDisableDirectives` to prevent stray disable
directives (tidied them up here too)
3. Added `no-extra-semi` for the moment (it's deprecated and I expect
it'll go away anyway with my further refactoring)
4. Added a lot to the prettier ignore (again mostly just for the moment,
I don't want a pr with meaningful changes flooded with misc prettier
reformatting)
5. Switched the prettier config file to the json format and added
plugins: "prettier-plugin curly", "prettier-plugin-sh",
"prettier-plugin-packagejson"
6. Updated the vscode recommended extensions to include prettier
7. Updated the vscode settings to lint and format on save
  • Loading branch information
Josh-Walker-GM authored Aug 14, 2024
1 parent 74e942e commit 780dc7f
Show file tree
Hide file tree
Showing 27 changed files with 212 additions and 47 deletions.
9 changes: 4 additions & 5 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ module.exports = {
extends: [
'eslint:recommended',
'plugin:react/recommended',
'plugin:prettier/recommended',
'plugin:jest-dom/recommended',
],
parser: '@babel/eslint-parser',
Expand All @@ -24,6 +23,8 @@ module.exports = {
configFile: findBabelConfig(),
},
},
// Prevents unused eslint-disable comments
reportUnusedDisableDirectives: true,
settings: {
react: {
version: 'detect',
Expand All @@ -47,7 +48,6 @@ module.exports = {
],
plugins: [
'unused-imports',
'prettier',
'@babel',
'import',
'jsx-a11y',
Expand All @@ -60,8 +60,8 @@ module.exports = {
curly: 'error',
'unused-imports/no-unused-imports': 'error',
'@redwoodjs/process-env-computed': 'error',
'prettier/prettier': 'warn',
'no-console': 'off',
'no-extra-semi': 'off',
'prefer-object-spread': 'warn',
'prefer-spread': 'warn',
'no-unused-expressions': [
Expand Down Expand Up @@ -122,7 +122,6 @@ module.exports = {
//
// Uses https://github.com/isaacs/minimatch under the hood
// See https://github.com/isaacs/node-glob#glob-primer for syntax
// eslint-disable-next-line prettier/prettier
pattern: 'src/*/**/*.?(sdl.){js,ts}',
patternOptions: {
nobrace: true,
Expand Down Expand Up @@ -167,7 +166,7 @@ module.exports = {
{
files: ['*.ts', '*.tsx'],
parser: '@typescript-eslint/parser',
extends: ['plugin:@typescript-eslint/recommended', 'prettier'],
extends: ['plugin:@typescript-eslint/recommended'],
rules: {
// TODO: look into enabling these eventually
'@typescript-eslint/no-empty-function': 'off',
Expand Down
22 changes: 19 additions & 3 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,22 @@
# Ignore built code
**/dist

# Do not format Markdown files to allow easier documentation contribution
*.md
# Ignore fixture projects
__fixtures__

/.nx/cache
# Ignore the docs (docusaurus) project
docs/

# Ignore the .nx directory
/.nx

# TODO(jgmw): Is this too broad?
# Ignore test fixtures
**/__testfixtures__
**/__tests__/fixtures

# TODO(jgmw): Re-enable these in managable chunks
tasks
.github
.changesets
packages/create-redwood-app/tests/e2e_prompts*
12 changes: 12 additions & 0 deletions .prettierrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"$schema": "http://json.schemastore.org/prettierrc",
"bracketSpacing": true,
"tabWidth": 2,
"semi": false,
"singleQuote": true,
"plugins": [
"prettier-plugin-curly",
"prettier-plugin-sh",
"prettier-plugin-packagejson"
]
}
5 changes: 3 additions & 2 deletions .vscode/extensions.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
{
"recommendations": [
"EditorConfig.EditorConfig",
"dbaeumer.vscode-eslint",
"mgmcdermott.vscode-language-babel"
"dbaeumer.vscode-eslint", // eslint
"mgmcdermott.vscode-language-babel", // babel
"esbenp.prettier-vscode", // prettier
],
"unwantedRecommendations": [
"johnpapa.vscode-peacock" // not used, still tries to apply it's settings
Expand Down
6 changes: 3 additions & 3 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
"editor.tabSize": 2,
"files.trimTrailingWhitespace": true,
"editor.formatOnSave": false,
"editor.defaultFormatter": "esbenp.prettier-vscode",
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.fixAll.eslint": "explicit"
},
Expand All @@ -20,7 +21,6 @@
"files.trimTrailingWhitespace": false
},
"typescript.tsdk": "node_modules/typescript/lib",
"peacock.color": "#b85833",
"cSpell.words": [
"attw",
"autoplay",
Expand All @@ -47,4 +47,4 @@
"tailwindcss",
"waku"
]
}
}
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
"check:package": "nx run-many -t check:package --output-style static",
"clean:prisma": "rimraf node_modules/.prisma/client && node node_modules/@prisma/client/scripts/postinstall.js",
"e2e": "node ./tasks/run-e2e",
"format": "prettier . --write",
"format:check": "prettier . --check",
"generate-dependency-graph": "node ./tasks/generateDependencyGraph.mjs",
"install:all": "concurrently -g -c auto -n install:fw \"yarn install\" npm:install:crwrsca",
"install:ci": "concurrently -g -c auto -n install:fw:ci \"yarn install --inline-builds\" npm:install:crwrsca:ci",
Expand Down Expand Up @@ -113,6 +115,10 @@
"npm-packlist": "8.0.2",
"nx": "19.5.7",
"ora": "8.0.1",
"prettier": "3.3.3",
"prettier-plugin-curly": "0.2.2",
"prettier-plugin-packagejson": "2.5.1",
"prettier-plugin-sh": "0.14.0",
"prompts": "2.4.2",
"rimraf": "6.0.1",
"tstyche": "2.1.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/babel-config/src/__tests__/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ describe('api', () => {
loglevel: string
}

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const [_, babelPluginModuleResolverConfig] = apiSideBabelPlugins.find(
(plugin) => plugin[0] === 'babel-plugin-module-resolver',
)! as [any, ModuleResolverConfig, any]
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/__tests__/studioHandler.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Have to use `var` here to avoid "Temporal Dead Zone" issues
// eslint-disable-next-line
var mockedRedwoodVersion = '0.0.0'

vi.mock('@redwoodjs/project-config', async (importOriginal) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/context/src/context.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable react-hooks/rules-of-hooks */

import { getAsyncStoreInstance } from './store'

Expand Down
2 changes: 1 addition & 1 deletion packages/context/src/global.api-auto-imports.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable no-redeclare, no-undef */
import type { GlobalContext } from './context'

declare global {
Expand Down
2 changes: 1 addition & 1 deletion packages/context/src/store.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable react-hooks/rules-of-hooks */

import { AsyncLocalStorage } from 'async_hooks'

Expand Down
2 changes: 1 addition & 1 deletion packages/cookie-jar/src/CookieJar.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
/* to keep the tests a little cleaner by using ! */

import { describe, expect, test } from 'vitest'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const hasRole = ({ roles }) => {
// in ./api/src/directives/requireAuth

// Roles are passed in by the requireAuth directive if you have auth setup
// eslint-disable-next-line no-unused-vars, @typescript-eslint/no-unused-vars
// eslint-disable-next-line no-unused-vars
export const requireAuth = ({ roles }) => {
return isAuthenticated()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const hasRole = ({ roles }) => {
// in ./api/src/directives/requireAuth

// Roles are passed in by the requireAuth directive if you have auth setup
// eslint-disable-next-line no-unused-vars, @typescript-eslint/no-unused-vars
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export const requireAuth = ({ roles }) => {
return isAuthenticated()
}
Expand Down
1 change: 0 additions & 1 deletion packages/eslint-config/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ module.exports = {
//
// Uses https://github.com/isaacs/minimatch under the hood
// See https://github.com/isaacs/node-glob#glob-primer for syntax
// eslint-disable-next-line prettier/prettier
pattern: 'src/*/**/*.?(sdl.){js,ts}',
patternOptions: {
nobrace: true,
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql-server/src/createGraphQLYoga.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable react-hooks/rules-of-hooks */
import { useDisableIntrospection } from '@envelop/disable-introspection'
import { useFilterAllowedOperations } from '@envelop/filter-operation-type'
import type { GraphQLSchema } from 'graphql'
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql-server/src/global.api-auto-imports.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable no-redeclare, no-undef */
import type _gql from 'graphql-tag'

declare global {
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql-server/src/globalContext.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable react-hooks/rules-of-hooks */

import { getAsyncStoreInstance } from './globalContextStore'

Expand Down
2 changes: 1 addition & 1 deletion packages/graphql-server/src/globalContextStore.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable react-hooks/rules-of-hooks */

import { AsyncLocalStorage } from 'async_hooks'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export const useRedwoodOpenTelemetry = (
onExecuteDone({ result }) {
if (isAsyncIterable(result)) {
executionSpan.end()
// eslint-disable-next-line no-console
console.warn(
`Plugin "RedwoodOpenTelemetry" encountered an AsyncIterator which is not supported yet, so tracing data is not available for the operation.`,
)
Expand Down
2 changes: 1 addition & 1 deletion packages/internal/src/jsx.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */
import type { types } from '@babel/core'
import traverse from '@babel/traverse'

Expand Down
2 changes: 1 addition & 1 deletion packages/structure/src/ide.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ export abstract class BaseNode {
}

@lazy() get closestContainingUri(): string | undefined {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const { uri } = this as any
if (uri) {
return uri
Expand Down
4 changes: 2 additions & 2 deletions packages/structure/src/x/vscode.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */

// vscode is a compile-time only dependency
// we only use it in type declarations
Expand Down Expand Up @@ -228,7 +228,7 @@ export class RemoteTreeDataProviderImpl implements RemoteTreeDataProvider {
onDidChangeTreeData(listener: (e: string | undefined) => void) {
this.lazyInit()
this.listeners.push(listener)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return null as any // TODO: disposable (we're not using it for now)
}

Expand Down
2 changes: 1 addition & 1 deletion packages/testing/src/web/mockRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ const mockGraphQL = (
)
}

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
registerHandler(graphql[type](operation, resolver))
return data
}
Expand Down
2 changes: 1 addition & 1 deletion packages/web/src/global.web-auto-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ declare global {

type GraphQLOperationVariables = Record<string, any>

/* eslint-disable no-unused-vars, @typescript-eslint/no-unused-vars */
/* eslint-disable @typescript-eslint/no-unused-vars */
// Overridable graphQL hook return types
interface QueryOperationResult<
TData = any,
Expand Down
8 changes: 0 additions & 8 deletions prettier.config.js

This file was deleted.

Loading

0 comments on commit 780dc7f

Please sign in to comment.