Configuration admin page: four deferred review nits from #1988

Aperta
  • Qualcosa non va?
#2,033 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 2 giorni

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
25/100
Tipo di issue
Refactoring
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
typescript
Ambito
developer-experience

Direzione di ricerca

Start with src/features/admin/configuration/index.tsx, getConfiguration.ts, useUpdateConfiguration.ts, and api.patch.d.ts, then check the repo-wide import and query conventions and the generated API types from central-manager’s openapi.json. The issue lists four deferred review nits, but three depend on repo-wide decisions or growth; agree on which changes Studio wants before editing. Done means the selected conventions are applied consistently and the Configuration page still works.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Cross-model review of #1988 (the Admin → Configuration page) raised four nits that were deferred rather than fixed in that PR. Three match how the rest of Studio is written today, so each one says what the repo-wide change would be rather than proposing a one-off fix for this page.

1. Local imports omit file extensions

src/features/admin/configuration/index.tsx imports its siblings without .ts/.tsx extensions. This is Studio's convention: about 1 in 1,500 relative imports under src/ carries an extension. It is only worth acting on if Studio decides to adopt extensioned imports everywhere; otherwise close this item.

Review-Deferral: e3cf9431bf9907fc severity=nit category=maintainability

2. The filter redoes work for every row on each keystroke

Typing in the filter box:

  • lower-cases every key's name and description;
  • re-stringifies every row's value through formatValue.

The closed edit dialog also parses its placeholder value on every render. With the current few dozen keys this costs nothing measurable. If the table grows, memoize the formatted and lower-cased strings per entry alongside the query data.

Review-Deferral: c061a4cfe6c1e8e6 severity=nit category=performance

3. Double casts instead of generated types

getConfiguration.ts and useUpdateConfiguration.ts cast data as unknown as AdminConfigurationEntry[], and api.patch.d.ts carries a hand-written AdminConfigurationEntry, because Studio's generated API types predate the superUserOnly flag. Every other admin query does the same (regions, organization regions, locations). Central-manager's openapi.json on main now includes superUserOnly. Regenerating Studio's types from it would let this page drop both the patch type and the casts.

Review-Deferral: 64d604298c694ca2 severity=nit category=maintainability

4. Exports with no consumer outside their own file

These exports are used only inside their own file:

  • getConfiguration and ParsedValue;
  • updateConfiguration and UpdateConfigurationInput.

(parseConfigValue and jsonType are used by the page's tests.) Other query and mutation modules export their fetch functions the same way. Drop the exports if Studio adopts an unused-exports lint rule; otherwise close this item.

Review-Deferral: 7244681086f2b812 severity=nit category=maintainability

🤖 Generated by Claude Code (Claude Opus 5.5); posted via @Devin-Holland.

Lingua principale
TypeScript
Stelle
5
Fork
4
Merge medio
1g 19h
PR unite (30g)
144

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di HarperFast/studio

Tutte le issue di HarperFast/studio

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.