Skip to content

Apply prettier formatting to client - #7786

Open
yoshiokatsuneo wants to merge 4 commits into
getredash:masterfrom
yoshiokatsuneo:chore/apply-prettier-formatting
Open

Apply prettier formatting to client#7786
yoshiokatsuneo wants to merge 4 commits into
getredash:masterfrom
yoshiokatsuneo:chore/apply-prettier-formatting

Conversation

@yoshiokatsuneo

@yoshiokatsuneo yoshiokatsuneo commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • Refactor
  • Feature
  • Bug Fix
  • New Query Runner (Data Source)
  • New Alert Destination
  • Other

Description

Purpose: stop every individual PR from having to carry prettier changes.

Today 180 files under the Restyled prettier scope are unformatted on master. Any PR that touches one of them either fails the Restyled check or has to include a reformatting commit of its own. That is inefficient and it makes PRs harder to read: the formatting hunks bury the actual change, so reviewers have to separate "what this PR really does" from "what prettier did" by hand, in every single PR. Doing it once here removes that cost from all future PRs.

The remaining drift comes almost entirely from the prettier 2 → 3 upgrade, which changed the arrowParens default from "avoid" to "always" (x => x(x) => x). These files were never reformatted after that upgrade.

Reformatting alone would only reset the clock, so the last commit also makes pnpm run prettier and the restyler agree on one file list — including viz-lib, which nothing in CI checks today.

No behaviour change. Best reviewed with whitespace changes hidden (?w=1), and commit by commit.

How this diff was produced

Everything here is the output of the prettier scripts already defined in the repo — with exactly one hand-written exception, kept in its own commit. The three commits are meant to be reviewed separately:

1. Apply prettier formatting to client — 186 files, verbatim output of:

$ pnpm install --frozen-lockfile
$ pnpm run prettier

which expands to:

$ prettier --write 'client/app/**/*.{js,jsx,ts,tsx}' 'client/cypress/**/*.{js,jsx,ts,tsx}'

2. Apply prettier formatting to viz-lib — 103 files, verbatim output of:

$ pnpm --filter @redash/viz run prettier

which expands to prettier --write 'src/**/*.{ts,tsx}'. viz-lib is not in the restyler include list, so CI never flags it, but it carries the same drift.

3. Keep @ts-expect-error suppressions working after reformatting — 2 files, +2 lines. One of the two hand-written commits. // @ts-expect-error suppresses only the line that follows it. In DefaultColorsSettings.tsx and PieColorsSettings.tsx the Select.Option element used to fit on one line, so a single directive covered both the opening and the closing tag; prettier splits the element across three lines, leaving the closing tag uncovered and tsc failing with Property 'Option' does not exist on type .... That is not just a type-check failure: postinstall runs build:viz, whose type-gen step is tsc --emitDeclarationOnly, so pnpm install --frozen-lockfile itself would fail. The commit adds the closing-tag suppression in the JSX comment form already used for exactly this case in AxisSettings.tsx and word-cloud/Editor.tsx. Re-running prettier afterwards produces no further changes.

4. Align the prettier scope between pnpm run prettier and the restyler — 2 files, one line of package.json plus the restyler include. The other hand-written commit, and the part that keeps the drift from coming back. Today there are three different answers to "what does prettier cover here":

scope
pnpm run prettier client js/jsx/ts/tsx
prettier restyler in .restyled.yaml, run by restyled.yml client js/jsx only
viz-lib's own prettier script viz-lib/src

So running the documented command locally does not tell you whether the Restyled check will be happy. And viz-lib is checked by nothing: it is outside the restyler include, and the restyler runs prettier from its own image rather than any of the package scripts, so viz-lib's script is only ever run by hand. This commit adds viz-lib/src to the root script and widens the restyler include to the same set of files. No new CI step is needed: the Restyled workflow already runs with fail-on-differences: true, so widening include is what enforces it.

All of the above uses prettier 3.3.2 — the version pinned in devDependencies, and the same version as the Restyled prettier restyler in .restyled.yaml.

How is this tested?

  • Unit tests (pytest, jest)
  • E2E Tests (Cypress)
  • Manually
  • N/A

Everything the frontend-lint and frontend-unit-tests jobs run, on a clean pnpm install --frozen-lockfile:

$ pnpm run test                      # 15 suites, 89 passed / 1 skipped
$ pnpm --filter @redash/viz test     # 24 suites, 154 passed, 61 snapshots
$ pnpm run lint --max-warnings 0     # passes

All 61 viz-lib snapshots still match. They capture the options objects the editors produce in response to interaction rather than rendered markup, so what they confirm is that the editor behaviour is unchanged.

Plus, specific to this PR:

$ pnpm run prettier
# no changes — the tree is stable under prettier, including viz-lib

