Refactored the generator into explicit abstractions and typed the JavaScript surface by Andrzej-Swietek · Pull Request #7 · AVSystem/openapi-ng

Andrzej-Swietek · 2026-09-08T16:11:27Z

Summary

The generator was correct but hard to change safely: no traits, diagnostics
threaded as &mut through 22 signatures, schema depth counted by convention
at each call site, four files past 400 lines. Two checks were wrong: a
repeated key under paths reported as a duplicate schema name, and the
fixture list regen-snapshots read had drifted from test/fixtures/.
Nothing type-checked the published JavaScript or the tests.

Behaviour changes

Generated output is unchanged. These are not covered by that claim:

A repeated key under paths now fails as E_INPUT_INVALID, not
E_POLICY_VIOLATION — caught while decoding, before the policy pass.
A JSON input with a repeated key was last-wins and now fails. serde_json
carries no field path, so in JSON a duplicate schema name gets no
duplicate-schema-name subcode; YAML still routes it.
duplicate-schema-name now matches components.schemas: exactly, so a
nested path like Pet.properties is a plain decode failure.
The anchor guard no longer passes a source whose Value parse failed on a
duplicate key.
A group name with no letters or digits is rejected; it produced
rest/.rest.ts and rest//index.ts.
InputFormat and ResponseType stop being const enum and are exported
as runtime constants from both entries.
build:debug now runs postbuild.

Verified

131 pre-existing snapshots unchanged; CLI output identical across 21
invocations against the base; templates/, browser.d.ts and native.js
byte-identical to main. 325 Rust tests, 398 ava, clippy and three
typecheck projects clean. YAML throughput 1.6–2.2x, JSON unchanged — the win
is the removed double parse.

One open question: rest-util-operations.spec.ts asserts
bound.request({}, { injector }) while standalone-proof.ts carries
@ts-expect-error — the bound .request() takes no options. templates/ is
left as #8 wrote it; the test casts.

…generator

…ions

pkurcx

Thanks, this is a careful refactor and the structure is a clear improvement. I built both this head and its base commit (1f1944e) and ran every fixture in test/fixtures through both CLIs: generated files, stdout, stderr and exit codes are identical for all 73, except the disclosed duplicate-schema-name wording. That covers the ten newly pinned fixtures too, which the new snapshots alone cannot prove. Also confirmed: fmt/clippy/cargo test clean (305), typecheck/lint/coverage/ava clean (345), and all ten commits compile standalone.

Two things need fixing before merge, and one rebase hazard:

Runtime enums are declared but not exported. index.d.ts now promises InputFormat and ResponseType as runtime constants; lib/index.js and lib/browser.js export only generate, GenerateError, EmitTarget. InputFormat.Yaml (shown in the node-api docs) type-checks and throws at runtime. Under the old const enum it was inlined and worked. Inline comment below.
Inline-object depth is charged twice per level in schema/mod.rs, roughly halving the nesting cap. Inline comment below.
Rebase onto main. main has moved (v0.6.0, #8): 20 files conflict. #8 also added a Layout const-enum rewrite to scripts/patch-types.mjs, which this PR deletes, so that rewrite has to be ported into patch-types.ts or it silently disappears. The ten new snapshot dirs need regenerating under the new model.ts / rest/<tag>.rest.ts names.

Smaller behaviour changes worth listing in the description, since they are not covered by the byte-identical claim:

JSON inputs with a repeated key were last-wins on the base and now fail with E_INPUT_INVALID (no subcode, so a duplicate schema name in JSON never gets duplicate-schema-name).
A repeated key under paths changes code from E_POLICY_VIOLATION to E_INPUT_INVALID, not only its message.
default_group now returns Default for tags: [""] where the base fell through to the path segment.
Re-export lines (export type { A as X, ... } from) now wrap at 100 columns.

Non-blocking: a few why-comments dropped in e60917d are worth restoring (the Params-suffix collision rationale in plan/naming/fixed.rs, the verbatim-brace-instead-of-panic note in emit/angular/request.rs, the alias-collision note in emit/model/emit_ts_models.rs). The Emitter trait has one impl per target and no second implementation planned, so a plain function would carry less indirection; fine to keep if you prefer the shape.

pkurcx · 2026-09-08T21:22:43Z

-  Yaml = 'yaml'
-}
+export type InputFormat = 'json' | 'yaml';
+export declare const InputFormat: {


This declares a runtime value, but lib/index.js and lib/browser.js do not export InputFormat (or ResponseType). Verified on this head:

node -e "console.log(require('./lib/index.js').InputFormat)" # undefined

A consumer writing inputFormat: InputFormat.Yaml, as website/src/content/docs/reference/node-api.md shows, compiles under strict + isolatedModules and throws at runtime. The old const enum was inlined to 'yaml', so this is a regression rather than the strictly-more-permissive change the description states.

Fix: export frozen mirrors the way EmitTarget already is, re-export them from browser.d.ts, and widen the allow-list in __test__/package.spec.ts (public surface is a fixed allow-list), which currently pins the export set and would have caught this if the enums had been added there.

pkurcx · 2026-09-08T21:22:43Z

+      )?)))
+    }
+    Some("object") => Ok(SchemaType::InlineObject {
+      properties: normalize_properties(schema, walk.item())?,


Depth is now charged twice per inline-object level: walk.item() here, then walk.property(name) inside normalize_properties before normalize_type runs check_depth. The base charged one unit (normalize_properties(.., depth + 1, ..) and then normalize_schema_raw(.., depth, ..) per property), so the effective cap for nested inline objects roughly halves. Arrays, compositions and maps are unaffected. The existing depth test uses arrays only, so it does not catch this. Passing walk instead of walk.item() restores the old accounting and matches walk.rs's contract that each constructor descends exactly one level.

pkurcx · 2026-09-08T21:22:43Z

+/// measured by re-serialising the node tree. A source this cannot parse
+/// passes, leaving the typed parse to report the error.
+fn check_anchor_expansion(source: &str, display_path: &Rc<str>) -> Result<(), Diagnostic> {
+  let Ok(value) = serde_yml::from_str::<serde_yml::Value>(source) else {


Returning Ok when the Value parse fails opens a gap: serde_yml 0.0.12's Value rejects duplicate mapping keys, while the typed parse accepts them last-wins inside the serde_json::Value-typed fields (example, enum). A document with one repeated key inside example plus a fanned-out anchor skips this guard and decodes; the base rejected it (mislabelled, but fatal). serde_yml's own repetition limit still stops large bombs, so the impact is bounded, but the guard is no longer total. Options: fail on a duplicate-key Value error, or fall back to a cheap alias-count check when the Value parse fails.

pkurcx · 2026-09-08T21:22:43Z

+/// under `components.schemas` carries the `duplicate-schema-name` subcode so
+/// consumers can route on it; anything else is a plain decode failure.
+fn decode_failure(message: &str, display_path: &Rc<str>) -> Diagnostic {
+  if message.contains(DUPLICATE_KEY) && message.contains(SCHEMAS_FIELD_PATH) {


contains("components.schemas") also matches nested paths, so components.schemas.Pet.properties: duplicate key 'id' and a repeated discriminator.mapping key are still reported as duplicate-schema-name with "Each schema name must be declared once." Same behaviour as the base, but the description presents the misrouting as fixed. Matching the prefix components.schemas: would make it exact.

Related: for JSON input the same defect surfaces as plain E_INPUT_INVALID with no subcode (serde_json carries no field path), so the subcode is format-dependent. Worth a line in the description at least.

pkurcx · 2026-09-08T21:22:43Z

-# duplicate-mapping-key rejection, which the duplicate-schema-name and
-# mapping-expansion-exceeded diagnostics both read out of serde_yml errors.
+# Pinned: 0.0.13 swapped its YAML backend and dropped the line/column suffix
+# from decode errors, which every `E_INPUT_INVALID` message forwards verbatim.


Checked 0.0.13: it drops the line/column suffix and also the field.path: prefix on decode errors. decode_failure above depends on that prefix to route duplicate-schema-name, so the pin has two reasons now; worth naming both here.

pkurcx · 2026-09-08T21:22:43Z

+    .tags()
+    .first()
+    .map(String::as_str)
+    .or_else(|| ctx.lookup_indexed("pathSegments", 0))


or_else only fires when tags() is empty, so tags: [""] yields Default here where the base fell through to the path segment (Pets for /pets). Tags are copied unfiltered from the spec. Low frequency, but a behaviour change with no test; filtering the tag before or_else restores the old result.

pkurcx · 2026-09-08T21:22:43Z

-    const first = await generate({ inputPath: fixture(name), emit: [...DEFAULT_EMIT] });
-    const second = await generate({ inputPath: fixture(name), emit: [...DEFAULT_EMIT] });
-    const third = await generate({ inputPath: fixture(name), emit: [...DEFAULT_EMIT] });
+    const [first, second, third] = await Promise.all(


Running the three generations concurrently weakens what this test checks: state leaking from run 1 into run 2 is only observable when they run in sequence, and a separate concurrency test already exists further down. Suggest keeping these sequential.

pkurcx · 2026-09-08T21:22:43Z

-    const binding = await load();
+    const [prepared, binding] = await Promise.all([
+      prepareOptions(options, unreachableFetch),
+      load(),


prepareOptions does no I/O in the browser (URL inputs are rejected earlier), so Promise.all gains nothing here, and it changes behaviour slightly: invalid options now trigger the wasm download before failing, and if both reject the first to settle wins. Sequential was simpler and had the stricter ordering. Non-blocking.

pkurcx · 2026-09-08T21:22:43Z

+    [
+      EMIT_TARGET_UNION,
+      'export declare const EmitTarget: {',
+      "  readonly Models: 'models';",


Heads-up for the rebase: main (#8) added a Layout const-enum rewrite to scripts/patch-types.mjs, which this branch deletes. Git will resolve that as a modify/delete conflict and the rewrite is gone unless it is ported here as a fourth pattern.

…e references

…yout

…ed must-use coverage

…h iterator chains

…ode routing

…ents onto one line

Andrzej Świętek added 10 commits September 8, 2026 17:25

Refactored the generator pipeline into explicit abstractions

2223867

Converted the build scripts to TypeScript

4fe8dbc

Added typecheck and test-target clippy to CI

f0f71dc

Pinned every remaining fixture and unified the snapshot manifest

3598e3b

Typed the published runtime and gated it in CI

0999e2f

Trimmed every code comment to its contract

e60917d

Parallelised the independent awaits in the browser entry

ee6f6ef

Replaced the value-building loops with iterator chains

f89bcc1

Fixed the lint job by splitting the fixture-coverage check off the re…

40d3e65

…generator

Renamed the abbreviated modules and bindings and split the long funct…

7d19fd5

…ions

pkurcx self-assigned this Sep 8, 2026

pkurcx reviewed Sep 8, 2026

View reviewed changes

Andrzej Świętek added 7 commits September 9, 2026 23:31

Shortened the doc comments to one sentence and dropped the stale phas…

5c92860

…e references

Merged main and addressed the review findings

21bca48

Extended the review fixes to the analogous paths in the operations la…

1dd0516

…yout

Split the oversized modules, named every diagnostic subcode and widen…

bf5d3a4

…ed must-use coverage

Cut the doc comments to one line and replaced the remaining loops wit…

76e535b

…h iterator chains

Restored the why-clauses the comment pass removed and pinned the subc…

6158819

…ode routing

Renamed the leftover ir bindings to model and folded the wrapped comm…

9fda550

…ents onto one line

Navigation Menu

Uh oh!

Refactored the generator into explicit abstractions and typed the JavaScript surface - #7

Refactored the generator into explicit abstractions and typed the JavaScript surface#7
Andrzej-Swietek wants to merge 17 commits into
AVSystem:mainfrom
Andrzej-Swietek:refactor/pipeline-abstractions

Andrzej-Swietek commented Sep 8, 2026 •

edited

Loading

Uh oh!

pkurcx left a comment

Uh oh!

pkurcx Sep 8, 2026

Uh oh!

pkurcx Sep 8, 2026

Uh oh!

pkurcx Sep 8, 2026

Uh oh!

pkurcx Sep 8, 2026

Uh oh!

pkurcx Sep 8, 2026

Uh oh!

pkurcx Sep 8, 2026

Uh oh!

pkurcx Sep 8, 2026

Uh oh!

pkurcx Sep 8, 2026

Uh oh!

pkurcx Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Andrzej-Swietek commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Behaviour changes

Verified

Uh oh!

pkurcx left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Andrzej-Swietek commented Sep 8, 2026 •

edited

Loading