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

Internal: Add prettier and simplify eslint config #530

Merged
merged 13 commits into from
Dec 14, 2023
Merged
38 changes: 32 additions & 6 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -1,7 +1,33 @@
{
Copy link
Collaborator Author

@beckpaul beckpaul Dec 12, 2023

Choose a reason for hiding this comment

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

Add prettier as a plugin for eslint - it gets run now when yarn lint is ran and --fix will also fix prettier issues

Not sure why there were 4 different .eslintrc files in different places but i've removed them and compressed the rules into one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

it's the same with .gitignore, there are multiple ways to configure things, globally or per directory.
i think we can work with overrides for now

"root": true,
"extends": ["standard"],
"rules": {
"indent": ["error", 4]
}
}
"root": true,
"extends": ["standard", "plugin:prettier/recommended"],
"plugins": ["prettier"],
"rules": {
"prettier/prettier": "error"
},
"overrides": [
{
"files": ["src/frontend/**/*.js"],
"env": {
"browser": true,
"es2020": true
}
},
{
"files": ["src/backend/**/*.js"],
"env": {
"node": true,
"es2020": true
}
},
{
"files": ["tests/**/*.js"],
"env": {
"es2020": true,
"node": true,
"jest": true,
"jasmine": true
}
}
]
}
16 changes: 16 additions & 0 deletions .prettierrc.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# See https://prettier.io/docs/en/options. most are set to default
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Config for prettier - per comment: most are set to default. We can change this to JSON TOML or plain js if anyone has an aversion to yaml

trailingComma: "es5"
tabWidth: 4
semi: false
singleQuote: true
bracketSpacing: true
bracketSameLine: false
arrowParens: always
htmlWhitespaceSensitivity: strict
singleAttributePerLine: false
plugins:
- "@prettier/plugin-pug"
overrides:
- files: "*.pug"
options:
parser: "pug"
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,16 @@
"url-slug": "^4.0.1"
},
"devDependencies": {
"@prettier/plugin-pug": "^3.0.0",
"awesomplete": "^1.1.5",
"css-loader": "^6.8.1",
"dart-sass": "^1.25.0",
"eslint": "^8.54.0",
"eslint-config-prettier": "^9.1.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.

Add prettier, the plugin to socket it in, and the config that prevents conflicts with eslint.

Essentially we keep eslint as a way to find and prevent bugs, while prettier is soley focused on code styling per:
https://prettier.io/docs/en/comparison

"eslint-config-standard": "^17.1.0",
"eslint-plugin-import": "^2.29.0",
"eslint-plugin-n": "^16.3.1",
"eslint-plugin-prettier": "^5.0.1",
"eslint-plugin-promise": "^6.1.1",
"grunt": "1.6.1",
"grunt-concurrent": "3.0.0",
Expand All @@ -51,6 +54,7 @@
"load-grunt-tasks": "5.1.0",
"nock": "^13.3.8",
"octokit": "^3.1.2",
"prettier": "^3.1.1",
"style-loader": "^3.3.3",
"supertest": "^6.3.3",
"typescript": "^5.3.2",
beckpaul marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -64,7 +68,7 @@
},
"scripts": {
"test": "jest",
"lint": "eslint --ignore-path .gitignore src tests",
"lint:fix": "eslint --fix --ignore-path .gitignore src tests"
"lint": "eslint --ignore-path .gitignore src tests && prettier --check **/*.pug",
"lint:fix": "eslint --fix --ignore-path .gitignore src tests && prettier --write **/*.pug"
}
}
6 changes: 0 additions & 6 deletions src/backend/.eslintrc

This file was deleted.

95 changes: 61 additions & 34 deletions src/backend/AppKernel.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const { appContainer } = require('./dependency-injection/AppContainer')
const { RequestContainer } = require('./dependency-injection/RequestContainer')
const { RequestContainerCompilerPass } = require('./dependency-injection/RequestContainerCompilerPass')
const {
RequestContainerCompilerPass,
} = require('./dependency-injection/RequestContainerCompilerPass')
const { webpackAsset } = require('./middleware/webpackAsset')
const { bootPassport } = require('./security/bootPassport')
const express = require('./ExpressApp')
Expand All @@ -22,71 +24,82 @@
const dataRouter = require('./routes/views/dataRouter')

