fix(eslint): pass #6 #1871
fix(eslint): pass #6 #1871
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| session({ | ||
| secret: process.env['NANGO_ADMIN_KEY'] || 'nango', | ||
| resave: false, | ||
| saveUninitialized: false, | ||
| store: sessionStore, | ||
| name: 'nango_session', | ||
| unset: 'destroy', | ||
| cookie: { maxAge: 30 * 24 * 60 * 60 * 1000, secure: false }, | ||
| rolling: true | ||
| }) |
Check warning
Code scanning / CodeQL
Clear text transmission of sensitive cookie Medium
There was a problem hiding this comment.
any reason we have set secure: false?
| @@ -3,116 +3,114 @@ export interface WSErr { | |||
| message: string; | |||
| } | |||
|
|
|||
| export class WSErrBuilder { | |||
There was a problem hiding this comment.
Everything could be replaced by actual class [..] extend Error
| @@ -73,7 +74,7 @@ class CompileService { | |||
| continue; | |||
| } | |||
| const result = compiler.compile(fs.readFileSync(filePath, 'utf8'), filePath); | |||
| const jsFilePath = filePath.replace(/\/[^\/]*$/, `/dist/${path.basename(filePath.replace('.ts', '.js'))}`); | |||
| const jsFilePath = filePath.replace(/\/[^/]*$/, `/dist/${path.basename(filePath.replace('.ts', '.js'))}`); | |||
There was a problem hiding this comment.
how was it working before? or maybe it wasn't?
There was a problem hiding this comment.
I think it's just ignored by the regex compiler hence why it's unnecessary, cf: https://eslint.org/docs/latest/rules/no-useless-escape
| @@ -39,7 +39,7 @@ server.post( | |||
| msg: z.string() | |||
| }) | |||
| }), | |||
| persistController.saveActivityLog | |||
| persistController.saveActivityLog.bind(persistController) | |||
There was a problem hiding this comment.
why is bind necessary here?
There was a problem hiding this comment.
it's not necessary to make it work but because it's in a class this reference can be mistakenly overrode.
overall our usage of class is probably unnecessary
There was a problem hiding this comment.
what would you recommend instead of using a class?
There was a problem hiding this comment.
straight up function, if we had state it would be debatable but it's not the case and I would argue keeping state between api calls is dangerous
There was a problem hiding this comment.
agreed. In this specific case the class in only used as a namespace mechanism
|
|
||
| export function extractQueryParams(data: string | Buffer | undefined) { | ||
| return new URLSearchParams(typeof data === 'string' ? data : data?.toString()); | ||
| } |
There was a problem hiding this comment.
why not add Object.fromEntries as part of the logic here?
Describe your changes