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.

Sign up for GitHub

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

Update LPLs eslint config to 3.0.1 #26

Merged
merged 14 commits into from
Apr 11, 2023
Merged

Update LPLs eslint config to 3.0.1 #26

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
31d209f
fix workspace naming, and update deps
oroth8 Apr 10, 2023
b7a3e9b
renamed babelrc to modern naming
oroth8 Apr 10, 2023
c50c15c
added default url resolver to devolopment webpack config
oroth8 Apr 10, 2023
d011b51
fixed linting errors
oroth8 Apr 10, 2023
c1bf3e8
add eslint to backend and add eslint prettier config to backend
oroth8 Apr 10, 2023
2b0e8a0
update be eslint config and run prettier
oroth8 Apr 10, 2023
5261140
revert fe babel cs version
oroth8 Apr 10, 2023
ef8ce74
Merge branch 'main' into update-eslint-config
oroth8 Apr 10, 2023
9d21a66
update yarn lock
oroth8 Apr 10, 2023
9b01c9e
remove stale ignores
oroth8 Apr 10, 2023
abee28f
update gitignore and remove node_modules
oroth8 Apr 10, 2023
f4e4e86
update lint-staged script to include cjs files
oroth8 Apr 10, 2023
ca66164
rerun prettier using .prettierrc config
oroth8 Apr 10, 2023
ecc8493
Merge branch 'main' into update-eslint-config
oroth8 Apr 11, 2023
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: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ notifications:
addons:
apt:
packages:
- libgconf-2-4
- libgconf-2-4
Copy link
Collaborator

Choose a reason for hiding this comment

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

Inner monologue: Any chance we want to add linting to travis CI? It could avoid the manual Nit: * comments in PRs, and let CI do the job for us. Just a thought

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@vanderhoop I was thinking of creating a github action and add linting there in a separate PR. Reason being, Im not sure if we will be able to use travisCI with veritext so it might be better to use github's

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, now that I'm thinking about it, we should hold off until fully in Veritext's GitHub and they've given us a Jenkins file we can add scripts to.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, now that I'm thinking about it, we should hold off until fully in Veritext's GitHub and they've given us a Jenkins file we can add scripts to.

Reason being the work would be redundant, and we're gonna have to switch to their repo relatively soon

before_script:
# ensure application config template is copied, as the server requires these env-var configs to be available
- cp -n fe/config/application.template.yml fe/config/application.yml
4 changes: 4 additions & 0 deletions be/.eslintignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
tmp
node_modules
swagger.yml
.eslintrc.json
25 changes: 25 additions & 0 deletions be/.eslintrc.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
module.exports = {
root: true,
env: {
node: true,
browser: true,
es6: true,
},
extends: [
"prettier",
"eslint:recommended",
Copy link
Collaborator

Choose a reason for hiding this comment

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

@oroth8 Small thing, but I think your editor's default settings might be setting you up with 4 spaces per tab. The source files are mostly 2-spaces, so we might want to that.

Normally, I wouldn't comment on lint stuff, but early in the project, it makes sense to get our editor configs' aligned so that future PR diffss don't end up with a bunch of whitespace noise.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@vanderhoop LPL uses a 2-tab width across all projects:

Screenshot 2023-04-10 at 1 41 20 PM

this is what the .prettierrc file at the root of our project uses

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think tabWidth: 2 means 2 spaces per tab? See the be/src/controllers/users.controller.js file and other source files below for cases where the indentation is 2 spaces.

I suspect the reason this particular file isn't taking on the prettier defaults is that the path pattern isn't included in launchpad's prettier config. (Will check)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suspect the reason this particular file isn't taking on the prettier defaults is that the path pattern isn't included in launchpad's prettier config. (Will check)

I checked, and it's not the .prettierrc or .prettierignore file that's causing this file to go unformatted.

I think it's this line in the package.json, which doesn't include *.cjs files:

"be/**/*.{js,json}": [
      "prettier --write",
      "eslint --fix --max-warnings=0"
    ]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think my vscode plugin workspace settings were overriding the .prettierrc file at the root. Anyways I added cjs to the lint-staged script and also reran prettier using their --config flag so I knew for a fact it was using .prettierrc

"plugin:import/errors",
"plugin:import/warnings"
],
rules: {
"no-console": "warn",
"no-unreachable": "warn",
"no-debugger": "warn",
"no-unused-vars": "warn",
"no-throw-literal": "error"
},
parserOptions: {
ecmaVersion: 12,
sourceType: "module",
},
}
3 changes: 2 additions & 1 deletion be/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# Keep environment variables out of version control
.env
/src/logs/**/*
/src/logs/**/*
test.json
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we'll want to remove this from the diff.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done thx

