style: apply eslint prettier formatting - #301
Conversation
ca8b2b0 to
f020bcc
Compare
tony8713
left a comment
There was a problem hiding this comment.
Reviewed for behavior preservation. The vast majority of this diff is genuinely no-op: prettier line-wrapping, import-order sorting, import type -> import, catch (e) -> catch (err) / bare catch {}, plus the dep/config bumps carried over from #300. RPC routes, SQL/drizzle queries, sign/verify logic, email templates, status codes and payload shapes are all unchanged.
One line is not pure formatting and does change behavior (flagged inline): src/preview/utils.ts. Prettier would never rewrite [x] || y into a ternary, so this is a hand edit. It's a benign bugfix, but it means the "formatting-only" claim isn't strictly true. Fine to keep as-is or split out.
CI: no checks are configured/reported on the branch, so lint/typecheck/test:unit haven't run here; worth confirming they pass locally before merge.
|
|
||
| if (templateId === 'summary') { | ||
| params.addresses = [customParams.id] || constants.example.addresses; | ||
| params.addresses = customParams.id |
There was a problem hiding this comment.
This is the one behavior change in the PR (not a prettier reformat). Before: [customParams.id] || constants.example.addresses — an array literal is always truthy, so the || example.addresses fallback was dead code and params.addresses was always [customParams.id] (i.e. [undefined] when no id query param). After: the ternary actually falls back to constants.example.addresses when id is absent, so the summary preview now renders example data instead of an empty digest. Benign/correct, but it is a functional change to GET /preview/:template, not formatting.
f020bcc to
3e31728
Compare
tony8713
left a comment
There was a problem hiding this comment.
Re-reviewed after the split. The one line I flagged last time (src/preview/utils.ts) no longer changes behavior.
Previously it was rewritten into a ternary that fell back to constants.example.addresses when no id was passed. Now it reads:
const addresses = [customParams.id];
params.addresses = addresses || constants.example.addresses;[customParams.id] is always a truthy array, so || constants.example.addresses stays dead exactly as on main ([x] || y). Net result is params.addresses = [customParams.id] in both, so this is now a pure extract-to-const with no behavior change. The functional bugfix was correctly pulled out into its own PR.
Scanned the rest again with the same lens (flipped booleans, changed defaults, added/removed awaits, altered conditionals, status codes, payload shapes): everything else is prettier line-wrapping, import-order sorting, import type -> import, and catch (e) -> catch (err) / bare catch {}. RPC routes, drizzle/SQL queries, sign/verify logic, email templates and response codes are all untouched. This now matches the "no functional changes" claim.
Note: still carries the #300 dep/eslint9/TS5.9 bumps (base is main, #300 not yet merged), and no CI checks run on the branch, so worth confirming lint/typecheck/test pass locally before merge. LGTM on the formatting.
|
Closing this one: the shared ESLint and Prettier config it applies already landed in #312 and #313, and main is formatted to it now, so there is nothing left here to change. Rebasing would mean working through the conflicts only to re-apply formatting that is already in place, with a real chance of reverting more recent work along the way. #300 stays open on its own merits, since the dependency bumps in this stack have not landed. |
Summary
Stacked on #300
Checks
Review note
Until #300 is merged, GitHub may show the dependency/config commit here too; the actual formatting-only diff is the second commit:
3e31728.