Repository navigation
Generate frontend API client from OpenAPI spec and enhance docs - #934
Merged
Merged
Conversation
adminv2, finance-dashboard and inventory each maintained their own
hand-written copy of src/common/api/{entity,provider,hook}.ts, and the
old sdk/ directory was a fourth copy that nothing depended on. They had
already drifted: EventEntity.fastPass existed in adminv2 and in the API
but not in the SDK, and location.capacity existed in the SDK but not in
adminv2. Nothing could detect either, because nothing tied the copies to
the API.
The API already describes itself: 155 of 167 operations carry @apidoc,
every entity carries @ApiProperty, and the resulting document has zero
untyped properties. Make that document the source of truth.
Pipeline:
scripts/generate-openapi.ts emits openapi.json offline, with no
database and no Firebase credentials. An operationIdFactory reshapes
EventController_createOne into event_createOne so generated hooks read
useEventCreateOne rather than useEventControllerCreateOne.
packages/api-client is generated from that document by orval: types,
fetchers and React Query hooks for all 167 operations, grouped by tag.
GET operations become queries, everything else mutations. Nothing under
src/generated is hand-edited.
packages/react-sdk keeps the parts that cannot be generated: the SSO
session, Firebase, the role-based AuthGuard and React Query wiring. It
re-exports the client so apps install one package.
Two bugs kept the previous release from working at all:
process.env.NEXT_PUBLIC_* was compiled into the published bundle.
Bundlers only inline those into code they compile, and they do not
compile node_modules, so every value resolved to undefined in the
consumer's browser and Firebase initialized with an empty API key.
Configuration is now passed to HackPSUProvider by the app.
tsup's rollup treeshaker stripped the "use client" directive, leaving
it only in the .map files. Every Next App Router consumer failed on
import. Treeshaking is off for react-sdk and the directive is emitted
as a banner.
Firebase also no longer initializes at module scope, which used to run
during SSR before the app could supply config.
Auth behaviour is otherwise unchanged: sessionUser, redirect to login on
401, signInWithCustomToken, PostHog identify, sessionLogout. API requests
are authorized with the Firebase ID token rather than the custom token,
matching what FirebaseAuthService validates, and getRole reads claims
from either token shape.
Also:
GET /organizer-applications/by-team/{team} declared a path parameter it
never documented, which made the spec fail validation.
packages/ is excluded from tsconfig.build.json and .dockerignore, in
place of the old sdk/ exclusion, so neither the Nest build nor the
Docker image picks up React sources.
.github/workflows/sdk.yml regenerates the spec on every push, fails the
build if the committed client is stale, and publishes a patch release
when the spec changes. The absence of this step is why the last SDK was
published once in December and never again.
Verified by building a Next.js 16 App Router app against packed
tarballs: compile, type check and static generation all pass, the
previously drifted fields resolve, an unknown field fails to compile, and
seven runtime checks cover bearer auth, FormData uploads, 204s, binary
downloads and 401 handling.
…ation
ApiKeyStrategy passed its verify callback to super(). @nestjs/passport
supplies that callback itself and strips it from the constructor
signature (WithoutCallback<AllConstructorParameters<T>>), requiring an
abstract validate() instead, so the class failed to type-check once
passport-custom 1.2.1 started shipping its own declarations:
TS2515 does not implement inherited abstract member validate
TS2345 argument not assignable to '[] | [options: StrategyOptions]'
Runtime was unaffected, which is why this shipped: passport-custom's
constructor does `if (typeof options === 'function') verify = options`,
so it silently picked the hand-written callback over the one Nest
appended. Moving the logic into validate() removes the reliance on that
accident. Behaviour is preserved: a missing or non-string header returns
false, an unknown key throws UnauthorizedException, and the resolved user
keeps its production/staging role fields.
With the type error gone, spec generation no longer needs
--transpile-only. That flag skipped the type checker that
emitDecoratorMetadata depends on, so parameter types degraded to empty
schemas:
GET /organizer-applications/by-team/{team} emitted schema {}, and the
generated client typed the team argument as unknown rather than string.
BroadcastMessageEntity.broadcast and ActivateFlagBody.broadcast emitted
type object rather than string.
Also annotate broadcast with @ApiProperty({ enum: DefaultTopic }) so the
generated client gets an ALL | ORGANIZER union instead of a bare string.
…egex Spec generation failed in CI because it inherited configuration from a developer's .env. Two providers validate config in their constructors: FirebaseAuthService throws unless AUTH_ENVIRONMENT is production or staging, and AppleWalletService reads its certificates with readFileSync and re-throws when they are missing. The certificates are gitignored, so CI has none. Supply placeholders for both, only when real values are absent, and write throwaway certificate files to a temp directory that is removed after the document is written. AppleWalletService stores those bytes without parsing them at construction, so empty files suffice. Verified that the document generated with no .env is byte-identical to the one generated with real credentials, so CI and local runs cannot diverge. Also replace the trailing-slash strip /\/+$/ with a linear scan in both packages. CodeQL flagged it as js/polynomial-redos, since a string of many slashes backtracks. stripTrailingSlashes is exported from @hackpsu/api-client and reused by react-sdk.
Request id-token: write and upgrade npm in the publish job, so the workflow works unchanged whether CI authenticates with an NPM_TOKEN secret or with npm trusted publishing. Trusted publishing cannot be configured for a package that does not exist yet, and @hackpsu/api-client is new, so document the one manual bootstrap publish followed by either authentication option. Also record why GitHub Packages was not chosen: its registry requires auth to install even public packages, which would put a PAT in every consumer repo and every Vercel deployment.
react-sdk type-checks against @hackpsu/api-client's emitted declarations, so on a clean checkout tsc failed with TS2307 for every import of it. The step order only worked locally because dist already existed. Add a verify script that runs build, typecheck and test in that order, and call it from CI instead of typecheck followed by test.
The maps referenced ../src, which is not in the published files, and esbuild did not inline sourcesContent, so they could never resolve in a consumer. Dropping them halves the tarball: api-client goes from 234kB to 112kB packed, 3.2MB to 1.5MB unpacked.
npm blocked bypass-2FA granular tokens from managing packages in July 2026 and removes their direct-publish ability in January 2027, so an NPM_TOKEN secret is a dead end. Document trusted publishing as the path and keep the token only as an interim fallback.
Publishing and configuring a trusted publisher both require an interactive 2FA challenge that no automation can satisfy, so the setup cannot be fully scripted. Collect the steps into one idempotent script that prompts for a code before each npm write instead: publish both packages, then point each at the sdk.yml workflow so CI publishes with no stored credential.
npm now requires an interactive 2FA challenge to publish and to change package settings, with every bypass removed, and bypass-2FA tokens lose direct-publish ability in January 2027. That would have meant enabling 2FA on a shared account and keeping the seed somewhere the whole team can reach through turnover. Artifact Registry reuses infrastructure the project already has. The API deploys from us-east4-docker.pkg.dev, and api-v3-github-action, the identity behind GCP_DEPLOYER_SA_KEY, already holds roles/artifactregistry.writer at the project level. Publishing therefore adds no new repository secret and no new credential. The repository grants allUsers roles/artifactregistry.reader, so consumers install with no credentials. Each consuming repo adds one line: @HackPSU:registry=https://us-east4-npm.pkg.dev/hackpsu-408118/npm/ That keeps adoption cheap, which matters more here than anything else: the previous SDK failed because copy-pasting entities was easier than adopting the package. Consumers must not copy the always-auth line from packages/.npmrc, which exists only so CI can publish. publishConfig.registry pins both packages to Artifact Registry so a stray npm publish cannot reach npmjs.org. @hackpsu/react-sdk@0.2.1 remains on npmjs.org from the earlier attempt and is abandoned. Verified end to end: both packages published, then a Next.js 16 app installed and built against the live registry with an empty HOME and only the registry line, resolving react-sdk and its transitive api-client dependency unauthenticated.
The release gate compared the regenerated spec against the committed one, which is the inverse of what it needed to check. That value is true only when someone forgot to regenerate, and the stale-artifact check fails the build in exactly that case. A correct commit produced spec-changed=false and skipped publishing; an incorrect one failed before publish could run. The job could never fire. Compare HEAD^..HEAD instead, so the question is "did this push change anything that ships": openapi.json, packages/api-client/src, or packages/react-sdk/src. Docs and CI edits no longer look like releases. Checkout needs fetch-depth 2 for the parent commit to exist. Also add the matching staleness check for openapi.json, which was only enforced for the generated client, and wire the previously unused workflow_dispatch release input into the condition.
Generate the frontend API client from the OpenAPI spec
The first automated release published 1.0.1 and then failed: branch protection on main rejects pushes that do not come from a pull request, so the bot could not commit the version bump back. package.json stayed at 1.0.0 while the registry moved to 1.0.1, and every later release would have computed 1.0.1 again and failed as already published. Read the latest published version from the registry and increment that instead, falling back to package.json only when nothing has been published. Git and the registry can no longer disagree, because git is no longer consulted. The release commit becomes best effort: it is attempted, and a rejection logs a warning rather than failing a job whose package publish already succeeded. Adding a bypass token would have worked too, but keeping the credential out and letting the registry be the source of truth is the smaller system. Version fields are synced to 1.0.1 to match what shipped.
Take release versions from the registry
Entities declared enums with class-validator but not with Swagger, so the document described them as bare strings and the generated client typed them as string. EventEntityResponse.type is now EventType rather than string, and the same for sponsor type and finance category. Where enum was already declared, add enumName. Without it NestJS emits an inline enum per property, so orval generated a separate type for every field that referenced the same enum: RegistrationEntityApplicationStatus, UpdateStatusDtoStatus and OrganizerApplicationEntityFirstChoiceTeam were all distinct types for ApplicationStatus and OrganizerTeam. They now collapse into one shared type each, which removes 611 lines and 16 files from the generated client and lets consumers import ApplicationStatus and OrganizerTeam directly. Organizer.privilege is deliberately left as type: "number". Naming it Role would collide with the auth Role that @hackpsu/react-sdk already exports, and the values are identical.
Three list endpoints declared their active query parameter with the class
that wraps it rather than with the parameter's own type:
{ name: "active", type: ActiveHackathonParams }
The document therefore described active as a required object. Every one of
the handlers branches on `active === undefined`, so it is an optional
boolean, and the generated client required callers to pass an object they
could not construct.
Declare it as an optional boolean on /hackathons, /users and /teams. The
three wrapper schemas drop out of the client, since nothing referenced
them any more.
No runtime change: ApiDoc only feeds the OpenAPI document, and the
handlers and their ValidationPipe transforms are untouched.
22 of 167 operations had no documented 2xx response, so the generated
client typed them as void and no consumer could use them. 12 routes had
no ApiDoc at all.
Every operation now declares a response, and every path parameter is
documented:
API Keys create, list and revoke, which had no ApiDoc at all.
The create response separates the raw key, returned once,
from the stored entity, and revoke is 204.
Drive all seven folder routes, with real schemas for
permissions and folder info rather than bare objects.
Scans organizer scan analytics, and the organizer-with-scans
shape they share, now OrganizerScansEntity.
Analytics GET /analytics/scans had no ApiDoc; the PDF report is
declared as an application/pdf binary alongside the 503
it returns when canvas is unavailable.
Judging both assignment routes and the project CSV upload.
Wallet the Google pass save link, and the Apple pass as an
application/vnd.apple.pkpass binary.
Photos approve and reject declared a description but no type,
so they emitted no schema.
Email the forwarding list, previously an untyped object.
Coverage is now 167 of 167 routes with ApiDoc, zero operations without a
response schema, zero untyped properties, and zero undocumented path
parameters.
No runtime change: ApiDoc only feeds the OpenAPI document. Handlers,
guards and validation pipes are untouched.
Close every OpenAPI documentation gap
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.