1 change: 1 addition & 0 deletions be/node_modules/.bin/eslint-config-prettier
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is a woopsy in the diff, as nothing in node_modules either at the root or in lower directories should be in version control. I think deleting this file from version control and adding the following two lines to the .gitignore file would address this:

# (rest of .gitignore)
be/node_modules/
fe/node_modules/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

also ran git rm -r on:

node_modules
be/node_modules
fe/node_modules

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions be/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
"bcrypt": "^5.1.0",
"cookie-parser": "^1.4.6",
"cors": "^2.8.5",
"fe": "1.0.0",
"prisma": "^4.12.0",
"swagger-jsdoc": "^6.2.8",
"swagger-ui-express": "^4.6.2",
"winston": "^3.8.2",
"winston-daily-rotate-file": "^4.7.1"
},
"devDependencies": {
"eslint-config-prettier": "^8.8.0",
"morgan": "^1.10.0",
"nodemon": "^2.0.22"
},
Expand Down
14 changes: 8 additions & 6 deletions be/src/controllers/users.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { UserService } from '../services/user.service.js'

export class UsersController {
user = new UserService()
static user() {
return new UserService()
}

getUsers = async (req, res, next) => {
async getUsers(req, res, next) {
try {
const findAllUsersData = await this.user.findAllUser()

Expand All @@ -13,7 +15,7 @@ export class UsersController {
}
}

getUserById = async (req, res, next) => {
async getUserById(req, res, next) {
try {
const userId = Number(req.params.id)
const findOneUserData = await this.user.findUserById(userId)
Expand All @@ -24,7 +26,7 @@ export class UsersController {
}
}

createUser = async (req, res, next) => {
async createUser(req, res, next) {
try {
const userData = req.body
const createUserData = await this.user.createUser(userData)
Expand All @@ -35,7 +37,7 @@ export class UsersController {
}
}

updateUser = async (req, res, next) => {
async updateUser(req, res, next) {
try {
const userId = Number(req.params.id)
const userData = req.body
Expand All @@ -47,7 +49,7 @@ export class UsersController {
}
}

deleteUser = async (req, res, next) => {
async deleteUser(req, res, next) {
try {
const userId = Number(req.params.id)
const deleteUserData = await this.user.deleteUser(userId)
Expand Down
12 changes: 9 additions & 3 deletions be/src/routes/user.routes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,15 @@ import { Router } from 'express'
import { UsersController } from '../controllers/users.controller.js'

export class UserRoute {
path = '/users'
router = Router()
user = new UsersController()
static path() {
return '/users'
}
get router() {
return Router()
}
get user() {
return new UsersController()
}

constructor() {
this.initializeRoutes()
Expand Down
4 changes: 3 additions & 1 deletion be/src/services/user.service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { hash } from 'bcrypt'
import { HttpException } from '../exceptions/HttpException.js'

export class UserService {
user = prisma.user
static user() {
return prisma.user
}

async findAllUser() {
const allUser = await this.user.findMany()
Expand Down
86 changes: 43 additions & 43 deletions be/swagger.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
tags:
- name: users
description: users API
- name: users
description: users API

paths:
# [GET] users
# [GET] users
/users:
get:
tags:
- users
- users
summary: Find All Users
responses:
200:
description: 'OK'
500:
description: 'Server Error'

# [POST] users
# [POST] users
post:
tags:
- users
- users
summary: Add User
parameters:
- name: body
in: body
description: user Data
required: true
schema:
$ref: '#/definitions/users'
- name: body
in: body
description: user Data
required: true
schema:
$ref: '#/definitions/users'
responses:
201:
description: 'Created'
Expand All @@ -37,18 +37,18 @@ paths:
500:
description: 'Server Error'

# [GET] users/id
# [GET] users/id
/users/{id}:
get:
tags:
- users
- users
summary: Find User By Id
parameters:
- name: id
in: path
description: User Id
required: true
type: integer
- name: id
in: path
description: User Id
required: true
type: integer
responses:
200:
description: 'OK'
Expand All @@ -57,23 +57,23 @@ paths:
500:
description: 'Server Error'

# [PUT] users/id
# [PUT] users/id
put:
tags:
- users
- users
summary: Update User By Id
parameters:
- name: id
in: path
description: user Id
required: true
type: integer
- name: body
in: body
description: user Data
required: true
schema:
$ref: '#/definitions/users'
- name: id
in: path
description: user Id
required: true
type: integer
- name: body
in: body
description: user Data
required: true
schema:
$ref: '#/definitions/users'
responses:
200:
description: 'OK'
Expand All @@ -84,17 +84,17 @@ paths:
500:
description: 'Server Error'

# [DELETE] users/id
# [DELETE] users/id
delete:
tags:
- users
- users
summary: Delete User By Id
parameters:
- name: id
in: path
description: user Id
required: true
type: integer
- name: id
in: path
description: user Id
required: true
type: integer
responses:
200:
description: 'OK'
Expand All @@ -108,8 +108,8 @@ definitions:
users:
type: object
required:
- email
- password
- email
- password
properties:
email:
type: string
Expand All @@ -119,5 +119,5 @@ definitions:
description: user Password

schemes:
- https
- http
- https
- http
14 changes: 12 additions & 2 deletions fe/.eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
const path = require('path')

module.exports = {
extends: ['@launchpadlab/eslint-config/react-rails'],
root: true,
extends: ['@launchpadlab/eslint-config/react'],
parser: '@babel/eslint-parser',
parserOptions: {
sourceType: 'module',
babelOptions: {
configFile: path.join(__dirname, 'babel.config.json'),
},
},
settings: {
'import/resolver': {
webpack: {
config: __dirname + '/config/webpack.config.development.js',
},
},
},
}
}
0 fe/.babelrc → fe/babel.config.json
View file
Open in desktop
File renamed without changes.
3 changes: 2 additions & 1 deletion fe/config/webpack.config.development.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ module.exports = {
{
loader: 'babel-loader',
options: {
cacheDirectory: true
cacheDirectory: true,
vanderhoop marked this conversation as resolved.
Show resolved Hide resolved
}
},
],
Expand Down Expand Up @@ -154,6 +154,7 @@ module.exports = {
// https://stackoverflow.com/questions/64557638/how-to-polyfill-node-core-modules-in-webpack-5
// for possible solutions, perhaps using https://github.com/Richienb/node-polyfill-webpack-plugin.
fallback: {
url: require.resolve('url/'),
fs: false,
net: false,
tls: false,
Expand Down
3 changes: 2 additions & 1 deletion fe/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,12 @@
"sass": "^1.56.2",
"sass-loader": "^13.2.0",
"style-loader": "^3.3.1",
"url": "^0.11.0",
"webpack": "^5.71.0",
"webpack-cli": "^5.0.1"
},
"devDependencies": {
"@launchpadlab/eslint-config": "^2.7.0",
"@launchpadlab/eslint-config": "^3.0.1",
"@launchpadlab/lp-subsection-generator": "^7.2.1",
"chalk": "^5.2.0",
"cli-prompt": "^0.6.0",
Expand Down
Loading