Add a codegen extension SPI, with a transformPath hook - #25
Merged
Conversation
The generated client builds each URL from the operation's @http URI verbatim. That is right when the model describes the whole path, but a service is often mounted under a prefix the model never mentions — a server framework that derives one from a trait, or a reverse proxy — and then every generated request misses it and 404s. A setting cannot express that: one codegen run can emit several services, and they need not share a prefix (`excludeServices` exists for exactly that multi-service case). Nor should it be a trait the generator knows, which would bake one organization's conventions in. So it is an interface to implement: class InternalPrefix extends TsCodegenExtension { override def transformPath(service, operation, path) = PathSegment.Literal("internal") :: PathSegment.Literal(service.getVersion) :: path } listed in META-INF/services and discovered with ServiceLoader. A path is a List[PathSegment], not a String. It has two consumers that must tell literals from labels — the client interpolates a label as ${encodeURIComponent(...)}, the Storybook mock router matches it as a wildcard. A String => String hook would make both re-parse the result and would let an extension return something that no longer parses. Both consumers now resolve through one PathResolver call, so they cannot disagree about a route. The hook replaces the whole path rather than prefixing it, so an extension can also reorder or drop segments; Nil means `/`. What comes back is validated: a label must name a member bound with @httpLabel, and a literal may not contain `/` (which would read as one segment to the client and never match the mock router). Both fail codegen naming the operation, rather than emitting a client that quietly cannot work. The interface lives in a new `smithy-ts-codegen-api` module, kept separate from `core` for the same reason as `traits`: an implementor needs `smithy-model` to inspect shapes, not the generator and its smithy-build/alloy/codegen-core dependencies. `tsCodegenExtensions` puts extension artifacts on the forked codegen's classpath, which is where ServiceLoader looks; without it the SPI would be unreachable from the recommended entry point. They resolve in the same coursier Fetch as the CLI, so a shared dependency lands once. `%%` resolves against the codegen's Scala version, not the enclosing project's — nothing on that classpath runs on the latter. With no extensions this changes nothing: the committed sample is byte-identical. A new scripted test publishes a real extension artifact and asserts the rewritten path in both the client and the mocks, exercising the full sbt -> forked CLI -> ServiceLoader path.
`generate` gains an `extensions` parameter. The default keeps it
source-compatible, but a defaulted parameter still changes the JVM signature, so
the two-argument overload disappears and MiMa catches it against 0.4.0:
* static method generate(Model,Set)String in class TsCodegenPlugin
does not have a correspondent in current version
A bump rather than a ProblemFilters.exclude: `generate` is the documented
programmatic entry point, so the break is genuinely user-visible — unlike the
TsWriter filter dropped when 0.4 opened, which covered a class no caller could
reach. The reasoning there was that a fresh baseline starts clean and a lingering
filter would hide a future break of the same shape; adding one here would do
exactly that.
Needs a v0.5.0 tag when it ships — sbt-typelevel checks the release tag against
tlBaseVersion.
Both files are generated from the build definition, and adding `api` to the root aggregate changes both: `api/target` joins the target directories CI tars up and uploads, and sbt-typelevel-mergify derives a per-project label rule from the aggregate, so it wants a "Label api PRs" entry. Output of `githubWorkflowGenerate` and `mergifyGenerate`; no hand edits. `githubWorkflowCheck` and `mergifyCheck` are what the "Check that workflows are up to date" CI step runs, and they pass now — I had run `test` and `scripted` locally but not these, which is why CI caught it and I did not.
kubukoz
force-pushed
the
path-transform
branch
from
September 2, 2026 18:12
1b02ad6 to
81882b0
Compare
Member
Author
|
Rebased onto main to pick up #24. One conflict in Re-verified on the rebased tree: 49 tests, scalafmt/headers/mima/workflow+mergify checks, both scripted tests ( |
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.
Replaces #23, which added a static
pathPrefixsetting. That design was wrong: one string for the whole model, but a single codegen run can emit several services that need not share a prefix — which is exactly the multi-service caseexcludeServicesalready exists for.The generated client builds each URL from the operation's
@httpURI verbatim. That is right when the model describes the whole path, but a service is often mounted under a prefix the model never mentions — a server framework that derives one from a trait, or a reverse proxy — and then every generated request misses it and 404s.So instead of a setting, an interface:
listed in
META-INF/services/org.polyvariant.smithy.ts.api.TsCodegenExtensionand discovered withServiceLoader. An extension sees each service and operation, so per-service decisions fall out for free — and a convention the model does carry (a trait, theversionfield) is something the extension reads rather than something the generator has to know about.Notes on the design
A path is
List[PathSegment], notString. It has two consumers that must tell literals from labels: the client interpolates a label as${encodeURIComponent(…)}, while the Storybook mock router matches it as a wildcard. AString => Stringhook would force both to re-parse the result, and would let an extension hand back something that no longer parses at all. Both consumers now resolve through onePathResolvercall, so an extension cannot make the client and the mocks disagree about a route.transformPath, notprefixPath. It replaces the whole path, so an extension can reorder or drop segments too, not only prepend.Nilmeans/.What comes back is validated. A
Labelmust name a member bound with@httpLabel, and aLiteralmay not contain/(which reads as one segment to the client but never matches the mock router's segment-by-segment comparison). Both fail codegen with an error naming the operation, rather than emitting a client that quietly cannot work.A new
smithy-ts-codegen-apimodule. Same reasontraitsis separate: an implementor needssmithy-modelto inspect shapes, not the generator and its smithy-build/alloy/codegen-core dependencies.tsCodegenExtensionson the sbt plugin. The forked classpath was closed —resolveCliClasspath(version)and nothing else — so without this the SPI would be unreachable from the recommended entry point. Extension artifacts resolve in the same coursierFetchas the CLI, so a dependency they share with the codegen (smithy-model,smithy-ts-codegen-api) is reconciled to one version instead of landing on the classpath twice.%%resolves against the codegen's Scala version, not the enclosing project's — nothing on that classpath runs on the latter. The setting is already part of the task's cache key via the resolved jars, so adding or bumping an extension re-runs codegen.No
pathPrefixsetting, so there is nothing to deprecate — the plugin, CLI and sbt surfaces are untouched apart from the new setting.Release note: base version moved to 0.5
generategains anextensionsparameter. The default keeps it source-compatible, but a defaulted parameter still changes the JVM signature, so the two-arg overload disappears — and MiMa catches it against the published0.4.0:A bump rather than a
ProblemFilters.exclude:generateis the documented programmatic entry point, so the break is genuinely user-visible — unlike theTsWriterfilter dropped when 0.4 opened, which covered a class no caller could reach. The reasoning there was that a fresh baseline starts clean and a lingering filter would hide a future break of the same shape; adding one here would do exactly that. Happy to switch to a filter if you would rather keep the next release a patch.Needs a
v0.5.0tag when it ships — sbt-typelevel checks the release tag againsttlBaseVersion.Verification
Nil, extensions composing, and both validation failures.extensionsscripted test publishes a real extension artifact and asserts the rewritten path in both the client and the mock segments — the full sbt → forked CLI →ServiceLoaderpath, including the coursier resolution of%%at the codegen's Scala version. I checked it can actually fail by breaking the expected URL (seetsCodegenSampleCheckcan never fail: it regenerates the file it is checking #22 for why that is worth checking here).typecheck/src/generated.tsis byte-identical.clean test,scalafmtCheckAll,scalafmtSbtCheck,headerCheckAll,mimaReportBinaryIssues,doc(no warnings),sbtPlugin/scripted(both tests), andnix flake checkall green locally.