Fix: Refer to external refs correctly in strict interfaces#1387
Merged
Conversation
jamietanna
commented
Dec 18, 2023
875db16 to
71813ff
Compare
71813ff to
520ad55
Compare
This was referenced Dec 18, 2023
Closed
The current approach doesn't often work when generating types that are in external packages, unless that package is also a strict server's definition, which isn't always the case.
As part of follow-up changes, we'll need it, and it's handy to keep this consistent with other types that have this.
In the case that we're referencing a type defined in an external package, using an alias here results in build errors i.e.: ./api.gen.go:93:7: cannot define new methods on non-local type common.TypeNameHere Instead, we can make sure we only alias when there's not an external reference in use, using the new `Schema.IsExternalRef` function.
520ad55 to
1ef78fc
Compare
mromaszewicz
added a commit
that referenced
this pull request
Apr 30, 2026
) (#2357) PR #1387 added an `$isExternalRef` branch to the strict-{,fiber-,iris-} interface templates that strips the `<NameTag>Response` suffix when embedding an external response ref. That made external refs to a `components/responses/...` resolve to the bare schema name (`N400`) instead of the strict envelope (`N400JSONResponse`). The result: when spec A and spec B both generate strict-server and A $refs B's response component, A's local envelope embeds `N400JSONResponse` while A's external-ref envelope embeds `externalRef0.N400`. The two struct shapes are no longer identical, so cross-package response casts (the standard pattern for sharing error shapes across services) stop compiling — the regression filed as #2010. Investigation showed there is no smarter alternative: non-strict server modes emit no top-level type for `components/responses/...`, only `models: true` (gives the bare alias) and `strict-server: true` (gives the `<Name>JSONResponse` envelope, which is also the only form that carries a `Headers` field in the with-headers case) do. Changes: - Drop the `$isExternalRef` carve-out from the three strict-interface templates so external refs use the same `<Name>JSONResponse` embedding as internal refs. - Update `internal/test/issues/issue-removed-external-ref` golden output to match. - Update `internal/test/issues/issue-2113`'s common-package config to also generate `strict-server: true`. The fixture was relying on the PR #1387 behavior; under the new policy the destination of a strict-server external ref must also generate a strict server, so `StandardErrorJSONResponse` is in scope. - Add `internal/test/issues/issue-2010` regression fixture: two specs with strict-server, the second `$ref`s the first's `components/responses/400`, and the test exercises the cross-package cast that was broken. - README: note the cross-spec strict-server requirement under the strict-server section. The earlier two commits of #1387 are kept: the `Schema.IsExternalRef` helper, and the alias-vs-defined-type fix for content-schema external refs (which is a genuinely independent bug fix — methods can't be attached to non-local aliases). BREAKING CHANGE: external `$ref` to a `components/responses/...` from a strict-server target now requires the destination spec to also generate `strict-server: true`. This restores cross-package response casting that worked in v2.0.0 and earlier. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
The current approach doesn't often work when generating types that are
in external packages, unless that package is also a strict server's
definition, which isn't always the case.
Noticed while working on #1378