No blank lines between sibling elements in JSX

[georgekaran]

Context

Blank lines between sibling elements in JSX, like this:

<>
  {failure !== null && <AuditFailure failure={failure} onRetry={retry} />}

  <AuditStatus message={announcement} />

  {failure === null && done && audit.data !== undefined && (
    <>
      <AuditResult audit={audit.data} />
      <TrackThisPage />
    </>
  )}
</>

Markup is a tree, and vertical gaps in it suggest a grouping that the tree does not have. Indentation already says what contains what; a blank line between two siblings says something additional and unspecified, and different files end up saying different things with it.

The codebase mostly agrees already. 36 of 44 .tsx files have none. The 42 that exist sit in eight files:

File	Count
screens/Home/landing.tsx	22
components/AuditFailure/index.tsx	4
components/AuditResult/index.tsx	4
screens/Home/landing-parts/score-chart.tsx	4
components/Layout/index.tsx	2
components/UrlField/index.tsx	2
components/ViolationList/index.tsx	2
screens/Home/index.tsx	2

So this is finishing a convention rather than introducing one - and over half of it is in one file.

The decision this needs

There is no linter in this repository. No ESLint, no Biome, no Prettier, no config and no lint script in any package. The style has been held by hand, and consistently enough that a survey turns up 42 exceptions in 7,000 lines.

That makes "turn on react/jsx-newline" a bigger change than it sounds: it means adopting ESLint, a plugin, a config and a CI step for one rule, and every other rule that config brings with it becomes a decision too.

And the usual answer does not work here. This repo is on typescript@7.0.2, the native port, which ships two exports and puts the AST behind ./unstable/ast:

$ node -e "const ts=require('typescript'); console.log(typeof ts.createSourceFile, typeof ts.createProgram)"
undefined undefined

typescript-eslint parses through ts.createSourceFile and builds type information through ts.createProgram, so ESLint's standard TypeScript setup has nothing to parse with. server/src/architecture.spec.ts already hit the same wall and says so - it matches imports by regex because there is no AST to walk.

A linter here therefore has to bring its own TypeScript parser rather than borrow the compiler's, which narrows the field to the Rust-based tools and makes the choice a real one rather than a default.

The alternative is the mechanism this repo already uses. web/src/conventions.spec.ts asserts that component folders are PascalCase, that each has an index and a spec, and that the casing is recorded in git. server/src/architecture.spec.ts enforces import rules by reading source and matching patterns. House rules here are already specs, not lint config.

Adding this rule to conventions.spec.ts costs one test and no new dependency. Recommend that, and record the choice either way.

Scope

In

• Remove the 42 blank lines.
• A check that keeps them out, in conventions.spec.ts unless the decision above goes the other way.

Out

• Blank lines anywhere else. Between imports and code, between functions, inside hook bodies - all fine, all unchanged. This is siblings inside JSX only.
• Any other formatting rule. If a linter is adopted, the rules it brings are a separate conversation.

Things to be careful about

• A pattern-matching check gets this wrong in ways worth enumerating. A blank line before a {/* … */} comment that documents the next element, a blank line inside a multi-line expression that happens to sit between elements, and a blank line between the end of JSX and the code after it are all different cases. architecture.spec.ts carries a comment about exactly this hazard - its own list has been wrong three times - and the same honesty applies here: the check should state which forms it covers and no more.
• landing.tsx is a port, and holds 22 of the 42. Reformatting it makes the next diff against its source harder to read. Worth deciding whether it is exempt, reformatted once and owned from then on, or reformatted at its source first.
• Removing a blank line above a comment can attach the comment to the wrong element if the comment was documenting what came before it. Read each one rather than running a regex over all 42.
• The change is whitespace, so nothing should move in the rendered output. If a snapshot or a text assertion shifts, that is a real finding and not noise.

Acceptance criteria

• No blank lines between sibling elements in JSX across web/src
• A check keeps them out, and names the forms it covers
• The mechanism - spec or linter - is recorded in DECISIONS.md
• landing.tsx's status is decided explicitly rather than by default
• pnpm typecheck, pnpm test and pnpm build pass, with no assertion changing

[georgekaran]

Checked this against the toolchain in #91, since that removes the "adopting a linter for one rule" objection this issue was weighing. It does not resolve this issue. The recommendation here still stands, for a new reason.

react/jsx-newline does not exist in oxlint. It implements 64 react/* rules — including 18 jsx-* ones — and that is not among them.

oxfmt does not remove the blank lines either. Verified rather than assumed:

// input                                  // after oxfmt
<>                                        <>
  {failure !== null && <AuditFailure />}    {failure !== null && <AuditFailure />}
                                          
  <AuditStatus message={announcement} />    <AuditStatus message={announcement} />
                                          
  {/* comment */}                           {/* comment */}
  <Result />                                <Result />
                                          
</>                                       </>

Both interior blank lines survive. The only one it removed was the trailing blank before the closing tag. That is Prettier's behaviour too — a single blank line between siblings is treated as authored intent and preserved.

So after #91 lands there is still no lint rule and no formatter setting that covers this, and the choice is unchanged: conventions.spec.ts, which costs one test and no dependency.

One thing this does settle: the issue asked whether adopting a linter would bring this rule with it. It would not, so that no longer needs to be weighed. Everything else in "Things to be careful about" is untouched — particularly that the check must name the forms it covers, and that landing.tsx's 22 instances need an explicit decision rather than a default.