From 6be665f88708c99d77ad9622fe0a35e3ecb03f6e Mon Sep 17 00:00:00 2001 From: Sasha <64744993+r1tsuu@users.noreply.github.com> Date: Mon, 14 Oct 2024 02:10:47 +0300 Subject: [PATCH] chore(eslint): payload-logger proper usage works with template literals (#8660) Improves this https://github.com/payloadcms/payload/pull/8578 to work with template literals as well, This is invalid: ```ts this.payload.logger.error('Failed to create database', err) ``` But this is valid: ```ts this.payload.logger.error(`Failed to create database ${dbName}`, err) ``` This PR fixes that image --- packages/db-postgres/src/connect.ts | 5 ++++- packages/db-vercel-postgres/src/connect.ts | 5 ++++- packages/drizzle/src/migrateReset.ts | 2 +- packages/drizzle/src/postgres/createDatabase.ts | 6 +++--- .../customRules/proper-payload-logger-usage.js | 12 ++++++------ .../tests/proper-payload-logger-usage.js | 13 +++++++++++++ .../payload/src/database/migrations/migrateReset.ts | 2 +- 7 files changed, 32 insertions(+), 13 deletions(-) diff --git a/packages/db-postgres/src/connect.ts b/packages/db-postgres/src/connect.ts index a88a62dfa..37c077c0f 100644 --- a/packages/db-postgres/src/connect.ts +++ b/packages/db-postgres/src/connect.ts @@ -88,7 +88,10 @@ export const connect: Connect = async function connect( return } } else { - this.payload.logger.error(`Error: cannot connect to Postgres. Details: ${err.message}`, err) + this.payload.logger.error({ + err, + msg: `Error: cannot connect to Postgres. Details: ${err.message}`, + }) } if (typeof this.rejectInitializing === 'function') { diff --git a/packages/db-vercel-postgres/src/connect.ts b/packages/db-vercel-postgres/src/connect.ts index 30644bc90..1da8676f9 100644 --- a/packages/db-vercel-postgres/src/connect.ts +++ b/packages/db-vercel-postgres/src/connect.ts @@ -51,7 +51,10 @@ export const connect: Connect = async function connect( return } } else { - this.payload.logger.error(`Error: cannot connect to Postgres. Details: ${err.message}`, err) + this.payload.logger.error({ + err, + msg: `Error: cannot connect to Postgres. Details: ${err.message}`, + }) } if (typeof this.rejectInitializing === 'function') { diff --git a/packages/drizzle/src/migrateReset.ts b/packages/drizzle/src/migrateReset.ts index bf5588204..b61114069 100644 --- a/packages/drizzle/src/migrateReset.ts +++ b/packages/drizzle/src/migrateReset.ts @@ -84,7 +84,7 @@ export async function migrateReset(this: DrizzleAdapter): Promise { }, }) } catch (err: unknown) { - payload.logger.error({ error: err, msg: 'Error deleting dev migration' }) + payload.logger.error({ err, msg: 'Error deleting dev migration' }) } } } diff --git a/packages/drizzle/src/postgres/createDatabase.ts b/packages/drizzle/src/postgres/createDatabase.ts index a19dc7179..f7524d2db 100644 --- a/packages/drizzle/src/postgres/createDatabase.ts +++ b/packages/drizzle/src/postgres/createDatabase.ts @@ -98,10 +98,10 @@ export const createDatabase = async function (this: BasePostgresAdapter, args: A return true } catch (err) { - this.payload.logger.error( - `Error: failed to create database ${dbName}. Details: ${err.message}`, + this.payload.logger.error({ err, - ) + msg: `Error: failed to create database ${dbName}. Details: ${err.message}`, + }) return false } finally { diff --git a/packages/eslint-plugin/customRules/proper-payload-logger-usage.js b/packages/eslint-plugin/customRules/proper-payload-logger-usage.js index 01817687f..84b007621 100644 --- a/packages/eslint-plugin/customRules/proper-payload-logger-usage.js +++ b/packages/eslint-plugin/customRules/proper-payload-logger-usage.js @@ -34,11 +34,11 @@ export const rule = { if (isPayloadLoggerError(callee)) { const args = node.arguments - // Case 1: Single string is passed as the argument + // Case 1: Single string / templated string is passed as the argument if ( args.length === 1 && - args[0].type === 'Literal' && - typeof args[0].value === 'string' + ((args[0].type === 'Literal' && typeof args[0].value === 'string') || + args[0].type === 'TemplateLiteral') ) { return // Valid: single string argument } @@ -67,11 +67,11 @@ export const rule = { return // Valid object, checked for 'err'/'error' keys } - // Case 3: Improper usage (string + error or additional err/error) + // Case 3: Improper usage (string / templated string + error or additional err/error) if ( args.length > 1 && - args[0].type === 'Literal' && - typeof args[0].value === 'string' && + ((args[0].type === 'Literal' && typeof args[0].value === 'string') || + args[0].type === 'TemplateLiteral') && args[1].type === 'Identifier' && (args[1].name === 'err' || args[1].name === 'error') ) { diff --git a/packages/eslint-plugin/tests/proper-payload-logger-usage.js b/packages/eslint-plugin/tests/proper-payload-logger-usage.js index 1eccd5123..050b21a96 100644 --- a/packages/eslint-plugin/tests/proper-payload-logger-usage.js +++ b/packages/eslint-plugin/tests/proper-payload-logger-usage.js @@ -14,6 +14,10 @@ ruleTester.run('no-improper-payload-logger-error', rule, { { code: "payload.logger.error('Some error message')", }, + // Valid: payload.logger.error with a templated string + { + code: 'payload.logger.error(`Some error message`)', + }, // Valid: *.payload.logger.error with object { code: "this.payload.logger.error({ msg: 'another message', err })", @@ -33,6 +37,15 @@ ruleTester.run('no-improper-payload-logger-error', rule, { }, ], }, + // Invalid: payload.logger.error with both templated string and error + { + code: 'payload.logger.error(`Some error message`, err)', + errors: [ + { + messageId: 'improperUsage', + }, + ], + }, // Invalid: *.payload.logger.error with both string and error { code: "this.payload.logger.error('Some error message', error)", diff --git a/packages/payload/src/database/migrations/migrateReset.ts b/packages/payload/src/database/migrations/migrateReset.ts index 070a35716..7cd1b0b0a 100644 --- a/packages/payload/src/database/migrations/migrateReset.ts +++ b/packages/payload/src/database/migrations/migrateReset.ts @@ -62,6 +62,6 @@ export async function migrateReset(this: BaseDatabaseAdapter): Promise { }, }) } catch (err: unknown) { - payload.logger.error({ error: err, msg: 'Error deleting dev migration' }) + payload.logger.error({ err, msg: 'Error deleting dev migration' }) } }