Skip to content

Fix: Refer to external refs correctly in strict interfaces#1387

Merged
jamietanna merged 3 commits into
masterfrom
feature/external-ref-strict
Jan 25, 2024
Merged

Fix: Refer to external refs correctly in strict interfaces#1387
jamietanna merged 3 commits into
masterfrom
feature/external-ref-strict

Conversation

@jamietanna

@jamietanna jamietanna commented Dec 18, 2023

Copy link
Copy Markdown
Member

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

Comment thread pkg/codegen/templates/strict/strict-fiber-interface.tmpl
@jamietanna jamietanna marked this pull request as draft December 18, 2023 12:14
@jamietanna jamietanna force-pushed the feature/external-ref-strict branch from 875db16 to 71813ff Compare December 18, 2023 12:22
@jamietanna jamietanna marked this pull request as ready for review December 18, 2023 12:22
@jamietanna jamietanna force-pushed the feature/external-ref-strict branch from 71813ff to 520ad55 Compare December 18, 2023 12:33
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.
@jamietanna jamietanna force-pushed the feature/external-ref-strict branch from 520ad55 to 1ef78fc Compare January 19, 2024 10:57
@jamietanna jamietanna added bug Something isn't working area:external-refs Anything related to external references (referencing other file(s) within a spec) labels Jan 25, 2024
@jamietanna jamietanna merged commit 390058d into master Jan 25, 2024
@jamietanna jamietanna deleted the feature/external-ref-strict branch May 5, 2025 16:24
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:external-refs Anything related to external references (referencing other file(s) within a spec) bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant