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

feat: disable prettier conflicts, change lint-staged to just lint-all #119

Merged
merged 10 commits into from
Jan 25, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
19 changes: 13 additions & 6 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ module.exports = {
'plugin:react/recommended',
'plugin:import/typescript',
'plugin:@typescript-eslint/recommended',

// override all prettier rules, see https://github.com/prettier/eslint-config-prettier
'prettier',
],
parser: '@typescript-eslint/parser',
parserOptions: {
Expand All @@ -22,11 +25,15 @@ module.exports = {
rules: {
// A few more opinions in addition to extensions

// Enforce new line at end of file
'eol-last': ['error', 'always'],

// As per React 17 changes! https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html
'react/react-in-jsx-scope': 'off',
'react/jsx-uses-react': 'off',

'linebreak-style': ['error', 'unix'],
'no-multiple-empty-lines': 'error',

'@typescript-eslint/no-require-imports': ['error'],

Expand All @@ -40,8 +47,8 @@ module.exports = {
// Style
quotes: ['error', 'single', { avoidEscape: true }],

// ensures clean diffs, see https://medium.com/@nikgraf/why-you-should-enforce-dangling-commas-for-multiline-statements-d034c98e36f8
'comma-dangle': ['error', 'always-multiline'],
// Covered by prettier
'comma-dangle': 'off',
Copy link
Member

@matthewcn56 matthewcn56 Jan 25, 2022

Choose a reason for hiding this comment

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

Specifically adding in the 'comma-dangle': off clashes with prettier when linting and does not enforce that dangling commas should be there.

When using eslint with prettier, I think it would be better to add in the prettier plugin to our .eslintrc.js as specified here, and adding it at the end of our array of plugins.

And for things that are already covered by prettier like comma-dangle, instead of specifying off, we can just not mention the comma-dangle property at all, as comma-dangle off doesn't enforce dangling commas when linting within our pre-commit hook or yarn lint-fix command, but commenting it out does enforce it (when used in conjunction with the prettier es-lint plugin).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kk sounds good, ty for the review!


// Require all imported dependencies are actually declared in package.json
'import/no-extraneous-dependencies': [
Expand Down Expand Up @@ -69,8 +76,8 @@ module.exports = {

// Cannot shadow names
// note you must disable the base rule as it can report incorrect errors
"no-shadow": "off",
"@typescript-eslint/no-shadow": ["error"],
'no-shadow': 'off',
'@typescript-eslint/no-shadow': ['error'],

// Required spacing in property declarations (copied from TSLint, defaults are good)
'key-spacing': ['error'],
Expand Down Expand Up @@ -100,8 +107,8 @@ module.exports = {
'@typescript-eslint/no-floating-promises': ['error'],

// Don't leave log statements littering the premises!
'no-console': ["error", { allow: ["info", "warn", "error"] }],
'no-console': ['error', { allow: ['info', 'warn', 'error'] }],

// Useless diff results
'no-trailing-spaces': ['error'],

Expand Down
12 changes: 6 additions & 6 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
version: 2
updates:
- package-ecosystem: npm
directory: "/"
schedule:
interval: monthly
time: "13:00"
open-pull-requests-limit: 10
- package-ecosystem: npm
directory: '/'
schedule:
interval: monthly
time: '13:00'
open-pull-requests-limit: 10
3 changes: 1 addition & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ on: [push]

jobs:
build:

runs-on: ubuntu-latest

strategy:
Expand All @@ -14,7 +13,7 @@ jobs:
steps:
- name: Checkout Git repo
uses: actions/checkout@v2

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v1
with:
Expand Down
3 changes: 2 additions & 1 deletion .prettierrc
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"singleQuote": true
"singleQuote": true,
"trailingComma": "es5"
}
6 changes: 5 additions & 1 deletion .stylelintrc.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
{
"extends": ["stylelint-config-recommended", "stylelint-config-sass-guidelines"],
"extends": [
"stylelint-config-recommended",
"stylelint-config-sass-guidelines",
"stylelint-config-prettier"
],
"rules": {
"color-named": null,
"selector-max-id": null,
Expand Down
16 changes: 6 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@
"scripts": {
"start": "webpack serve --open --config webpack.dev.js",
"build": "webpack --config webpack.prod.js",
"lint-staged": "lint-staged",
"lint": "eslint \"**/*.tsx\" \"**/*.ts\" && npx stylelint \"**/*.css\" \"**/*.scss\"",
"lint-fix": "eslint --fix \"**/*.tsx\" \"**/*.ts\" && npx stylelint --fix \"**/*.css\" \"**/*.scss\"",
"pre-commit": "lint-staged; exit 0",
"lint": "eslint \"**/*.tsx\" \"**/*.ts\" && npx stylelint \"**/*.css\" \"**/*.scss\" && prettier --check .",
"lint-fix": "eslint --fix \"**/*.tsx\" \"**/*.ts\" && npx stylelint --fix \"**/*.css\" \"**/*.scss\" && prettier --write .",
"pre-commit": "yarn lint-fix",
reginawang3495 marked this conversation as resolved.
Show resolved Hide resolved
"prepare": "husky install"
},
"dependencies": {
"@types/react": "^17.0.37",
"@types/react-dom": "^17.0.10",
"babelify": "^10.0.0",
"browserify": "^17.0.0",
"eslint-plugin-prettier": "^4.0.0",
"prettier": "^2.5.1",
"react": "^17.0.2",
"react-dom": "^17.0.2"
Expand All @@ -33,19 +33,20 @@
"core-js": "^3.20.2",
"css-loader": "^6.5.0",
"eslint": "^7.32.0",
"eslint-config-prettier": "^8.3.0",
"eslint-plugin-import": "^2.25.2",
"eslint-plugin-jsx-a11y": "^6.5.1",
"eslint-plugin-react": "^7.26.1",
"eslint-plugin-react-hooks": "^4.0.8",
"file-loader": "^6.2.0",
"html-webpack-plugin": "^5.5.0",
"husky": "^7.0.4",
"lint-staged": "^11.1.2",
"mini-css-extract-plugin": "^2.4.5",
"sass": "^1.44.0",
"sass-loader": "^12.4.0",
"style-loader": "^3.3.1",
"stylelint": "^13.13.1",
"stylelint-config-prettier": "^9.0.3",
"stylelint-config-sass-guidelines": "^8.0.0",
"stylelint-config-standard": "^22.0.0",
"ts-loader": "^9.2.4",
Expand All @@ -58,10 +59,5 @@
"webpack-merge": "^5.8.0",
"webpack-nano": "^1.1.1",
"webpack-pwa-manifest": "^4.3.0"
},
"lint-staged": {
"*.(ts|tsx)": "eslint --cache --fix",
"*.(css|scss|sass)": "stylelint --fix",
"*.{ts,tsx,css,scss,sass,md}": "prettier --write"
}
}
3 changes: 1 addition & 2 deletions src/components/shared/AppWrapper/Footer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ function Footer(): JSX.Element {
return (
<div id="footer">
<h3>
made with ❤️ by
{' '}
made with ❤️ by{' '}
<a
href="https://teachla.uclaacm.com"
target="_blank"
Expand Down
2 changes: 1 addition & 1 deletion src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ ReactDOM.render(
<StrictMode>
<App />
</StrictMode>,
document.getElementById('root'),
document.getElementById('root')
);
2 changes: 1 addition & 1 deletion src/styles/AppWrapper.scss
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ $header-height: 8vh;
background-color: white;
bottom: 0;
display: flex;
font-family: 'Westwood Sans','Helvetica Neue', 'Helvetica', sans-serif;
font-family: 'Westwood Sans', 'Helvetica Neue', 'Helvetica', sans-serif;
height: $header-height;
justify-content: center;
position: fixed;
Expand Down
2 changes: 1 addition & 1 deletion src/styles/global.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

@font-face {
font-family: 'Westwood Sans';
src: local('Westwood Sans'),
src: local('Westwood Sans'),
url('../assets/WestwoodSans-Regular.ttf') format('truetype');
}

Expand Down
5 changes: 1 addition & 4 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@
"resolveJsonModule": true,
"composite": true,
"incremental": true,
"typeRoots": [
"./node_modules/@types",
"./custom_typing"
]
"typeRoots": ["./node_modules/@types", "./custom_typing"]
},
"include": ["**/*.ts", "**/*.tsx"]
}
6 changes: 1 addition & 5 deletions webpack.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,7 @@ module.exports = merge(common, {
rules: [
{
test: /\.(sa|sc|c)ss$/,
use: [
MiniCssExtractPlugin.loader,
'css-loader',
'sass-loader',
],
use: [MiniCssExtractPlugin.loader, 'css-loader', 'sass-loader'],
},
{
test: /\.(png|svg|jpe?g|gif|mp3|ttf)$/i,
Expand Down
Loading