Login: Remove @hey-api/openapi-ts from the login project (closes #23500) - #23501
Merged
leekelleher merged 4 commits intoAug 5, 2026
Merged
Conversation
|
Claude finished @iOvergaard's task in 4m 19s —— View job PR ReviewTarget: Removes
Suggestions
Approved with Suggestions for improvementGood to go. The dependency removal is clean and the error handling correctly maps to how the backoffice services behave ( |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the Login project’s locally generated OpenAPI client (and its @hey-api/openapi-ts dependency) and switches the login app to consume the already-exported backend API services from @umbraco-cms/backoffice/external/backend-api, reducing duplicated code and eliminating the advisory chain originating from the generator tooling.
Changes:
- Replaced generated
./src/api/*usage withSecurityService/UserServiceimports from the backoffice client. - Deleted the generated API client files and removed
openapi-tsgeneration config/scripts/dependency. - Updated Login project documentation to reflect the new API integration approach.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Umbraco.Web.UI.Login/src/types.ts | Switched PasswordConfigurationModel to come from backoffice external/backend-api and kept deprecated UmbProblemDetails type. |
| src/Umbraco.Web.UI.Login/src/mocks/handlers/login.handlers.ts | Removed dependency on generated API types; mocks now only rely on backoffice API types where needed. |
| src/Umbraco.Web.UI.Login/src/contexts/auth.repository.ts | Migrated 5 API calls from generated client functions to SecurityService/UserService methods. |
| src/Umbraco.Web.UI.Login/src/api/types.gen.ts | Removed generated OpenAPI types (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/sdk.gen.ts | Removed generated OpenAPI SDK wrapper (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/index.ts | Removed generated OpenAPI entrypoint (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/core/utils.gen.ts | Removed generated OpenAPI core utility code (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/core/types.gen.ts | Removed generated OpenAPI core types (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/core/serverSentEvents.gen.ts | Removed generated SSE client code (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/core/queryKeySerializer.gen.ts | Removed generated query-key serializer (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/core/pathSerializer.gen.ts | Removed generated path serializer (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/core/params.gen.ts | Removed generated params builder (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/core/bodySerializer.gen.ts | Removed generated body/query serializers (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/core/auth.gen.ts | Removed generated auth helpers (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/client/utils.gen.ts | Removed generated fetch-client utilities and interceptor scaffolding (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/client/types.gen.ts | Removed generated fetch-client types (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/client/index.ts | Removed generated client barrel exports (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/client/client.gen.ts | Removed generated fetch client implementation (no longer needed). |
| src/Umbraco.Web.UI.Login/src/api/client.gen.ts | Removed generated client singleton (no longer needed). |
| src/Umbraco.Web.UI.Login/package.json | Removed generate:server-api script and @hey-api/openapi-ts devDependency. |
| src/Umbraco.Web.UI.Login/package-lock.json | Removed @hey-api/openapi-ts and its transitive dependency chain from the lockfile. |
| src/Umbraco.Web.UI.Login/openapi-ts.config.ts | Removed generator configuration file (no longer used). |
| src/Umbraco.Web.UI.Login/CLAUDE.md | Updated docs to describe the new API integration approach and removed API generation guidance. |
Files not reviewed (1)
- src/Umbraco.Web.UI.Login/package-lock.json: Generated file
Comments suppressed due to low confidence (4)
src/Umbraco.Web.UI.Login/src/contexts/auth.repository.ts:127
- The fallback error text is hardcoded (and not localized), but it is surfaced directly to the user. Use an existing localized term (e.g. auth_defaultError) instead of an English string.
error: this.#getApiErrorDetailText(error, 'Could not validate the password reset code'),
src/Umbraco.Web.UI.Login/src/contexts/auth.repository.ts:146
- The fallback error text is hardcoded (and not localized). Since this message can be displayed in the UI, prefer an existing localization key (e.g. auth_defaultError) to keep translations consistent.
error: this.#getApiErrorDetailText(error, 'Could not reset the password'),
src/Umbraco.Web.UI.Login/src/contexts/auth.repository.ts:183
- The fallback error text is hardcoded (and not localized). Since this ends up shown to the user, prefer an existing localization key (e.g. auth_defaultError) over an English-only string.
error: this.#getApiErrorDetailText(error, 'Could not create a password for the invited user'),
src/Umbraco.Web.UI.Login/src/contexts/auth.repository.ts:164
- The fallback error text is hardcoded (and not localized). Use a localized term (e.g. auth_defaultError) so the invite flow error can be translated consistently.
error: this.#getApiErrorDetailText(error, 'Could not validate the invite code'),
Contributor
Author
|
Thanks — one fixed, one deliberately left. Fixed in 7b9e497: the Not changing here — the hardcoded fallback error strings in
Happy to raise it as an issue so it isn't lost. |
iOvergaard
force-pushed
the
v17/improvement/23500-remove-login-openapi-ts
branch
3 times, most recently
from
August 4, 2026 07:51
0e727f9 to
84c68ea
Compare
iOvergaard
added a commit
that referenced
this pull request
Aug 4, 2026
CodeScene flagged Code Duplication in OperationIdHandlerTests and Excess Number of Function Arguments on its CreateApiDescription helper. The six ID-generation tests were near-identical two-line bodies, so they collapse into a TestCaseSource, and the case data moves into an object initializer that takes no constructor arguments. Also drops the Sonar exclusions added in the previous commit. The login app's generated client is deleted by #23501, so excluding it is short-lived, and it was not what the failing gate was reporting anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
iOvergaard
force-pushed
the
v17/improvement/23500-remove-login-openapi-ts
branch
from
August 4, 2026 07:59
84c68ea to
d1df69d
Compare
Base automatically changed from
v17/bugfix/operation-id-minor-api-versions
to
v17/dev
August 4, 2026 12:44
iOvergaard
added a commit
that referenced
this pull request
Aug 4, 2026
…and upgrade @hey-api/openapi-ts (#23499) * fix(tiptap): load configured stylesheets regardless of extension styles `#renderStyles()` gated the configured `<link>` stylesheets on whether any enabled extension contributed a `<style>` block, so a minimal RTE config (e.g. only Bold/Italic) silently dropped both the user's stylesheets and Umbraco's own base RTE stylesheet. Also fixes the stylesheet root path resolution, which could hang indefinitely if the server never reports an UmbracoCssPath, dropping configured stylesheets for the whole session. Closes #21819 * fix(tiptap): resolve configured stylesheets against the server origin Configured RTE stylesheets were rendered as origin-relative `<link>` hrefs, so when the backoffice is run as a split Vite dev client (VITE_UMBRACO_API_URL pointing at a separate Umbraco server), the browser resolved them against the Vite client's own origin and 404'd instead of hitting the real server. Prefixing with UMB_SERVER_CONTEXT's serverUrl fixes this without affecting production, where it already equals location.origin. * fix(tiptap): avoid doubling the root path when prefixing stylesheets with the server origin Baking the server URL directly into the stylesheetRootPath broke the existing check that skips re-prefixing a configured stylesheet whose stored value already includes the root path segment (e.g. `/css/foo.css`), producing hrefs like `/css/css/foo.css`. The root path now stays origin-relative for that comparison, and the server URL is only applied to the final resolved href via a separate UmbTiptapRteContext.getServerUrl(). * fix(tiptap): address PR review feedback on stylesheet resolution Adds JSDoc to the new public getServerUrl() method, and widens the already-absolute stylesheet check to also skip protocol-relative URLs (e.g. //cdn.example.com/style.css), which would otherwise have been incorrectly prefixed with the root path and server origin. * chore(backoffice): update npm packages to latest minors and patches Refreshes the devDependencies of Umbraco.Web.UI.Client and both lockfiles, clearing the transitive advisories reported in #23429 (16 -> 8 in the client, 2 remaining in the login app). dompurify moves 3.4.1 -> 3.4.12, the only runtime-shipped package among them. eslint is pinned at 10.2.0 rather than bumped: eslint-plugin-import 2.32.0 is the latest release and declares support only up to eslint 9, crashing on 10.8 in the import/order rule. typescript stays at 5.9.3 and @hey-api/openapi-ts stays at ^0.85.2 in the dependencies of src/packages/core, so the published peer floor is unchanged. Refs #23429 Co-Authored-By: Dirk Seefeld <dirk.seefeld@idseefeld.de> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(backoffice): keep both msw worker scripts in sync on install The repo carries two generated msw worker scripts: public/ serves Vite dev and the built app (via umbServiceWorker.js), while the project root serves web-test-runner, whose rootDir is '.'. Only "public" was listed in msw.workerDirectory, so the root copy was never refreshed by postinstall and had drifted to 2.12.4, warning on every test run. msw.workerDirectory accepts an array, so list both and regenerate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(build): exclude the generated msw worker from Sonar analysis Both copies of mockServiceWorker.js are byte-identical vendored output from msw, so Sonar reports 100% duplication on new code once they are in sync. Excluded alongside the other generated sources. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(login): bump msw to 2.15 to match the backoffice client The login app was left on 2.14.6 while Umbraco.Web.UI.Client moved to 2.15.0, so the two apps in the repo were a minor apart for no reason. Its msw.workerDirectory is already configured, so postinstall regenerated the worker script on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(build): allow the esbuild and msw install scripts for npm 12 npm 12 blocks dependency lifecycle scripts unless they are listed in allowScripts. Our engines already permit npm >=11, and #23429 was reported against npm 12, where the install warns and neither postinstall runs. Enumerated what actually declares install scripts across both projects, and it is exactly these two — nothing else, and notably not Playwright, whose browsers we install explicitly. msw's postinstall is what keeps the committed worker scripts in sync with the installed version, so losing it silently reintroduces the drift fixed earlier in this PR. Entries are name-only rather than pinned (npm's default is pkg@version), so a version bump does not require re-approval. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(api): generate valid operation IDs for minor API versions The v1.1 document validate endpoint produced the operation ID "PutUmbracoManagementApiV1.1DocumentByIdValidate1.1", with dots in it, from two separate defects in OperationIdHandler: - VersionPrefixRegex only matched /v<digits>/, so a minor version such as /v1.1/ (and, incidentally, /v10/) was never stripped and the whole route prefix leaked into the ID. - The MapToApiVersion value was appended verbatim, putting a second dot at the end. Dots are not valid in generated client identifiers. @hey-api/openapi-ts 0.85 mangled them into "…Validate11"; newer versions split on them and emit a method literally named "1". Sanitise the version suffix, widen the prefix regex, and give the endpoint an explicit route name so it follows the same convention as its siblings (PutMediaByIdValidate, PutMemberByIdValidate). The regenerated client folds the operation into DocumentService and drops two junk containers: DocumentByIdValidate1Service, and PutUmbracoManagementApiV1Service, which was an empty class the malformed ID had produced. Breaking: DocumentService.putUmbracoManagementApiV1Service .documentByIdValidate1Service.putUmbracoManagementApiV11DocumentByIdValidate11 is now DocumentService.putDocumentByIdValidate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(backoffice): upgrade @hey-api/openapi-ts to 0.99.0 Now possible because the malformed v1.1 operation ID is fixed: 0.99 splits operation IDs on dots and slashes to derive nesting, which turned "…Validate1.1" into a container named _1DocumentByIdValidate1Service holding a method literally named "1". Migrates the deprecated asClass/classNameBuilder options to the new operations config. The generated SDK surface is unchanged: all 516 exported classes and methods keep their names. The fetch client now types request results as Response | undefined, so the 401 retry path throws when no response came back. That rejection is already routed to the caller by the existing .catch(req.reject). Security-wise this is a trade rather than a clear win, and worth being explicit about: it drops the handlebars critical, but json-schema-ref-parser pins js-yaml 4.2.0 exactly, pulling in a high-severity DoS advisory that cascades onto @hey-api/shared and openapi-ts itself. No release of openapi-ts is currently free of both. All of it is build-time codegen that never reaches the browser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(api): address review feedback on operation ID handling Drops a dead null check — GetMapToApiVersionAttributeValue returns a non-nullable string via string.Join, so the branch was unreachable — and renames the tests to the documented Can_/Cannot_ convention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Updated and tidied comments on OperationIdRegexes. * fix(api): scope the version-suffix fix to the Management API Addresses review feedback from @AndyButland. OperationIdHandler also serves the Delivery API, whose 2.0 endpoints have published operation IDs like "GetContent2.0". Sanitising the suffix globally renamed all 8 of them, breaking headless consumers that generate clients from the Delivery spec — and failing DeliveryApi.OpenApiContractTest, which I had not run. Scoped to the Management API namespace, with a TODO to apply it to Delivery in V19 as a documented breaking change. The version-prefix regex needs no scoping: Delivery routes are /v2/, which matched both the old and new patterns. Substitute the dot rather than removing it, so "1.1" becomes "1_1" and cannot collide with a genuine "11" version. Note openapi-ts normalises the underscore away in the generated identifier (putDocumentByIdValidate11), so this protects the spec rather than the hey-api client. Drop the explicit route name from ValidateUpdateDocumentController, so the ID follows the generated convention instead of introducing a hand-written one. Expand the tests from 3 to 8: both branches of the explicit route name short-circuit, the ArgumentException and InvalidOperationException paths, and a Delivery case pinning GetContent2.0 so the scoping cannot silently regress. Writing that coverage surfaced a bug: the missing-relative-path branch built its message from RouteValues["controller"] via the indexer, so callers got KeyNotFoundException instead of the intended InvalidOperationException. Now read defensively. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Revert "Merge branch 'v17/bugfix/21819-tiptap-stylesheets-with-minimal-config' of https://github.com/umbraco/Umbraco-CMS into v17/bugfix/operation-id-minor-api-versions" This reverts commit 96140a0, reversing changes made to 9e3c6e7. * fix(api): pin the v1.1 validate operation ID with an explicit route name Reinstates Name = "PutDocumentByIdValidate", reversing the earlier removal after @AndyButland reconsidered on seeing the generated client method. The backoffice always calls the latest version of an endpoint, so a version in the client method name carries no information — and without a pinned ID every version bump renames the method and churns every call site. Pinning decouples the client from the version: DocumentService.putDocumentByIdValidate again. The handler still sanitises the suffix for endpoints without an explicit name, which is what keeps generated IDs valid, so the unit tests covering that stay. Documents the convention in the Management API CLAUDE.md, including two rules worth stating: only the latest version takes the unversioned name, and two versions must never share an ID since duplicates break OpenAPI generation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(login): restore the openapi-ts bump and exclude generated clients from Sonar Two fixes to the previous merge resolution. The Login app's package.json had been reverted to openapi-ts 0.85.2 by taking v17/dev's side wholesale, while its src/api/* had already been regenerated with 0.99 — so the declared generator no longer matched the generated output. The 0.99.0 bump is restored and the lockfile reconciled. Sonar was also reporting cognitive-complexity and duplication issues against those regenerated files. The backoffice client's generated tree is excluded but the login app's never was, so it has always been analysed; regenerating it just made the pre-existing smells count as new code. Excluded the login api directory to mirror the client, plus a generic **/*.gen.ts glob so a future generated client cannot reintroduce the same asymmetry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(api): make the operation ID tests data-driven CodeScene flagged Code Duplication in OperationIdHandlerTests and Excess Number of Function Arguments on its CreateApiDescription helper. The six ID-generation tests were near-identical two-line bodies, so they collapse into a TestCaseSource, and the case data moves into an object initializer that takes no constructor arguments. Also drops the Sonar exclusions added in the previous commit. The login app's generated client is deleted by #23501, so excluding it is short-lived, and it was not what the failing gate was reporting anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Clarified documentation for when we do have two live versioned endpoints. * Added tests for CanHandle on OperationIdHandler. * Add tests for duplicate or invalid IDs in the stored OpenApi.json file. * Updated outdated documentation. * Addressed further Sonarqube feedback. --------- Co-authored-by: leekelleher <leekelleher@gmail.com> Co-authored-by: Dirk Seefeld <dirk.seefeld@idseefeld.de> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Andy Butland <abutland73@gmail.com>
Backports #22757 from main. The login project generated its own API client for five operations that the backoffice client already exports, so it now imports SecurityService and UserService from @umbraco-cms/backoffice/external/backend-api instead. This removes the dependency entirely rather than upgrading it, which takes the login app from 1 critical + 1 high advisory to zero: every advisory against it came from the openapi-ts chain (handlebars, then json-schema-ref-parser -> js-yaml), and no release of openapi-ts is currently free of both. login.js drops from 76.24 kB to 60.65 kB (gzip 17.88 -> 13.44 kB). Unlike on main, the deprecated UmbProblemDetails type and isProblemDetails() are kept: they were deprecated in V16 (#19495), so the earliest removal is V18. They are hand-written and independent of the generated client, so keeping them costs nothing and this stays non-breaking. Closes #23500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The cherry-pick pulled main's wording over two v17-accurate details: the runtime line (v17 enforces node >=24.13 / npm >=11 in package.json) and the utils listing, which dropped is-problem-details.function.ts even though this backport deliberately keeps that file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
iOvergaard
force-pushed
the
v17/improvement/23500-remove-login-openapi-ts
branch
from
August 4, 2026 13:12
d1df69d to
3c5432f
Compare
Resolves the TODO left by the previous commit. UmbProblemDetails and isProblemDetails() were kept out of caution for the V16 deprecation window, but the login project is private with no exports, main or types, nothing outside it can import from it, and its only internal use is isProblemDetailsLike from @umbraco-cms/backoffice/resources — the replacement. So there is no consumer to break and no reason to hold them until V18. Brings this fully in line with #22757 on main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TS 6 was held back on v17 for the published backoffice types, but the login project is private with no exports, so its compiler version affects nothing downstream. Matches main. Verified that tsc 6.0.3 still type-checks the Client source it reaches through the tsconfig path aliases, not just the login source. CLAUDE.md: drops the deleted is-problem-details helper from the folder listing, and makes the type-resolution note version-agnostic — it claimed the types align with "the in-repo v18 backoffice", which was already wrong on v17 and would drift again on every merge-up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
leekelleher
self-requested a review
August 5, 2026 07:46
leekelleher
approved these changes
Aug 5, 2026
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
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.
Backports #22757 from
mainto v17. Closes #23500.Fixes #23500
What and why
The login project generated its own API client for five operations that the backoffice client already exports. It now imports
SecurityServiceandUserServicefrom@umbraco-cms/backoffice/external/backend-apiinstead, and@hey-api/openapi-tsis gone from the project entirely.This supersedes the login half of #23499 (now merged). That PR bumped the login app's generator to 0.99.0 to keep it level with the backoffice client, which trades the
handlebarscritical for ajs-yamlhigh. Removing the dependency avoids the trade altogether:Every advisory against the login app came from the openapi-ts chain, and no release of openapi-ts is currently free of both
handlebarsand thejson-schema-ref-parser→js-yamlproblem. Deleting the dependency is the only clean exit, andmainalready took it.Side benefit:
login.jsdrops from 76.24 kB to 60.65 kB (gzip 17.88 → 13.44 kB), and ~3000 lines of generated code go with it.Faithful to #22757
This now matches the commit on
main, including itstypescriptbump to^6.0.3. TS 6 is held back on v17 for the published backoffice types, but the login project isprivate: truewith noexports, so its compiler version affects nothing downstream. Worth noting it does type-check the Client's source through the tsconfig path aliases, so this was verified rather than assumed —tsc 6.0.3compiles both cleanly.The deprecated
UmbProblemDetailstype andisProblemDetails()are removed, matching #22757. They were deprecated in #19495 (v16), so the removal policy would ordinarily hold them until v18 — but they have no consumer to protect: the login project isprivate: truewith noexports,mainortypes, nothing outside it can import from it, and its only internal use isisProblemDetailsLikefrom@umbraco-cms/backoffice/resources, which is the replacement rather than the deprecated local copy.I cherry-picked rather than copying
main's files, which mattered: v17 has since renamed the localisation keys inauth.repository.tsfromlogin_*toauth_*, and a straight copy would have silently reverted that. The 3-way merge preserved them.Verified
SecurityService.postSecurityForgotPassword/…Reset/…Verify,UserService.postUserInviteCreatePassword/…Verifynpm audit→ 0 vulnerabilitiesnpm run build(which istsc && vite build) passes./apiimports or@hey-apireferences anywhere in the projecttsc6.0.3 type-checks the login source and the Client source reached via path aliasesauth_*localisation keys intactallowScripts(added in Dependencies: Update backoffice npm packages to latest minors and patches (#23429) #23498 for npm 12) still coversesbuildandmsw, which the login app still needsAlso synced
CLAUDE.mdfor the login project: the deletedis-problem-details.function.tsis dropped from the folder listing, and the type-resolution note is now version-agnostic — it claimed Login's types align with "the in-repo v18 backoffice", which was already wrong on v17 and would drift again on every merge-up.Note
With hindsight the stack would have been better ordered with this PR first: #23499 regenerates the login client with 0.99 only for this PR to delete it, which is what caused a manifest/generated-code mismatch and some transient Sonar noise along the way. Not worth restructuring at this point.
🤖 Generated with Claude Code