Skip to content

refactor: guard FileNameCell's citation on publicationString so the string narrows #3219

Description

@frano-m

Problem

FileNameCell guards its citation line on a value derived from the citation rather than on the citation itself, so TypeScript's narrowing does not reach the string that is actually rendered.

// components/.../SourceDatasets/components/MainColumn/components/table/components/FileNameCell/fileNameCell.tsx
const citation = publicationString
  ? splitTrailingWord(publicationString)
  : null;

{citation && (                          // citation → { head, tail }        narrowed
  
  {doi ? <Link label={…citation.head…citation.tail…} />
       : publicationString }            // publicationString → string | null  NOT narrowed
)}

The relevant types:

  • publicationString: string | null (@types/network.ts:247)
  • splitTrailingWord(text: string): { head: string; tail: string } — never returns null
  • so citation: { head: string; tail: string } | null

Narrowing follows the expression that is tested. Testing citation tells TypeScript about citation; nothing in the type system records that citation was derived from publicationString, so inside the block publicationString remains string | null even though it provably cannot be null there.

Demonstrated. Changing the no-DOI branch from publicationString to publicationString.trimEnd() produces:

error TS18047: 'publicationString' is possibly 'null'.

Why nothing is broken today

The no-DOI branch renders publicationString as a JSX child, and null is a valid ReactNode — it renders nothing. So string | null satisfies the type and the weaker knowledge never bites.

Why it is still worth fixing

  1. It taxes the next edit. Anyone needing to use that string in that branch — uppercase it, measure it, pass it to a function taking string — hits TS18047 and has to add ! or ?? "", silencing a check that was never genuinely in doubt.
  2. The condition states a slightly different fact than it means. citation && reads as "if we produced a split citation"; the real condition is "if there is a citation string". Both spellings of one fact sit in the same block, and the no-DOI branch renders the original while the guard tested the derivative.

Root cause and suggested fix

splitTrailingWord is only needed in the DOI branch — it exists to hold the external-link icon against the citation's last word, and there is no icon without a DOI. Hoisting the split above the doi check is what forced the guard onto the derived value.

Guard on publicationString and split inside the DOI branch:

{publicationString && (          // publicationString → string
  
  {doi ? <CitationLink citation={splitTrailingWord(publicationString)} doi={doi} />
       : publicationString }
)}

Note this is not free: keeping it to a single splitTrailingWord call while splitting inside a JSX ternary needs either a small subcomponent (as sketched above) or accepting one inline call in the branch. Reverting to two calls would undo #3213, so whichever shape is chosen should keep it at one.

Acceptance criteria

  • The citation line is guarded on publicationString, and inside that block publicationString is typed string - verified by using it as a string (e.g. .trimEnd()) without a null assertion and with tsc clean.
  • splitTrailingWord is still called at most once per render.
  • Rendered output is unchanged: citation at ink.light / body-small-400, icon inside the anchor, final word and icon held on one line, (opens in a new tab) in the accessible name.
  • npm run lint, npm run check-format, npx tsc --noEmit and npm run build-prod:data-portal all pass.

Notes

Raised as a non-blocking nit on #3213 ("guarding on citation is a slightly indirect stand-in for 'we have a publication string' … narrowing is lost"), which is merged. Best folded into whichever ticket next touches FileNameCell rather than done on its own.

Compare rendered output by content rather than file hashes - this build is not byte-deterministic between runs.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions