Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Openapi enum naming og undefined istadenfor null #7015

Merged
merged 18 commits into from
Jan 24, 2025

Conversation

josstn
Copy link
Collaborator

@josstn josstn commented Jan 24, 2025

Denne PR introduserer typescript klient generert med nye enum definisjoner, og som berre bruker undefined for "nullable" properties. Fikser dermed tre ulike problem:

  1. Ved generering av typescript klient frå k9-sak openapi spesifikasjon har det blitt generert "dårlege" enum typer. Namnet på enum typer var namnet på property der enum type var brukt, noko som førte til kollisjoner sidan fleire properties med samme namn (på ulike klasser) har definert ulike enum typer.

  2. I tillegg til enum typer blir det generert typescript const objekter med enum verdiane. Disse vart tidlegare generert med property namn likt verdien av enum, sjølv om enum definisjon i java koden var definert med eit anna namn enn verdien. Dette gjorde det vanskeleg å bruke disse enum objekta når enum verdien var lite logisk.

  3. Det har og vore plageleg at genererte typescript typer for "nullable" properties vart definert som både potensielt undefined og null, så ein måtte heile tida sjekke for begge deler i typescript.

Er relatert til PR https://github.com/navikt/k9-sak/pull/11381

Inneheld og endringer frå PR #7012 som var nødvendige for å få kompilert med ny generert typescript klient kode. Så den bør godkjennast og mergast før denne.

josstn and others added 16 commits January 10, 2025 10:49
k9-sak openapi spesifikasjon blir generert med betre enum type namn og property namn som stemmer med java kjeldekoden. Så skriver om all bruk av disse til å stemme med nye definisjoner.

Endrer også til å returnere undefined istadenfor null for properties som ikkje er "required", slik at ein slepp å sjekke både for null og undefined. Endrer derfor frå null til undefined i der det trengs (testdata).
…ode.

Denne commit legger til verktøyet loose-ts-checks som gjere det mulig å ignorere enkelte ts error koder i eit spesifikt sett kodefiler.

Dette gjere det mulig å aktivere strengare tsconfig.json på gammal kode uten å måtte fikse alle problem på ein gang.

Aktiverer samtidig strictNullChecks på gammal tsconfig.json fil og ignorerer alle eksisterande feil.

Aktivering av strictNullChecks er nødvendig for at react-hook-form + yup validation skal fungere skikkeleg mtp undefined/null verdi validering.

v2/ tsconfig oppsett er ikkje påvirka av denne endring.

Sjå https://github.com/Gelio/loose-ts-check for meir info om bruken av dette verktøy.
ts-check:update-ignore Oppdaterer filene som lagrer kva ts feil loose-ts-check ignorerer og kva filer som har ignorerte feil i seg. Den skal køyrast når ein har fiksa ein ignorert feil, slik at det ikkje er mulig å reintrodusere den seinare utan å få feil.

ts-check:v2 køyrer ts-check berre på v2/, med tilhøyrande tsconfig.json. Kan brukast for å få raskare typesjekking lokalt når ein utvikler kode i v2/.
Var ved ein feil satt til NotNull i backend definisjon.
Proof of Concept av å validere og samtidig konvertere form input til korrekt typescript dto type.

Mangler handtering av valideringsfeil ved skjema submit.
# Conflicts:
#	packages/v2/gui/src/utils/aksjonspunkt.ts
@josstn josstn marked this pull request as ready for review January 24, 2025 08:48
@josstn josstn requested a review from a team as a code owner January 24, 2025 08:48
# Conflicts:
#	packages/v2/backend/package.json
#	packages/v2/gui/src/prosess/uttak/vurder-overlappende-sak/VurderOverlappendeSak.tsx
#	yarn.lock
@josstn josstn merged commit f0101f7 into master Jan 24, 2025
5 checks passed
@josstn josstn deleted the openapi-enum-naming-undefined branch January 24, 2025 12:33
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.

2 participants