class AppKernel {
constructor (nodeEnv = 'production') {
constructor(nodeEnv = 'production') {
this.env = nodeEnv
this.config = appConfig
this.expressApp = null
this.appContainer = null
this.schedulers = []
}

async boot () {
async boot() {
await this.compileContainer(this.config)
this.bootstrapExpress()
return this
}

async compileContainer (config) {
async compileContainer(config) {
this.appContainer = appContainer(config)
await this.appContainer.compile()
}

bootstrapExpress () {
bootstrapExpress() {
this.expressApp = express()

this.expressApp.locals.clanInvitations = {}
this.expressApp.use((req, res, next) => {
res.locals.navLinks = []
res.locals.cNavLinks = []
res.locals.appGlobals = {
loggedInUser: null
loggedInUser: null,
}
next()
})

this.expressApp.set('views', 'src/backend/templates/views')
this.expressApp.set('view engine', 'pug')
this.expressApp.use(express.static('public', {
immutable: true,
maxAge: 4 * 60 * 60 * 1000 // 4 hours
}))
this.expressApp.use(
express.static('public', {
immutable: true,
maxAge: 4 * 60 * 60 * 1000, // 4 hours
})
)

this.expressApp.use('/dist', express.static('dist', {
immutable: true,
maxAge: 4 * 60 * 60 * 1000 // 4 hours, could be longer since we got cache-busting
}))
this.expressApp.use(
'/dist',
express.static('dist', {
immutable: true,
maxAge: 4 * 60 * 60 * 1000, // 4 hours, could be longer since we got cache-busting
})
)

this.expressApp.use(express.json())
this.expressApp.use(bodyParser.json())
this.expressApp.use(bodyParser.urlencoded({ extended: false }))
this.expressApp.use(webpackAsset(this.appContainer.getParameter('webpackManifestJS')))

this.expressApp.use(session({
resave: false,
saveUninitialized: true,
secret: appConfig.session.key,
store: new FileStore({
retries: 0,
ttl: appConfig.session.tokenLifespan,
secret: appConfig.session.key
this.expressApp.use(
webpackAsset(this.appContainer.getParameter('webpackManifestJS'))
)

this.expressApp.use(
session({
resave: false,
saveUninitialized: true,
secret: appConfig.session.key,
store: new FileStore({
retries: 0,
ttl: appConfig.session.tokenLifespan,
secret: appConfig.session.key,
}),
})
}))
)
bootPassport(this.expressApp, this.config)

this.expressApp.use(async (req, res, next) => {
req.appContainer = this.appContainer
req.requestContainer = RequestContainer(this.appContainer, req)
req.requestContainer.addCompilerPass(new RequestContainerCompilerPass(this.config, req))
req.requestContainer.addCompilerPass(
new RequestContainerCompilerPass(this.config, req)
)
await req.requestContainer.compile()

if (req.requestContainer.fafThrownException) {
Expand All @@ -100,7 +113,7 @@
this.expressApp.use(function (req, res, next) {
req.asyncFlash = async function () {
const result = req.flash(...arguments)
await new Promise(resolve => req.session.save(resolve))
await new Promise((resolve) => req.session.save(resolve))

Check warning on line 116 in src/backend/AppKernel.js

View check run for this annotation

Codecov / codecov/patch

src/backend/AppKernel.js#L116

Added line #L116 was not covered by tests

return result
}
Expand All @@ -114,19 +127,27 @@

this.expressApp.use(function (req, res, next) {
if (req.isAuthenticated()) {
res.locals.appGlobals.loggedInUser = req.requestContainer.get('UserService').getUser()
res.locals.appGlobals.loggedInUser = req.requestContainer
.get('UserService')
.getUser()
}
next()
})
}

startCronJobs () {
this.schedulers.push(leaderboardCacheCrawler(this.appContainer.get('LeaderboardService')))
this.schedulers.push(wordpressCacheCrawler(this.appContainer.get('WordpressService')))
this.schedulers.push(clanCacheCrawler(this.appContainer.get('ClanService')))
startCronJobs() {
this.schedulers.push(

Check warning on line 139 in src/backend/AppKernel.js

View check run for this annotation

Codecov / codecov/patch

src/backend/AppKernel.js#L138-L139

Added lines #L138 - L139 were not covered by tests
leaderboardCacheCrawler(this.appContainer.get('LeaderboardService'))
)
this.schedulers.push(

Check warning on line 142 in src/backend/AppKernel.js

View check run for this annotation

Codecov / codecov/patch

src/backend/AppKernel.js#L142

Added line #L142 was not covered by tests
wordpressCacheCrawler(this.appContainer.get('WordpressService'))
)
this.schedulers.push(

Check warning on line 145 in src/backend/AppKernel.js

View check run for this annotation

Codecov / codecov/patch

src/backend/AppKernel.js#L145

Added line #L145 was not covered by tests
clanCacheCrawler(this.appContainer.get('ClanService'))
)
}

loadControllers () {
loadControllers() {
this.expressApp.use('/', defaultRouter)
this.expressApp.use('/', authRouter)
this.expressApp.use('/', staticMarkdownRouter)
Expand All @@ -140,7 +161,13 @@
res.status(404).render('errors/404')
})
this.expressApp.use((err, req, res, next) => {
console.error('[error] Incoming request to"', req.originalUrl, '"failed with error "', err.toString(), '"')
console.error(

Check warning on line 164 in src/backend/AppKernel.js

View check run for this annotation

Codecov / codecov/patch

src/backend/AppKernel.js#L164

Added line #L164 was not covered by tests
'[error] Incoming request to"',
req.originalUrl,
'"failed with error "',
err.toString(),
'"'
)
console.error(err.stack)

if (res.headersSent) {
Expand Down
8 changes: 4 additions & 4 deletions src/backend/config/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,26 @@ const appConfig = {
host: process.env.HOST || 'http://localhost',
session: {
key: process.env.SESSION_SECRET_KEY || '12345',
tokenLifespan: process.env.TOKEN_LIFESPAN || 43200
tokenLifespan: process.env.TOKEN_LIFESPAN || 43200,
},
oauth: {
strategy: 'faforever',
clientId: process.env.OAUTH_CLIENT_ID || '12345',
clientSecret: process.env.OAUTH_CLIENT_SECRET || '12345',
url: oauthUrl,
publicUrl: process.env.OAUTH_PUBLIC_URL || oauthUrl,
callback: process.env.CALLBACK || 'callback'
callback: process.env.CALLBACK || 'callback',
},
m2mOauth: {
clientId: process.env.OAUTH_M2M_CLIENT_ID || 'faf-website-public',
clientSecret: process.env.OAUTH_M2M_CLIENT_SECRET || 'banana',
url: oauthUrl
url: oauthUrl,
},
apiUrl: process.env.API_URL || 'https://api.faforever.com',
wordpressUrl: process.env.WP_URL || 'https://direct.faforever.com',
extractorInterval: process.env.EXTRACTOR_INTERVAL || 5,
playerCountInterval: process.env.PLAYER_COUNT_INTERVAL || 15,
recaptchaKey: process.env.RECAPTCHA_SITE_KEY || 'test'
recaptchaKey: process.env.RECAPTCHA_SITE_KEY || 'test',
}

module.exports = appConfig
18 changes: 13 additions & 5 deletions src/backend/cron-jobs/clanCacheCrawler.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,17 @@

const warmupClans = async (clanService) => {
try {
await clanService.getAll(true)
await clanService

Check warning on line 13 in src/backend/cron-jobs/clanCacheCrawler.js

View check run for this annotation

Codecov / codecov/patch

src/backend/cron-jobs/clanCacheCrawler.js#L13

Added line #L13 was not covered by tests
.getAll(true)
.then(() => successHandler('clanService::getAll(global)'))
.catch((e) => errorHandler(e, 'clanService::getAll(global)'))
} catch (e) {
console.error('Error: clanCacheCrawler::warmupClans failed with "' + e.toString() + '"',
{ entrypoint: 'clanCacheCrawler.js' })
console.error(

Check warning on line 18 in src/backend/cron-jobs/clanCacheCrawler.js

View check run for this annotation

Codecov / codecov/patch

src/backend/cron-jobs/clanCacheCrawler.js#L18

Added line #L18 was not covered by tests
'Error: clanCacheCrawler::warmupClans failed with "' +
e.toString() +
'"',
{ entrypoint: 'clanCacheCrawler.js' }
)
console.error(e.stack)
}
}
Expand All @@ -27,8 +32,11 @@
module.exports = (clanService) => {
warmupClans(clanService).then(() => {})

const clansScheduler = new Scheduler('createClanCache', // Refresh cache every 59 minutes
() => warmupClans(clanService).then(() => {}), 60 * 59 * 1000)
const clansScheduler = new Scheduler(

Check warning on line 35 in src/backend/cron-jobs/clanCacheCrawler.js

View check run for this annotation

Codecov / codecov/patch

src/backend/cron-jobs/clanCacheCrawler.js#L35

Added line #L35 was not covered by tests
'createClanCache', // Refresh cache every 59 minutes
() => warmupClans(clanService).then(() => {}),

Check warning on line 37 in src/backend/cron-jobs/clanCacheCrawler.js

View check run for this annotation

Codecov / codecov/patch

src/backend/cron-jobs/clanCacheCrawler.js#L37

Added line #L37 was not covered by tests
60 * 59 * 1000
)
clansScheduler.start()

return clansScheduler
Expand Down
Loading