ReviewOS

also looking at this

stacks/stx

style(desktop): format the two files holding the format check red

#1949
Merged glennmichael123 wants to merge fix/desktop-format into main
2 files +30 -29
8 days ago

format:check has been failing on main since 1b3f8c1 (fix(desktop): standardMenus built menus of separators), which landed dialogs.ts and menu.ts unformatted.

It runs inside the lint job, so every open PR in this repo is red for a reason unrelated to its own diff. #1948 is what turned this up: its own two files format clean, and these two are the entire failure.

Most of it is the formatter's own preference

pickier puts multi-line union members at column 0:

 export type MenuRole =
-  | 'about'
-  | 'hide'
+| 'about'
+| 'hide'

Unusual, but it is the house style here rather than a regression: these are the only two files in the format scope that indent a multi-line union. The other 23 that do all sit under packages/stx-native/** or packages/stx/src/**, both of which .config/code-style.ts lists in ignores. No in-scope file disagrees.

One spot is not style

pickier cannot indent a multi-line callback inside a ternary, and flattened the body against the surrounding lines:

    disposeHandlers = handlers.size > 0
      ? onCraftEvent<MenuActionEvent>('craft:menu:action', (event) => {
      handlers.get(event.id)?.()
    })
      : null

Committing that would make the file harder to read to satisfy a tool. Written as a statement instead, which the formatter indents correctly and which reads better anyway:

    disposeHandlers = null
    if (handlers.size > 0) {
      disposeHandlers = onCraftEvent<MenuActionEvent>('craft:menu:action', (event) => {
        handlers.get(event.id)?.()
      })
    }

Behaviour is identical: null when there are no handlers, the disposer otherwise. That is the only non-whitespace change in this PR.

Verification

  • format:check: clean, 233 files, 0 errors (was 2).
  • lint: 0 errors (258 warnings, all pre-existing).
  • typecheck: clean.
  • Desktop suite: 650 pass / 0 fail.
  • Formatter output confirmed idempotent - re-running --check after --write is clean.

Worth merging ahead of #1948 so that PR's CI reflects its own diff.

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.