$ pnpm --filter @redash/viz run type-check
# passes (fails without commit 3, see above)

The pnpm install above is itself part of the check: its postinstall runs build:viztype-gentsc --emitDeclarationOnly, which is exactly what commit 3 keeps working.

Related Tickets & Documents

Prettier was bumped to 3.x without a follow-up reformat, which is where the drift comes from.

Related: #7669 (React v19 update) — a concrete example of the problem this PR is meant to remove. That PR states in its own description:

Formatting note: Prettier was forcefully applied to all files, so the PR includes broad formatting-only churn in addition to the dependency updates.

The result is 418 changed files, in which the actual React 19 / Ant Design 6 migration is mixed with formatting-only churn, so a reviewer cannot tell the two apart without going file by file. Had the tree already been formatted, that churn would simply not have existed and the PR would show only the migration itself.

Ordering is not a blocker either way, but landing this PR first is the cheaper order: #7669 can then rebase and drop its formatting churn, leaving only the migration in its diff. Merging this PR will conflict with #7669, but every such conflict is formatting-only and is resolved by re-running pnpm run prettier on the rebased branch. Note that #7669 applied prettier to all files, which is wider than the scope here (e.g. it also covers viz-lib), so a smaller amount of formatting churn will remain in it.

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

N/A


🤖 This PR was written by Claude Opus 5 (model id: claude-opus-5) in Claude Code, and reviewed by @yoshiokatsuneo before opening.

@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown

Too many files changed for review (291 files, 100 file limit).

Bypass the limit by tagging @greptile-apps to review.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 186 files

Re-trigger cubic

Run `pnpm run prettier` (prettier 3.3.2, the same version used by the
Restyled prettier restyler) across the client sources so that the tree is
already formatted.

Most of the diff comes from the prettier 2 -> 3 upgrade changing the
`arrowParens` default from "avoid" to "always". Because these files were
never reformatted after the upgrade, every PR touching them picks up
unrelated formatting changes and gets flagged by the Restyled check.

Formatting only, no behaviour change.

The whole diff is machine-generated: it is the verbatim output of
`pnpm run prettier`, no file was edited by hand.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
AI-Model: Claude Opus 5 (model id: claude-opus-5)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@yoshiokatsuneo
yoshiokatsuneo force-pushed the chore/apply-prettier-formatting branch from 8cbf6ae to ff3df7d Compare August 12, 2026 13:06
yoshiokatsuneo and others added 3 commits August 12, 2026 22:21
Run the viz-lib `prettier` script over its sources, for the same reason as
the previous commit: so that unrelated formatting churn stops showing up in
every PR that touches these files.

viz-lib is not covered by the prettier restyler in `.restyled.yaml`, so this
part is not enforced by CI, but the drift is the same prettier 2 -> 3
`arrowParens` change.

Formatting only, no behaviour change. Verbatim output of
`pnpm --filter @redash/viz run prettier`, no file edited by hand.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
AI-Model: Claude Opus 5 (model id: claude-opus-5)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`// @ts-expect-error` only suppresses the line that follows it. In these two
files the `Select.Option` element used to fit on a single line, so one
directive covered both the opening and the closing tag. Prettier splits the
element across three lines, which leaves the closing tag uncovered and makes
`tsc` fail with:

    Property 'Option' does not exist on type '({ className, id, layout, ... }: any) => Element'

That breaks more than `type-check`: `postinstall` runs `build:viz`, whose
`type-gen` step is `tsc --emitDeclarationOnly`, so `pnpm install` itself
would fail.

Add the closing-tag suppression in the JSX comment form already used for
exactly this case elsewhere in viz-lib (see `AxisSettings.tsx` and
`word-cloud/Editor.tsx`).

This is the only hand-written change in this PR; the formatting commits
contain nothing but prettier output.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
AI-Model: Claude Opus 5 (model id: claude-opus-5)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Until now there were three different definitions of "what prettier covers":

  - `pnpm run prettier` formatted client js/jsx/ts/tsx
  - the restyler in `.restyled.yaml` checked client js/jsx only
  - `viz-lib` had its own script that nothing in CI ever ran

So running the documented command locally did not tell you whether the
Restyled check would be happy, and viz-lib was not covered at all. Add
`viz-lib/src` to the root script and widen the restyler `include` to the
same set of files, so the command and CI agree.

No new CI step is needed: the Restyled workflow already runs with
`fail-on-differences: true`, so the widened `include` is enforced.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
AI-Model: Claude Opus 5 (model id: claude-opus-5)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@yoshiokatsuneo
yoshiokatsuneo force-pushed the chore/apply-prettier-formatting branch from 5f5a472 to d327424 Compare August 12, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant