Feat: Dart client#3315
Draft
harshitsangani wants to merge 2 commits into
Draft
Conversation
Contributor
📝 WalkthroughWalkthroughAdded a new Dart/Flutter client generator to Orval: type-to-Dart resolution, model class generation (fromJson/toJson/copyWith), Dio-based API client emission, package/build configs for a new ChangesDart Client Generator
Sequence Diagram(s)(Skipped — conditions for diagram generation not met.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/orval/src/write-specs.ts (1)
424-434:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExclude skipped schema paths from formatter/hook inputs for Dart
Lines 425-434 still add schema directories even when Line 150 skips writing schemas for Dart. This can trigger noisy formatter/hook behavior on paths not generated in this mode.
Suggested fix
const paths = [ - ...(output.schemas + ...(!isDart && output.schemas ? [ getFileInfo( isString(output.schemas) ? output.schemas : output.schemas.path, ).dirname, ] : []), - ...(output.operationSchemas + ...(!isDart && output.operationSchemas ? [getFileInfo(output.operationSchemas).dirname] : []), ...implementationPaths, ];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/orval/src/write-specs.ts` around lines 424 - 434, The paths array currently adds schema directories unconditionally when output.schemas or output.operationSchemas exist, causing formatter/hooks to run on schema paths that were skipped for Dart; update the paths construction to only include schema directories when the same condition used earlier to decide to write schemas is true (i.e., reuse the boolean/logic that skips schema generation for Dart), so guard the getFileInfo(output.schemas) and getFileInfo(output.operationSchemas) entries with that same check and avoid adding those dirs when schemas were not actually emitted.
🧹 Nitpick comments (2)
packages/dart/src/generate-client.ts (1)
115-138: 💤 Low valueMinor: success-response detection misses common status codes;
requiresAuthandbodyTypeNameare computed but never used.A few small cleanups while you're in this area:
successResp(line 116-117) only looks at'200'and'201'. Specs commonly use'202','204','2xx', ordefault. Consider iterating response keys and picking the first 2xx (ordefault).requiresAuth(line 123) is set on everyOperationInfobut never read bygenerateMethodor anywhere else.bodyTypeName(lines 283, 286 ingenerateMethod) is assigned but never read.If
requiresAuthis intended for a future feature, leaving a TODO is fine; otherwise drop them so the data model reflects what's actually used.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dart/src/generate-client.ts` around lines 115 - 138, The success-response detection in generate-client.ts currently only checks '200' and '201' (successResp); change it to iterate operation.responses keys and select the first key that is a 2xx status (e.g., /^2\d\d$/ or '2xx') or fallback to 'default', then use its content.schema for responseSchema; also remove the unused requiresAuth property from the OperationInfo object (and its earlier computation) and remove the unused bodyTypeName assignments in generateMethod (or replace them with a clear TODO comment if you intend to use them later) so the data model and method generation only contain fields actually consumed.packages/dart/src/dart-types.ts (1)
124-126: 💤 Low value
enumschemas always lowered toString— type safety lost.Every enum-bearing schema becomes a plain
Stringfield with no validation, losing both compile-time checking and Dart-idiomaticenum/sealed-class generation. This may be acceptable for v1, but consider tracking it explicitly so consumers know what to expect.A short TODO comment near this branch (or in the docs) calling out the limitation would help users.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dart/src/dart-types.ts` around lines 124 - 126, The branch in dart-types.ts that returns makeResult({ type: 'String' }) when schema.enum is present discards enum semantics; update this branch to preserve enum metadata (e.g., return makeResult({ type: 'String', kind: 'enum', enumValues: schema.enum })) so downstream code can generate Dart enums or emit a clear fallback, and add a short TODO comment above that branch noting the current limitation and that consumers should expect an enum-metadata-aware result for future v2 generation; ensure references to makeResult and schema.enum are updated accordingly so callers can detect kind:'enum' and handle generation/validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/docs/guides/dart.mdx`:
- Around line 80-89: Add a short note under the Pet.copyWith example explaining
that the current pattern (id: id ?? this.id, name: name ?? this.name, tag: tag
?? this.tag) cannot distinguish a caller explicitly passing null for a nullable
field from passing nothing, so nullable fields cannot be set to null via this
generated copyWith; recommend either documenting that limitation or showing the
alternative sentinel pattern (e.g., using a sentinel parameter like Object? tag
= _unset and checking for the sentinel inside copyWith) so callers can
explicitly assign null to nullable fields.
In `@docs/content/docs/reference/configuration/output.mdx`:
- Line 28: The docs list of supported output.client options is missing three
valid values; update the options array shown for output.client (the line
containing "`angular`, `axios`, `axios-functions`, ...") to include
`angular-query`, `solid-start`, and `solid-query` so the list fully reflects all
supported client values and stays in sync with the implementation.
In `@packages/dart/src/dart-types.ts`:
- Around line 205-270: The function escapeDartFieldName currently defines a
local DART_RESERVED set that duplicates and diverges from the canonical list in
./utils; replace that duplication by importing and using the exported helper
(escapeDartReserved) from ./utils, remove the local DART_RESERVED declaration,
and change escapeDartFieldName to return escapeDartReserved(name) ? `${name}_` :
name (or directly call the helper to produce the escaped name) so all
reserved-word handling is centralized and consistent with utils.ts.
In `@packages/dart/src/generate-client.ts`:
- Around line 302-322: Remove the dead calculations and simplify the returnType:
delete the unused responseType assignment and the isEmptyResponse logic (which
both reference resolveDartType and op.responseSchema) and replace them with a
single line that sets returnType = 'Response<dynamic>'; keep the comment about
typed deserialization if desired so future wiring can reintroduce
resolveDartType/responseType when typed responses are implemented.
- Around line 146-173: groupByTag currently uses a meaningless ternary and
ignores OpenAPI operation.tags; update extractOperations to include
operation.tags on the OperationInfo type, then simplify groupByTag to pick a tag
from the OperationInfo (e.g. use the first entry in op.tags if present), falling
back to getTag(op) and finally to 'Default' if neither yields a value, removing
the broken extractOperations.length/operations.find logic; also adjust
getTag(op: OperationInfo) to return 'Default' (capitalization consistent with
existing code) instead of 'default' so bucket names match, and ensure all call
sites that rely on OperationInfo access the new tags field.
In `@packages/dart/src/generate-models.ts`:
- Around line 41-58: The class name is used verbatim in generateDartClass which
can emit invalid Dart for schema keys containing punctuation or reserved words;
call sanitizeClassName and then escapeDartReserved from ./utils to produce a
safe identifier (e.g., const safeClassName =
escapeDartReserved(sanitizeClassName(className))) and use safeClassName wherever
the class identifier is emitted (class declaration, fromJson/constructor
invocations, type annotations). Leave file names/import paths as they are if you
want stable filenames, but ensure any emitted Dart identifiers use the
sanitized/escaped name; update generateDartClass and any places it writes the
class identifier to reference safeClassName instead of className.
In `@packages/dart/src/index.ts`:
- Around line 25-27: The current outputDir logic treats any trailing ".word" as
an extension (const outputDir = /\.\w+$/.test(nodePath.basename(raw)) ?
nodePath.dirname(raw) : raw;), which wrongly classifies paths like "gen/v1.0" as
files; change the test to only match real filename suffixes (a dot followed by
at least one letter, optionally letters/digits) e.g. use a regex like
/\.[A-Za-z][A-Za-z0-9]*$/ on nodePath.basename(raw) so directories with
version-like segments (v1.0) are not treated as files; update the condition
around outputDir accordingly.
---
Outside diff comments:
In `@packages/orval/src/write-specs.ts`:
- Around line 424-434: The paths array currently adds schema directories
unconditionally when output.schemas or output.operationSchemas exist, causing
formatter/hooks to run on schema paths that were skipped for Dart; update the
paths construction to only include schema directories when the same condition
used earlier to decide to write schemas is true (i.e., reuse the boolean/logic
that skips schema generation for Dart), so guard the getFileInfo(output.schemas)
and getFileInfo(output.operationSchemas) entries with that same check and avoid
adding those dirs when schemas were not actually emitted.
---
Nitpick comments:
In `@packages/dart/src/dart-types.ts`:
- Around line 124-126: The branch in dart-types.ts that returns makeResult({
type: 'String' }) when schema.enum is present discards enum semantics; update
this branch to preserve enum metadata (e.g., return makeResult({ type: 'String',
kind: 'enum', enumValues: schema.enum })) so downstream code can generate Dart
enums or emit a clear fallback, and add a short TODO comment above that branch
noting the current limitation and that consumers should expect an
enum-metadata-aware result for future v2 generation; ensure references to
makeResult and schema.enum are updated accordingly so callers can detect
kind:'enum' and handle generation/validation.
In `@packages/dart/src/generate-client.ts`:
- Around line 115-138: The success-response detection in generate-client.ts
currently only checks '200' and '201' (successResp); change it to iterate
operation.responses keys and select the first key that is a 2xx status (e.g.,
/^2\d\d$/ or '2xx') or fallback to 'default', then use its content.schema for
responseSchema; also remove the unused requiresAuth property from the
OperationInfo object (and its earlier computation) and remove the unused
bodyTypeName assignments in generateMethod (or replace them with a clear TODO
comment if you intend to use them later) so the data model and method generation
only contain fields actually consumed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba7b1934-f202-4ddb-a917-a4e6b1d4fbde
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
docs/content/docs/guides/dart.mdxdocs/content/docs/guides/meta.jsondocs/content/docs/reference/configuration/output.mdxorval.dart.config.tspackages/core/src/types.tspackages/dart/package.jsonpackages/dart/src/dart-types.tspackages/dart/src/generate-client.tspackages/dart/src/generate-models.tspackages/dart/src/index.tspackages/dart/src/utils.tspackages/dart/tsconfig.build.jsonpackages/dart/tsconfig.jsonpackages/dart/tsdown.config.tspackages/orval/package.jsonpackages/orval/src/client.tspackages/orval/src/write-specs.ts
Comment on lines
+80
to
+89
| Pet copyWith({ | ||
| int? id, | ||
| String? name, | ||
| String? tag, | ||
| }) { | ||
| return Pet( | ||
| id: id ?? this.id, | ||
| name: name ?? this.name, | ||
| tag: tag ?? this.tag, | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching Dart generator/model templates for copyWith nullable fallback patterns..."
rg -n -C3 "copyWith|\\?\\? this\\." --type ts --type js --type md --type dart
echo
echo "Looking specifically for nullable params in copyWith signatures near fallback usage..."
rg -n -C4 "copyWith\\(|\\w+\\?\\s+\\w+|\\?\\? this\\." --type dart --type tsRepository: orval-labs/orval
Length of output: 2934
Document the copyWith limitation for nullable fields
The example (lines 80-89) correctly reflects what the Dart generator produces, but doesn't explain that the ?? this.field pattern prevents callers from explicitly setting nullable fields to null. Add a note clarifying this limitation, or show an alternative pattern using a sentinel parameter (e.g., Object? tag = _unset) to allow null assignment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/content/docs/guides/dart.mdx` around lines 80 - 89, Add a short note
under the Pet.copyWith example explaining that the current pattern (id: id ??
this.id, name: name ?? this.name, tag: tag ?? this.tag) cannot distinguish a
caller explicitly passing null for a nullable field from passing nothing, so
nullable fields cannot be set to null via this generated copyWith; recommend
either documenting that limitation or showing the alternative sentinel pattern
(e.g., using a sentinel parameter like Object? tag = _unset and checking for the
sentinel inside copyWith) so callers can explicitly assign null to nullable
fields.
| **Type:** `String | Function` | ||
| **Default:** `'axios-functions'` | ||
| **Options:** `angular`, `axios`, `axios-functions`, `react-query`, `svelte-query`, `vue-query`, `swr`, `zod`, `hono`, `fetch`, `mcp` | ||
| **Options:** `angular`, `axios`, `axios-functions`, `react-query`, `svelte-query`, `vue-query`, `swr`, `zod`, `hono`, `fetch`, `mcp`, `dart` |
Contributor
There was a problem hiding this comment.
Keep the output.client options list fully in sync
Line 28 is still missing valid options (angular-query, solid-start, solid-query), so users won’t see all supported client values.
Suggested fix
-**Options:** `angular`, `axios`, `axios-functions`, `react-query`, `svelte-query`, `vue-query`, `swr`, `zod`, `hono`, `fetch`, `mcp`, `dart`
+**Options:** `angular`, `angular-query`, `axios`, `axios-functions`, `react-query`, `solid-start`, `solid-query`, `svelte-query`, `vue-query`, `swr`, `zod`, `hono`, `fetch`, `mcp`, `dart`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **Options:** `angular`, `axios`, `axios-functions`, `react-query`, `svelte-query`, `vue-query`, `swr`, `zod`, `hono`, `fetch`, `mcp`, `dart` | |
| **Options:** `angular`, `angular-query`, `axios`, `axios-functions`, `react-query`, `solid-start`, `solid-query`, `svelte-query`, `vue-query`, `swr`, `zod`, `hono`, `fetch`, `mcp`, `dart` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/content/docs/reference/configuration/output.mdx` at line 28, The docs
list of supported output.client options is missing three valid values; update
the options array shown for output.client (the line containing "`angular`,
`axios`, `axios-functions`, ...") to include `angular-query`, `solid-start`, and
`solid-query` so the list fully reflects all supported client values and stays
in sync with the implementation.
Comment on lines
+205
to
+270
| function escapeDartFieldName(name: string): string { | ||
| const DART_RESERVED = new Set([ | ||
| 'abstract', | ||
| 'as', | ||
| 'assert', | ||
| 'async', | ||
| 'await', | ||
| 'break', | ||
| 'case', | ||
| 'catch', | ||
| 'class', | ||
| 'const', | ||
| 'continue', | ||
| 'default', | ||
| 'do', | ||
| 'dynamic', | ||
| 'else', | ||
| 'enum', | ||
| 'export', | ||
| 'extends', | ||
| 'extension', | ||
| 'external', | ||
| 'factory', | ||
| 'false', | ||
| 'final', | ||
| 'finally', | ||
| 'for', | ||
| 'get', | ||
| 'if', | ||
| 'implements', | ||
| 'import', | ||
| 'in', | ||
| 'interface', | ||
| 'is', | ||
| 'late', | ||
| 'library', | ||
| 'mixin', | ||
| 'new', | ||
| 'null', | ||
| 'on', | ||
| 'operator', | ||
| 'part', | ||
| 'required', | ||
| 'rethrow', | ||
| 'return', | ||
| 'sealed', | ||
| 'set', | ||
| 'show', | ||
| 'static', | ||
| 'super', | ||
| 'switch', | ||
| 'sync', | ||
| 'this', | ||
| 'throw', | ||
| 'true', | ||
| 'try', | ||
| 'typedef', | ||
| 'var', | ||
| 'void', | ||
| 'when', | ||
| 'while', | ||
| 'with', | ||
| 'yield', | ||
| ]); | ||
| return DART_RESERVED.has(name) ? `${name}_` : name; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Reuse escapeDartReserved from ./utils instead of redefining a (divergent) reserved-word set.
This local DART_RESERVED duplicates the one in utils.ts and also drifts — it is missing covariant, deferred, Function, and hide. As a result, schema fields named e.g. function, hide, or covariant will emit identifiers that are reserved/built-in in Dart and break compilation, while utils.ts correctly escapes them.
Import the exported helper and delete the local copy to keep one source of truth.
♻️ Proposed fix
-import { toCamelCase, toSnakeCase } from './utils';
+import { escapeDartReserved, toCamelCase, toSnakeCase } from './utils';
@@
-function escapeDartFieldName(name: string): string {
- const DART_RESERVED = new Set([
- 'abstract',
- ...
- 'yield',
- ]);
- return DART_RESERVED.has(name) ? `${name}_` : name;
-}
+// Use the shared helper from ./utils
@@
- const name = escapeDartFieldName(toCamelCase(jsonName));
+ const name = escapeDartReserved(toCamelCase(jsonName));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/dart-types.ts` around lines 205 - 270, The function
escapeDartFieldName currently defines a local DART_RESERVED set that duplicates
and diverges from the canonical list in ./utils; replace that duplication by
importing and using the exported helper (escapeDartReserved) from ./utils,
remove the local DART_RESERVED declaration, and change escapeDartFieldName to
return escapeDartReserved(name) ? `${name}_` : name (or directly call the helper
to produce the escaped name) so all reserved-word handling is centralized and
consistent with utils.ts.
Comment on lines
+146
to
+173
| function groupByTag( | ||
| operations: OperationInfo[], | ||
| ): Record<string, OperationInfo[]> { | ||
| const spec_operations = operations.reduce( | ||
| (acc, op) => { | ||
| const tag = | ||
| extractOperations.length > 0 | ||
| ? operations.find((o) => o === op) | ||
| ? getTag(op) | ||
| : 'Default' | ||
| : 'Default'; | ||
| if (!acc[tag]) acc[tag] = []; | ||
| acc[tag].push(op); | ||
| return acc; | ||
| }, | ||
| {} as Record<string, OperationInfo[]>, | ||
| ); | ||
| return spec_operations; | ||
| } | ||
|
|
||
| function getTag(op: OperationInfo): string { | ||
| // Derive tag from path: /api/{module}/... → module | ||
| const parts = op.path.split('/').filter(Boolean); | ||
| if (parts.length >= 2 && parts[0] === 'api') { | ||
| return toCamelCase(parts[1]); | ||
| } | ||
| return 'default'; | ||
| } |
Contributor
There was a problem hiding this comment.
groupByTag logic is broken — the ternary reduces to dead code, and OpenAPI tags are ignored.
Two distinct problems compound here:
- The condition is meaningless.
extractOperations.lengthreads the function's parameter count (extractOperationsis the imported function declared with one parameter, so.length === 1), not anything about operations. Andoperations.find((o) => o === op)will always returnopbecauseopis iterated fromoperations. The whole expression unconditionally falls through togetTag(op), so the ternary is just confusing dead code. getTagderives the tag frompath.split('/')[1]only when the path starts with/api/.... Specs that don't follow that exact prefix get bucketed under'default', and cruciallyoperation.tags(the canonical OpenAPI grouping mechanism, already declared onOperationObject) is never consulted.
For most real-world specs, every operation will end up in a single default bucket, producing one giant DefaultApi class.
🛠️ Proposed fix
-function groupByTag(
- operations: OperationInfo[],
-): Record<string, OperationInfo[]> {
- const spec_operations = operations.reduce(
- (acc, op) => {
- const tag =
- extractOperations.length > 0
- ? operations.find((o) => o === op)
- ? getTag(op)
- : 'Default'
- : 'Default';
- if (!acc[tag]) acc[tag] = [];
- acc[tag].push(op);
- return acc;
- },
- {} as Record<string, OperationInfo[]>,
- );
- return spec_operations;
-}
+function groupByTag(
+ operations: OperationInfo[],
+): Record<string, OperationInfo[]> {
+ return operations.reduce<Record<string, OperationInfo[]>>((acc, op) => {
+ const tag = op.tags?.[0] ?? getTag(op);
+ (acc[tag] ??= []).push(op);
+ return acc;
+ }, {});
+}This requires propagating operation.tags into OperationInfo in extractOperations (currently dropped).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/generate-client.ts` around lines 146 - 173, groupByTag
currently uses a meaningless ternary and ignores OpenAPI operation.tags; update
extractOperations to include operation.tags on the OperationInfo type, then
simplify groupByTag to pick a tag from the OperationInfo (e.g. use the first
entry in op.tags if present), falling back to getTag(op) and finally to
'Default' if neither yields a value, removing the broken
extractOperations.length/operations.find logic; also adjust getTag(op:
OperationInfo) to return 'Default' (capitalization consistent with existing
code) instead of 'default' so bucket names match, and ensure all call sites that
rely on OperationInfo access the new tags field.
Comment on lines
+302
to
+322
| // response type (currently returns Response<dynamic>; refs not imported | ||
| // to avoid unused-import warnings until typed deserialization is added) | ||
| let responseType = 'dynamic'; | ||
| if (op.responseSchema) { | ||
| const tr = resolveDartType(op.responseSchema); | ||
| responseType = tr.type; | ||
| } | ||
|
|
||
| // check if response is empty schema | ||
| const isEmptyResponse = | ||
| !op.responseSchema || | ||
| (op.responseSchema && | ||
| !op.responseSchema.$ref && | ||
| !op.responseSchema.type && | ||
| !op.responseSchema.anyOf && | ||
| !op.responseSchema.oneOf && | ||
| !op.responseSchema.allOf); | ||
|
|
||
| const returnType = isEmptyResponse | ||
| ? 'Response<dynamic>' | ||
| : `Response<dynamic>`; |
Contributor
There was a problem hiding this comment.
Dead code: responseType is unused and the isEmptyResponse ternary returns the same value in both branches.
Lines 304-308 compute responseType from op.responseSchema but the value is never referenced. Lines 311-322 then compute isEmptyResponse for a ternary whose branches are both 'Response<dynamic>', so the entire block has no effect on the generated code.
Either wire the typed response through (matching the comment on lines 302-303 about "until typed deserialization is added") or remove the unused calculations to keep the generator easy to follow.
🧹 Proposed cleanup until typed deserialization lands
- // response type (currently returns Response<dynamic>; refs not imported
- // to avoid unused-import warnings until typed deserialization is added)
- let responseType = 'dynamic';
- if (op.responseSchema) {
- const tr = resolveDartType(op.responseSchema);
- responseType = tr.type;
- }
-
- // check if response is empty schema
- const isEmptyResponse =
- !op.responseSchema ||
- (op.responseSchema &&
- !op.responseSchema.$ref &&
- !op.responseSchema.type &&
- !op.responseSchema.anyOf &&
- !op.responseSchema.oneOf &&
- !op.responseSchema.allOf);
-
- const returnType = isEmptyResponse
- ? 'Response<dynamic>'
- : `Response<dynamic>`;
+ // TODO: emit a typed Response<T> once typed deserialization is wired in.
+ const returnType = 'Response<dynamic>';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/generate-client.ts` around lines 302 - 322, Remove the dead
calculations and simplify the returnType: delete the unused responseType
assignment and the isEmptyResponse logic (which both reference resolveDartType
and op.responseSchema) and replace them with a single line that sets returnType
= 'Response<dynamic>'; keep the comment about typed deserialization if desired
so future wiring can reintroduce resolveDartType/responseType when typed
responses are implemented.
Comment on lines
+41
to
+58
| function generateDartClass( | ||
| className: string, | ||
| schema: SchemaLike, | ||
| allSchemaNames: string[], | ||
| ): string { | ||
| const fields = resolveSchemaFields(schema); | ||
| const imports = collectImports(fields, className, allSchemaNames); | ||
|
|
||
| let out = ''; | ||
|
|
||
| // imports | ||
| for (const imp of imports) { | ||
| out += `import '${imp}.dart';\n`; | ||
| } | ||
| if (imports.length > 0) out += '\n'; | ||
|
|
||
| // class declaration | ||
| out += `class ${className} {\n`; |
Contributor
There was a problem hiding this comment.
Class name is not sanitized — non-identifier schema names will produce invalid Dart.
OpenAPI schema keys can legally contain characters that are not valid in a Dart identifier (e.g., ., -, <, >), and could also coincide with reserved words. Inserting name verbatim into class ${className} { and into the fromJson/constructor invocations will then emit code that won't compile. The sanitizeClassName helper already exists in ./utils (and escapeDartReserved for the keyword case).
Apply the same sanitization here that the API client already does for tag-derived class names.
🛠️ Proposed fix
-import { toSnakeCase } from './utils';
+import { escapeDartReserved, sanitizeClassName, toSnakeCase } from './utils';
@@
for (const [name, schema] of Object.entries(schemas)) {
- const content = generateDartClass(name, schema, schemaNames);
+ const safeName = escapeDartReserved(sanitizeClassName(name));
+ const content = generateDartClass(safeName, schema, schemaNames);
const fileName = toSnakeCase(name) + '.dart';(File name remains derived from the original key for stable cross-references.)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/generate-models.ts` around lines 41 - 58, The class name is
used verbatim in generateDartClass which can emit invalid Dart for schema keys
containing punctuation or reserved words; call sanitizeClassName and then
escapeDartReserved from ./utils to produce a safe identifier (e.g., const
safeClassName = escapeDartReserved(sanitizeClassName(className))) and use
safeClassName wherever the class identifier is emitted (class declaration,
fromJson/constructor invocations, type annotations). Leave file names/import
paths as they are if you want stable filenames, but ensure any emitted Dart
identifiers use the sanitized/escaped name; update generateDartClass and any
places it writes the class identifier to reference safeClassName instead of
className.
Comment on lines
+113
to
+129
| function generateCopyWith(className: string, fields: DartField[]): string { | ||
| if (fields.length === 0) return ''; | ||
|
|
||
| let out = ` ${className} copyWith({\n`; | ||
| for (const f of fields) { | ||
| const paramType = f.dartType === 'dynamic' ? 'dynamic' : `${f.dartType}?`; | ||
| out += ` ${paramType} ${f.name},\n`; | ||
| } | ||
| out += ' }) {\n'; | ||
| out += ` return ${className}(\n`; | ||
| for (const f of fields) { | ||
| out += ` ${f.name}: ${f.name} ?? this.${f.name},\n`; | ||
| } | ||
| out += ' );\n'; | ||
| out += ' }\n'; | ||
| return out; | ||
| } |
Contributor
There was a problem hiding this comment.
copyWith cannot reset nullable fields to null.
The generated body ${name} ?? this.${name} makes it impossible for callers to distinguish "leave field unchanged" from "set field to null" on nullable fields — passing null to clear a value is silently ignored. This is a well-known Dart copyWith pitfall and tends to surface as subtle bugs in user code.
Common fixes: emit a per-field sentinel (e.g., Object? name = _undefined) and only override when not the sentinel, or generate copyWith only for non-nullable fields and a separate setter API for clearing nullable fields. Worth resolving before users build on top of this generator.
Comment on lines
+25
to
+27
| const outputDir = /\.\w+$/.test(nodePath.basename(raw)) | ||
| ? nodePath.dirname(raw) | ||
| : raw; |
Contributor
There was a problem hiding this comment.
Narrow extension stripping to real file suffixes
Line 25 currently treats any trailing .word as a file extension. Paths like gen/v1.0 would be treated as files, so generated output lands in the wrong directory.
Suggested fix
- const outputDir = /\.\w+$/.test(nodePath.basename(raw))
+ const outputDir = /\.(?:dart|[cm]?ts|[cm]?js)$/i.test(nodePath.basename(raw))
? nodePath.dirname(raw)
: raw;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const outputDir = /\.\w+$/.test(nodePath.basename(raw)) | |
| ? nodePath.dirname(raw) | |
| : raw; | |
| const outputDir = /\.(?:dart|[cm]?ts|[cm]?js)$/i.test(nodePath.basename(raw)) | |
| ? nodePath.dirname(raw) | |
| : raw; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/index.ts` around lines 25 - 27, The current outputDir logic
treats any trailing ".word" as an extension (const outputDir =
/\.\w+$/.test(nodePath.basename(raw)) ? nodePath.dirname(raw) : raw;), which
wrongly classifies paths like "gen/v1.0" as files; change the test to only match
real filename suffixes (a dot followed by at least one letter, optionally
letters/digits) e.g. use a regex like /\.[A-Za-z][A-Za-z0-9]*$/ on
nodePath.basename(raw) so directories with version-like segments (v1.0) are not
treated as files; update the condition around outputDir accordingly.
melloware
requested changes
May 5, 2026
Collaborator
melloware
left a comment
melloware
left a comment
There was a problem hiding this comment.
Please see all AI feedback and also you need to update the README.MD with the Dart Client
Add a new `@orval/dart` package that generates Dart model classes and Dio-based API client code from an OpenAPI specification. - Add `DART` to `OutputClient` enum in `@orval/core` - Implement Dart type mapper with support for nullable (`anyOf`), `$ref`, arrays, DateTime, binary, and mixed types - Generate model classes with `fromJson`, `toJson`, and `copyWith` - Generate Dio API client classes grouped by URL path segment - Wire `@orval/dart` into the main `orval` package client resolver - Skip TypeScript schema/implementation output when client is `dart` - Add example config (`orval.dart.config.ts`)
- Add guides/dart.mdx with configuration, generated output examples, type mapping table, and Flutter usage instructions - Add dart to the Frameworks section in guides/meta.json - Add dart to the client options list in reference/configuration/output.mdx
melloware
force-pushed
the
feat/dart-client
branch
from
8ef044c to
a64d8f5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (4)
docs/content/docs/reference/configuration/output.mdx (1)
28-28:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocumented
output.clientoptions are still incomplete.The list omits
angular-query,solid-start, andsolid-query, all of which are validOutputClientvalues inpackages/core/src/types.ts. Please align this list with the enum.Suggested fix
-**Options:** `angular`, `axios`, `axios-functions`, `react-query`, `svelte-query`, `vue-query`, `swr`, `zod`, `hono`, `fetch`, `mcp`, `dart` +**Options:** `angular`, `angular-query`, `axios`, `axios-functions`, `react-query`, `solid-start`, `solid-query`, `svelte-query`, `vue-query`, `swr`, `zod`, `hono`, `fetch`, `mcp`, `dart`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/docs/reference/configuration/output.mdx` at line 28, The docs list of supported output.client options is missing values present in the OutputClient enum; update the documentation entry in docs/content/docs/reference/configuration/output.mdx (the bolded Options list) to include the missing enum members: angular-query, solid-start, and solid-query so the documented options match packages/core/src/types.ts::OutputClient.docs/content/docs/guides/dart.mdx (1)
80-90:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocument that
copyWithcannot set a nullable field back tonull.The generated body uses
id: id ?? this.id(and friends) for every field including the optional ones, so callers cannot distinguish "leave unchanged" from "explicitlynull". For nullable fields that's a real loss of expressiveness. Either call this out in the guide or note the sentinel-parameter alternative (e.g.Object? tag = _unset) so users aren't surprised.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/docs/guides/dart.mdx` around lines 80 - 90, The docs show a copyWith implementation (function copyWith in the Pet example) that uses patterns like id: id ?? this.id which prevents callers from distinguishing "leave unchanged" from "explicitly set null" for nullable fields; update the guide text around the Pet.copyWith snippet to explicitly document this limitation and show the recommended fix: either call out that nullable fields cannot be set back to null with the shown implementation or demonstrate the sentinel-parameter pattern (e.g. use unique sentinel default parameters such as Object? tag = _unset and check for _unset inside copyWith) so users know how to express "set to null" versus "no change".packages/dart/src/generate-client.ts (2)
302-322:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDead code:
responseTypeis unused andisEmptyResponsereturns the same value in both branches.
responseTypeis computed but never referenced, and thereturnTypeternary evaluates to'Response<dynamic>'on both arms — the entire block has no effect on the generated code. Either wire the typed response through (matching the comment about typed deserialization) or drop the unused calculations.🧹 Proposed cleanup until typed deserialization lands
- // response type (currently returns Response<dynamic>; refs not imported - // to avoid unused-import warnings until typed deserialization is added) - let responseType = 'dynamic'; - if (op.responseSchema) { - const tr = resolveDartType(op.responseSchema); - responseType = tr.type; - } - - // check if response is empty schema - const isEmptyResponse = - !op.responseSchema || - (op.responseSchema && - !op.responseSchema.$ref && - !op.responseSchema.type && - !op.responseSchema.anyOf && - !op.responseSchema.oneOf && - !op.responseSchema.allOf); - - const returnType = isEmptyResponse - ? 'Response<dynamic>' - : `Response<dynamic>`; + // TODO: emit a typed Response<T> once typed deserialization is wired in. + const returnType = 'Response<dynamic>';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dart/src/generate-client.ts` around lines 302 - 322, The block computes responseType (via resolveDartType) and isEmptyResponse but never uses them; simplify by removing those unused variables and set returnType directly to 'Response<dynamic>' (replace the responseType/isEmptyResponse logic and remove resolveDartType/op.responseSchema-related code) so the generated code is no-op until typed deserialization is implemented.
146-173:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
groupByTagternary still reduces to dead code and OpenAPItagsare still ignored.The ternary's first branch
extractOperations.length > 0reads the imported function's parameter count (always1), andoperations.find((o) => o === op)always returnsopbecauseopis iterated fromoperations. The whole expression unconditionally falls through togetTag(op). Combined withgetTagonly inspectingpath.split('/')[1]when the path begins with/api/..., real-world specs end up bucketed into a single class, andoperation.tags(already on the OpenAPI object and parsed intoOperationInfo-equivalent fields elsewhere in the PR) is never consulted.🛠️ Suggested fix
-function groupByTag( - operations: OperationInfo[], -): Record<string, OperationInfo[]> { - const spec_operations = operations.reduce( - (acc, op) => { - const tag = - extractOperations.length > 0 - ? operations.find((o) => o === op) - ? getTag(op) - : 'Default' - : 'Default'; - if (!acc[tag]) acc[tag] = []; - acc[tag].push(op); - return acc; - }, - {} as Record<string, OperationInfo[]>, - ); - return spec_operations; -} +function groupByTag( + operations: OperationInfo[], +): Record<string, OperationInfo[]> { + return operations.reduce<Record<string, OperationInfo[]>>((acc, op) => { + const tag = op.tags?.[0] ?? getTag(op); + (acc[tag] ??= []).push(op); + return acc; + }, {}); +}This requires propagating
operation.tagsintoOperationInfoinextractOperations(currently dropped).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dart/src/generate-client.ts` around lines 146 - 173, groupByTag is ignoring OpenAPI operation.tags and the existing ternary is effectively dead; update extractOperations to carry operation.tags into OperationInfo, then change groupByTag to use op.tags (e.g. use the first tag or iterate tags to assign the operation to each tag) falling back to getTag(op) or 'default' when tags are missing; also normalize the casing between 'default' and 'Default' so getTag and groupByTag return the same fallback string. Ensure you update references to OperationInfo, groupByTag, getTag, and extractOperations accordingly.
🧹 Nitpick comments (2)
orval.dart.config.ts (1)
1-12: 💤 Low valueConsider wrapping with
defineConfigfor type-safety/consistency.All other in-repo example configs and the new Dart guide use
defineConfigfromorval. Using it here would catch typos in keys likeclient/modeat edit time.♻️ Proposed change
-export default { +import { defineConfig } from 'orval'; + +export default defineConfig({ dartApi: { input: { target: 'http://localhost:8000/openapi.json', }, output: { target: 'lib/generated/', client: 'dart', mode: 'single', }, }, -}; +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orval.dart.config.ts` around lines 1 - 12, The config is exported as a plain object which loses Orval typing; import defineConfig from 'orval' and wrap the existing default export object with defineConfig(...) (preserving keys like dartApi, input.target, output.target, client, mode) so TypeScript validates keys such as client/mode and catches typos at edit time.docs/content/docs/guides/dart.mdx (1)
161-180: 💤 Low valueUsage example imports a path that the generator doesn't emit.
The "Generated Structure" section (lines 34-46) lists
generated.dartas the top-level barrel, but the usage snippet importspackage:my_flutter_package/my_flutter_package.dartwithout explaining that this is user-sidelib/my_flutter_package.dartre-exporting the generated barrel. Either point users at the actual generated barrel (package:my_flutter_package/src/generated/generated.dart) or add a one-liner showing the user-side barrel that re-exports it; otherwise a copy-paste of the example won't compile.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/docs/guides/dart.mdx` around lines 161 - 180, The usage snippet imports package:my_flutter_package/my_flutter_package.dart but the generator emits a top-level barrel named generated.dart (src/generated/generated.dart); update the docs by either (A) changing the import in the "Usage in Flutter" example to import package:my_flutter_package/src/generated/generated.dart (referencing generated.dart) or (B) add a one-line note and example showing the user-side barrel lib/my_flutter_package.dart that simply re-exports src/generated/generated.dart (so the existing import package:my_flutter_package/my_flutter_package.dart works); modify the PetsApi usage snippet accordingly so a copy-paste will compile.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/dart/src/dart-types.ts`:
- Around line 3-27: Add a nullable?: boolean to the SchemaLike interface and
update the schema-type parsing so it recognizes both OpenAPI 3.0 nullable: true
and OpenAPI 3.1 type arrays that include "null": when inspecting schema.type (in
the logic used by resolveSchemaFields or the code path at/around where
schema.type[0] is used), if schema.type is an array check for the presence of
"null" and set a nullable flag accordingly, then pick the non-"null" string as
the actual type; also if schema.nullable === true set nullable as well. Ensure
downstream type rendering uses this combined nullable flag so required fields
that are nullable become e.g. String? instead of String.
- Around line 306-319: The list-item mapping currently uses a generic cast `(e)
=> e as ${itemTr.type}`, which fails for JSON integers when the list item type
is double; update the branch that builds mapExpr inside the tr.isList block to
special-case itemTr being double (e.g., itemTr.isDouble or itemTr.type ===
'double') and set mapExpr to `(e) => (e as num).toDouble()` so list items are
converted like the scalar double path; keep the existing checks for
itemTr.isReference, itemTr.isDateTime, and itemTr.isDateOnly, and leave nullable
handling and the final cast construction (`(${accessor} as
List<dynamic>).map(${mapExpr}).toList()`) unchanged.
In `@packages/dart/src/generate-client.ts`:
- Around line 325-328: The loop that builds dartPath using pathSubstitutions
only calls String.prototype.replace, which only replaces the first occurrence;
update the substitution in the block that iterates over pathSubstitutions (the
dartPath construction using dartPath.replace(`{${paramName}}`, `\$${dartName}`))
to use replaceAll (or a global-regex equivalent) so every `{paramName}` is
replaced (e.g., dartPath = dartPath.replaceAll(`{${paramName}}`,
`\$${dartName}`) or dartPath = dartPath.replace(new RegExp(`\\{${paramName}\\}`,
'g'), `\$${dartName}`)).
- Around line 116-121: The current logic only checks operation.responses['200']
and ['201'] which ignores other 2xx codes; update the selection of success
response used to set responseSchema (the block that assigns successResp and
responseSchema) to instead iterate operation.responses keys and pick the first
response whose status code matches the regex /^2\d{2}$/ (or fall back to a
'default' response if present), then extract the content schema from that
response; ensure you still handle missing content gracefully when setting
responseSchema.
- Around line 232-244: The current dedup logic in generateMethod uses
deriveMethodName(op) and does a single check against usedNames, appending
op.httpMethod once, which can still collide; change generateMethod to loop and
append a stable disambiguator (e.g., numeric counter) until a unique methodName
is found: compute baseName = deriveMethodName(op), try baseName, then baseName +
httpMethodSuffix, then baseName + httpMethodSuffix + counter (or baseName +
counter) incrementing counter until usedNames.has(methodName) is false, then
usedNames.add(methodName) and return; update references to methodName and keep
refs logic unchanged.
- Around line 248-279: The code constructs Dart identifiers for path/query
parameters without escaping reserved words; update the parameter name handling
to run generated names through escapeDartReserved from utils.ts: when creating
dartName for path params (used with resolveDartType, toCamelCase, params, and
pathSubstitutions) and for query params (used with resolveDartType, toCamelCase,
params, and queryParamNames), replace the raw toCamelCase(p.name) with
escapeDartReserved(toCamelCase(p.name)) so emitted identifiers are safe; also
apply the same escape to any generated identifiers for body/formData keys
elsewhere in this file to ensure all generated Dart identifiers are escaped
consistently.
- Around line 368-378: The body serialization check currently inspects
op.requestBodySchema?.$ref and misses array/oneOf/allOf wrapped refs; replace
that raw-schema check with the resolved type info you already computed via
resolveDartType (the variable tr) and bodyParamName: if tr.isReference OR
(tr.isList && tr.listItemType?.isReference) (and similarly handle any resolved
union/allOf indicators from tr if present) emit a serialized form (call toJson
on the model or map list items to toJson) instead of emitting the raw body,
otherwise emit data: bodyParamName as before; update the logic around
resolveDartType/tr and bodyParamName so multipart handling remains unchanged.
In `@packages/dart/src/utils.ts`:
- Around line 31-50: sanitizeTagName and sanitizeClassName can return
identifiers that start with a digit (e.g., "123Stuff"), which is invalid in
Dart; update both functions (sanitizeTagName and sanitizeClassName) to detect if
the resulting string's first character is a digit and, if so, prepend a safe
prefix (for example an underscore) before returning, ensuring the rest of the
existing transformation logic remains unchanged so existing
capitalization/formatting is preserved.
- Around line 120-125: Remove the dead exports in utils.ts: delete the
escapeDartReserved function and the DART_RESERVED constant since Dart reserved
handling is already implemented in escapeDartFieldName in dart-types.ts; update
any exports list in utils.ts to not export these symbols and run project-wide
search to confirm no remaining imports reference escapeDartReserved or
DART_RESERVED before committing.
---
Duplicate comments:
In `@docs/content/docs/guides/dart.mdx`:
- Around line 80-90: The docs show a copyWith implementation (function copyWith
in the Pet example) that uses patterns like id: id ?? this.id which prevents
callers from distinguishing "leave unchanged" from "explicitly set null" for
nullable fields; update the guide text around the Pet.copyWith snippet to
explicitly document this limitation and show the recommended fix: either call
out that nullable fields cannot be set back to null with the shown
implementation or demonstrate the sentinel-parameter pattern (e.g. use unique
sentinel default parameters such as Object? tag = _unset and check for _unset
inside copyWith) so users know how to express "set to null" versus "no change".
In `@docs/content/docs/reference/configuration/output.mdx`:
- Line 28: The docs list of supported output.client options is missing values
present in the OutputClient enum; update the documentation entry in
docs/content/docs/reference/configuration/output.mdx (the bolded Options list)
to include the missing enum members: angular-query, solid-start, and solid-query
so the documented options match packages/core/src/types.ts::OutputClient.
In `@packages/dart/src/generate-client.ts`:
- Around line 302-322: The block computes responseType (via resolveDartType) and
isEmptyResponse but never uses them; simplify by removing those unused variables
and set returnType directly to 'Response<dynamic>' (replace the
responseType/isEmptyResponse logic and remove
resolveDartType/op.responseSchema-related code) so the generated code is no-op
until typed deserialization is implemented.
- Around line 146-173: groupByTag is ignoring OpenAPI operation.tags and the
existing ternary is effectively dead; update extractOperations to carry
operation.tags into OperationInfo, then change groupByTag to use op.tags (e.g.
use the first tag or iterate tags to assign the operation to each tag) falling
back to getTag(op) or 'default' when tags are missing; also normalize the casing
between 'default' and 'Default' so getTag and groupByTag return the same
fallback string. Ensure you update references to OperationInfo, groupByTag,
getTag, and extractOperations accordingly.
---
Nitpick comments:
In `@docs/content/docs/guides/dart.mdx`:
- Around line 161-180: The usage snippet imports
package:my_flutter_package/my_flutter_package.dart but the generator emits a
top-level barrel named generated.dart (src/generated/generated.dart); update the
docs by either (A) changing the import in the "Usage in Flutter" example to
import package:my_flutter_package/src/generated/generated.dart (referencing
generated.dart) or (B) add a one-line note and example showing the user-side
barrel lib/my_flutter_package.dart that simply re-exports
src/generated/generated.dart (so the existing import
package:my_flutter_package/my_flutter_package.dart works); modify the PetsApi
usage snippet accordingly so a copy-paste will compile.
In `@orval.dart.config.ts`:
- Around line 1-12: The config is exported as a plain object which loses Orval
typing; import defineConfig from 'orval' and wrap the existing default export
object with defineConfig(...) (preserving keys like dartApi, input.target,
output.target, client, mode) so TypeScript validates keys such as client/mode
and catches typos at edit time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aedb50a1-d76a-44ab-8ad5-95aa24445dbb
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
docs/content/docs/guides/dart.mdxdocs/content/docs/guides/meta.jsondocs/content/docs/reference/configuration/output.mdxorval.dart.config.tspackages/core/src/types.tspackages/dart/package.jsonpackages/dart/src/dart-types.tspackages/dart/src/generate-client.tspackages/dart/src/generate-models.tspackages/dart/src/index.tspackages/dart/src/utils.tspackages/dart/tsconfig.build.jsonpackages/dart/tsconfig.jsonpackages/dart/tsdown.config.tspackages/orval/package.jsonpackages/orval/src/client.tspackages/orval/src/write-specs.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/dart/tsconfig.build.json
- packages/dart/tsdown.config.ts
- packages/dart/src/generate-models.ts
- packages/dart/tsconfig.json
Comment on lines
+3
to
+27
| interface SchemaLike { | ||
| type?: string | string[]; | ||
| format?: string; | ||
| $ref?: string; | ||
| items?: SchemaLike; | ||
| anyOf?: SchemaLike[]; | ||
| oneOf?: SchemaLike[]; | ||
| allOf?: SchemaLike[]; | ||
| properties?: Record<string, SchemaLike>; | ||
| additionalProperties?: boolean | SchemaLike; | ||
| enum?: unknown[]; | ||
| const?: unknown; | ||
| default?: unknown; | ||
| required?: string[]; | ||
| title?: string; | ||
| description?: string; | ||
| minimum?: number; | ||
| maximum?: number; | ||
| exclusiveMinimum?: number; | ||
| exclusiveMaximum?: number; | ||
| minLength?: number; | ||
| maxLength?: number; | ||
| minItems?: number; | ||
| maxItems?: number; | ||
| } |
Contributor
There was a problem hiding this comment.
Two nullable-detection gaps produce incorrect Dart types for required nullable fields.
Gap 1 – OpenAPI 3.0 nullable: true: SchemaLike has no nullable property, so { type: "string", nullable: true } resolves to non-nullable String. Any required field with nullable: true will be emitted as String instead of String?.
Gap 2 – OpenAPI 3.1 type array: Line 129 takes only schema.type[0] and discards the rest. For { type: ["string", "null"] }, the null entry is silently dropped. Again, only required fields are affected because resolveSchemaFields independently marks optional fields nullable.
🛡️ Proposed fix
interface SchemaLike {
type?: string | string[];
+ nullable?: boolean; // OpenAPI 3.0 nullable flag
format?: string;
// ...
}- const typ = Array.isArray(schema.type) ? schema.type[0] : schema.type;
+ const hasNullType =
+ Array.isArray(schema.type) && (schema.type as string[]).includes('null');
+ const oas30Nullable = schema.nullable === true;
+ const nullableOverride = hasNullType || oas30Nullable;
+ const typ = Array.isArray(schema.type) ? schema.type.find((t) => t !== 'null') ?? schema.type[0] : schema.type;
switch (typ) {
case 'string':
if (schema.format === 'date')
- return makeResult({ type: 'DateTime', isDateOnly: true });
+ return makeResult({ type: 'DateTime', isDateOnly: true, nullable: nullableOverride });
- if (schema.format === 'date-time')
+ if (schema.format === 'date-time')
- return makeResult({ type: 'DateTime', isDateTime: true });
+ return makeResult({ type: 'DateTime', isDateTime: true, nullable: nullableOverride });
// ... apply nullableOverride to every makeResult() call belowAlso applies to: 128-153
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/dart-types.ts` around lines 3 - 27, Add a nullable?:
boolean to the SchemaLike interface and update the schema-type parsing so it
recognizes both OpenAPI 3.0 nullable: true and OpenAPI 3.1 type arrays that
include "null": when inspecting schema.type (in the logic used by
resolveSchemaFields or the code path at/around where schema.type[0] is used), if
schema.type is an array check for the presence of "null" and set a nullable flag
accordingly, then pick the non-"null" string as the actual type; also if
schema.nullable === true set nullable as well. Ensure downstream type rendering
uses this combined nullable flag so required fields that are nullable become
e.g. String? instead of String.
Comment on lines
+306
to
+319
| if (tr.isList && tr.listItemType) { | ||
| const itemTr = tr.listItemType; | ||
| let mapExpr: string; | ||
| if (itemTr.isReference) { | ||
| mapExpr = `(e) => ${itemTr.type}.fromJson(e as Map<String, dynamic>)`; | ||
| } else if (itemTr.isDateTime || itemTr.isDateOnly) { | ||
| mapExpr = `(e) => DateTime.parse(e as String)`; | ||
| } else { | ||
| mapExpr = `(e) => e as ${itemTr.type}`; | ||
| } | ||
| const cast = `(${accessor} as List<dynamic>).map(${mapExpr}).toList()`; | ||
| if (nullable) return `${accessor} != null ? ${cast} : null`; | ||
| return cast; | ||
| } |
Contributor
There was a problem hiding this comment.
List<double> generates a failing cast: (e) => e as double.
Dart's json.decode throws type 'int' is not a subtype of type 'double' in type cast when a JSON number with no decimal (e.g., 1 or 217) is decoded — it becomes an int, not a double. The scalar double path already handles this correctly at lines 335–339 with (accessor as num).toDouble(), but the list-item branch falls through to the generic (e) => e as ${itemTr.type} at line 314.
🐛 Proposed fix
} else if (itemTr.isDateTime || itemTr.isDateOnly) {
mapExpr = `(e) => DateTime.parse(e as String)`;
+ } else if (itemTr.type === 'double') {
+ mapExpr = `(e) => (e as num).toDouble()`;
} else {
mapExpr = `(e) => e as ${itemTr.type}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (tr.isList && tr.listItemType) { | |
| const itemTr = tr.listItemType; | |
| let mapExpr: string; | |
| if (itemTr.isReference) { | |
| mapExpr = `(e) => ${itemTr.type}.fromJson(e as Map<String, dynamic>)`; | |
| } else if (itemTr.isDateTime || itemTr.isDateOnly) { | |
| mapExpr = `(e) => DateTime.parse(e as String)`; | |
| } else { | |
| mapExpr = `(e) => e as ${itemTr.type}`; | |
| } | |
| const cast = `(${accessor} as List<dynamic>).map(${mapExpr}).toList()`; | |
| if (nullable) return `${accessor} != null ? ${cast} : null`; | |
| return cast; | |
| } | |
| if (tr.isList && tr.listItemType) { | |
| const itemTr = tr.listItemType; | |
| let mapExpr: string; | |
| if (itemTr.isReference) { | |
| mapExpr = `(e) => ${itemTr.type}.fromJson(e as Map<String, dynamic>)`; | |
| } else if (itemTr.isDateTime || itemTr.isDateOnly) { | |
| mapExpr = `(e) => DateTime.parse(e as String)`; | |
| } else if (itemTr.type === 'double') { | |
| mapExpr = `(e) => (e as num).toDouble()`; | |
| } else { | |
| mapExpr = `(e) => e as ${itemTr.type}`; | |
| } | |
| const cast = `(${accessor} as List<dynamic>).map(${mapExpr}).toList()`; | |
| if (nullable) return `${accessor} != null ? ${cast} : null`; | |
| return cast; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/dart-types.ts` around lines 306 - 319, The list-item
mapping currently uses a generic cast `(e) => e as ${itemTr.type}`, which fails
for JSON integers when the list item type is double; update the branch that
builds mapExpr inside the tr.isList block to special-case itemTr being double
(e.g., itemTr.isDouble or itemTr.type === 'double') and set mapExpr to `(e) =>
(e as num).toDouble()` so list items are converted like the scalar double path;
keep the existing checks for itemTr.isReference, itemTr.isDateTime, and
itemTr.isDateOnly, and leave nullable handling and the final cast construction
(`(${accessor} as List<dynamic>).map(${mapExpr}).toList()`) unchanged.
Comment on lines
+116
to
+121
| const successResp = | ||
| operation.responses?.['200'] ?? operation.responses?.['201']; | ||
| if (successResp?.content) { | ||
| const ct = Object.keys(successResp.content)[0]; | ||
| responseSchema = successResp.content[ct]?.schema; | ||
| } |
Contributor
There was a problem hiding this comment.
Success response is restricted to 200/201 — broader 2xx codes are ignored.
Operations that return 204 No Content, 202 Accepted, or any other 2xx status will be treated as if they had no response schema, which is fine today (everything emits Response<dynamic>) but will become a real bug as soon as typed responses are wired in. Consider iterating operation.responses and picking the first key matching ^2\d\d$ (or special-casing default).
🛠️ Suggested change
- let responseSchema: SchemaLike | undefined;
- const successResp =
- operation.responses?.['200'] ?? operation.responses?.['201'];
- if (successResp?.content) {
- const ct = Object.keys(successResp.content)[0];
- responseSchema = successResp.content[ct]?.schema;
- }
+ let responseSchema: SchemaLike | undefined;
+ const successKey = Object.keys(operation.responses ?? {}).find((k) =>
+ /^2\d\d$/.test(k),
+ );
+ const successResp = successKey
+ ? operation.responses?.[successKey]
+ : undefined;
+ if (successResp?.content) {
+ const ct = Object.keys(successResp.content)[0];
+ responseSchema = successResp.content[ct]?.schema;
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/generate-client.ts` around lines 116 - 121, The current
logic only checks operation.responses['200'] and ['201'] which ignores other 2xx
codes; update the selection of success response used to set responseSchema (the
block that assigns successResp and responseSchema) to instead iterate
operation.responses keys and pick the first response whose status code matches
the regex /^2\d{2}$/ (or fall back to a 'default' response if present), then
extract the content schema from that response; ensure you still handle missing
content gracefully when setting responseSchema.
Comment on lines
+232
to
+244
| function generateMethod( | ||
| op: OperationInfo, | ||
| allSchemaSet: Set<string>, | ||
| usedNames: Set<string>, | ||
| ): { method: string; referencedSchemas: string[] } { | ||
| const refs: string[] = []; | ||
| let methodName = deriveMethodName(op); | ||
| // Deduplicate within the same API class | ||
| if (usedNames.has(methodName)) { | ||
| methodName = `${methodName}${op.httpMethod.charAt(0).toUpperCase() + op.httpMethod.slice(1)}`; | ||
| } | ||
| usedNames.add(methodName); | ||
|
|
Contributor
There was a problem hiding this comment.
Method-name deduplication only resolves the first collision.
usedNames.has(methodName) triggers a one-shot +verb suffix, but if two operations on the same verb still derive the same summary (e.g. two GETs both summarized "List items"), the suffixed names also collide and the second one silently overwrites the first in the resulting class without compile error in Dart. Consider falling back to a numeric counter when the suffixed name is also taken.
🛠️ Suggested fix
- let methodName = deriveMethodName(op);
- // Deduplicate within the same API class
- if (usedNames.has(methodName)) {
- methodName = `${methodName}${op.httpMethod.charAt(0).toUpperCase() + op.httpMethod.slice(1)}`;
- }
- usedNames.add(methodName);
+ const base = deriveMethodName(op);
+ let methodName = base;
+ if (usedNames.has(methodName)) {
+ const verb =
+ op.httpMethod.charAt(0).toUpperCase() + op.httpMethod.slice(1);
+ methodName = `${base}${verb}`;
+ let i = 2;
+ while (usedNames.has(methodName)) {
+ methodName = `${base}${verb}${i++}`;
+ }
+ }
+ usedNames.add(methodName);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function generateMethod( | |
| op: OperationInfo, | |
| allSchemaSet: Set<string>, | |
| usedNames: Set<string>, | |
| ): { method: string; referencedSchemas: string[] } { | |
| const refs: string[] = []; | |
| let methodName = deriveMethodName(op); | |
| // Deduplicate within the same API class | |
| if (usedNames.has(methodName)) { | |
| methodName = `${methodName}${op.httpMethod.charAt(0).toUpperCase() + op.httpMethod.slice(1)}`; | |
| } | |
| usedNames.add(methodName); | |
| function generateMethod( | |
| op: OperationInfo, | |
| allSchemaSet: Set<string>, | |
| usedNames: Set<string>, | |
| ): { method: string; referencedSchemas: string[] } { | |
| const refs: string[] = []; | |
| const base = deriveMethodName(op); | |
| let methodName = base; | |
| if (usedNames.has(methodName)) { | |
| const verb = | |
| op.httpMethod.charAt(0).toUpperCase() + op.httpMethod.slice(1); | |
| methodName = `${base}${verb}`; | |
| let i = 2; | |
| while (usedNames.has(methodName)) { | |
| methodName = `${base}${verb}${i++}`; | |
| } | |
| } | |
| usedNames.add(methodName); | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/generate-client.ts` around lines 232 - 244, The current
dedup logic in generateMethod uses deriveMethodName(op) and does a single check
against usedNames, appending op.httpMethod once, which can still collide; change
generateMethod to loop and append a stable disambiguator (e.g., numeric counter)
until a unique methodName is found: compute baseName = deriveMethodName(op), try
baseName, then baseName + httpMethodSuffix, then baseName + httpMethodSuffix +
counter (or baseName + counter) incrementing counter until
usedNames.has(methodName) is false, then usedNames.add(methodName) and return;
update references to methodName and keep refs logic unchanged.
Comment on lines
+248
to
+279
| // path params | ||
| for (const p of op.pathParams) { | ||
| const tr = resolveDartType(p.schema ?? { type: 'string' }); | ||
| const dartName = toCamelCase(p.name); | ||
| params.push(`required ${tr.type} ${dartName}`); | ||
| pathSubstitutions[p.name] = dartName; | ||
| } | ||
|
|
||
| // query params | ||
| const queryParamNames: { | ||
| jsonName: string; | ||
| dartName: string; | ||
| required: boolean; | ||
| dartType: string; | ||
| }[] = []; | ||
| for (const p of op.queryParams) { | ||
| const tr = resolveDartType(p.schema ?? { type: 'string' }); | ||
| const dartName = toCamelCase(p.name); | ||
| const isRequired = p.required ?? false; | ||
| const type = isRequired | ||
| ? tr.type | ||
| : tr.type === 'dynamic' | ||
| ? 'dynamic' | ||
| : `${tr.type}?`; | ||
| params.push(`${isRequired ? 'required ' : ''}${type} ${dartName}`); | ||
| queryParamNames.push({ | ||
| jsonName: p.name, | ||
| dartName, | ||
| required: isRequired, | ||
| dartType: tr.type, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Path/query parameter names are not escaped for Dart reserved words.
utils.ts already exports escapeDartReserved, but it's not used here. OpenAPI parameter names like in, class, new, var, for, is, default (legal in JSON Schema) will be emitted as raw Dart identifiers and produce uncompilable code, e.g. required String in or required int class.
🛠️ Suggested fix
-import { sanitizeTagName, toCamelCase, toSnakeCase } from './utils';
+import {
+ escapeDartReserved,
+ sanitizeTagName,
+ toCamelCase,
+ toSnakeCase,
+} from './utils';
@@
- for (const p of op.pathParams) {
- const tr = resolveDartType(p.schema ?? { type: 'string' });
- const dartName = toCamelCase(p.name);
+ for (const p of op.pathParams) {
+ const tr = resolveDartType(p.schema ?? { type: 'string' });
+ const dartName = escapeDartReserved(toCamelCase(p.name));
params.push(`required ${tr.type} ${dartName}`);
pathSubstitutions[p.name] = dartName;
}
@@
- for (const p of op.queryParams) {
- const tr = resolveDartType(p.schema ?? { type: 'string' });
- const dartName = toCamelCase(p.name);
+ for (const p of op.queryParams) {
+ const tr = resolveDartType(p.schema ?? { type: 'string' });
+ const dartName = escapeDartReserved(toCamelCase(p.name));The same applies to the body/formData literal names below — they happen to be safe today, but routing all generated identifiers through escapeDartReserved is the durable fix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/generate-client.ts` around lines 248 - 279, The code
constructs Dart identifiers for path/query parameters without escaping reserved
words; update the parameter name handling to run generated names through
escapeDartReserved from utils.ts: when creating dartName for path params (used
with resolveDartType, toCamelCase, params, and pathSubstitutions) and for query
params (used with resolveDartType, toCamelCase, params, and queryParamNames),
replace the raw toCamelCase(p.name) with escapeDartReserved(toCamelCase(p.name))
so emitted identifiers are safe; also apply the same escape to any generated
identifiers for body/formData keys elsewhere in this file to ensure all
generated Dart identifiers are escaped consistently.
Comment on lines
+325
to
+328
| let dartPath = op.path; | ||
| for (const [paramName, dartName] of Object.entries(pathSubstitutions)) { | ||
| dartPath = dartPath.replace(`{${paramName}}`, `\$${dartName}`); | ||
| } |
Contributor
There was a problem hiding this comment.
Path interpolation only replaces the first occurrence of each placeholder.
String.prototype.replace with a string pattern replaces only the first match, so a path like /things/{id}/sub/{id} (legal in OpenAPI although uncommon) will leave the second {id} untouched in the emitted Dart string. Use replaceAll (or a global regex) to be safe.
🛠️ Suggested fix
- let dartPath = op.path;
- for (const [paramName, dartName] of Object.entries(pathSubstitutions)) {
- dartPath = dartPath.replace(`{${paramName}}`, `\$${dartName}`);
- }
+ let dartPath = op.path;
+ for (const [paramName, dartName] of Object.entries(pathSubstitutions)) {
+ dartPath = dartPath.replaceAll(`{${paramName}}`, `\$${dartName}`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let dartPath = op.path; | |
| for (const [paramName, dartName] of Object.entries(pathSubstitutions)) { | |
| dartPath = dartPath.replace(`{${paramName}}`, `\$${dartName}`); | |
| } | |
| let dartPath = op.path; | |
| for (const [paramName, dartName] of Object.entries(pathSubstitutions)) { | |
| dartPath = dartPath.replaceAll(`{${paramName}}`, `\$${dartName}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/generate-client.ts` around lines 325 - 328, The loop that
builds dartPath using pathSubstitutions only calls String.prototype.replace,
which only replaces the first occurrence; update the substitution in the block
that iterates over pathSubstitutions (the dartPath construction using
dartPath.replace(`{${paramName}}`, `\$${dartName}`)) to use replaceAll (or a
global-regex equivalent) so every `{paramName}` is replaced (e.g., dartPath =
dartPath.replaceAll(`{${paramName}}`, `\$${dartName}`) or dartPath =
dartPath.replace(new RegExp(`\\{${paramName}\\}`, 'g'), `\$${dartName}`)).
Comment on lines
+368
to
+378
| if (bodyParamName && !op.isMultipart) { | ||
| const isRef = op.requestBodySchema?.$ref; | ||
| if (isRef) { | ||
| out += ` data: ${bodyParamName}.toJson(),\n`; | ||
| } else { | ||
| out += ` data: ${bodyParamName},\n`; | ||
| } | ||
| } | ||
| if (op.isMultipart) { | ||
| out += ` data: ${bodyParamName},\n`; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect resolveDartType return shape to determine the correct field names
# for "is array of reference" detection.
cat packages/dart/src/dart-types.tsRepository: orval-labs/orval
Length of output: 10047
🏁 Script executed:
# Check the actual generate-client.ts code around lines 368-378
sed -n '360,385p' packages/dart/src/generate-client.tsRepository: orval-labs/orval
Length of output: 719
🏁 Script executed:
# Search for any existing usage of resolveDartType in generate-client.ts
rg 'resolveDartType' packages/dart/src/generate-client.ts -B 2 -A 2Repository: orval-labs/orval
Length of output: 1029
🏁 Script executed:
# Search for test files related to dart and body serialization
find packages/dart -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) | head -20Repository: orval-labs/orval
Length of output: 42
🏁 Script executed:
# Check if there are any existing tests for body serialization
rg 'List.*Ref\|toJson.*body\|multipart.*body' packages/dart --type typescript -iRepository: orval-labs/orval
Length of output: 95
🏁 Script executed:
# Search for body serialization context without type restriction
rg 'isMultipart|bodyParamName|toJson' packages/dart/src/generate-client.ts -B 2 -A 2Repository: orval-labs/orval
Length of output: 1680
🏁 Script executed:
# Look for test files with different naming patterns
find packages/dart -type f -name "*.ts" | grep -iE 'test|spec'Repository: orval-labs/orval
Length of output: 42
🏁 Script executed:
# Check how the already-computed tr variable is used for body type
sed -n '330,360p' packages/dart/src/generate-client.tsRepository: orval-labs/orval
Length of output: 877
🏁 Script executed:
# Verify the full function signature and context around body handling
sed -n '300,380p' packages/dart/src/generate-client.tsRepository: orval-labs/orval
Length of output: 2334
Body serialization breaks for List<RefType> and other non-direct-$ref model bodies.
The code at lines 368–378 checks only op.requestBodySchema?.$ref directly, missing List<Pet> (array with items.$ref), allOf-wrapped refs, and oneOf refs. When the request body is { type: 'array', items: { $ref: '#/.../Pet' } }, the check fails and emits data: body, causing Dio to receive a List<Pet> of Dart class instances it cannot serialize — runtime failure.
The function resolveDartType already resolves these correctly (via isList, listItemType.isReference, etc.). Reuse the tr computed a few lines above instead of re-checking the raw schema:
🛠️ Suggested fix
if (bodyParamName && !op.isMultipart) {
- const isRef = op.requestBodySchema?.$ref;
- if (isRef) {
+ const tr = resolveDartType(op.requestBodySchema!);
+ if (tr.isReference) {
out += ` data: ${bodyParamName}.toJson(),\n`;
- } else {
+ } else if (tr.isList && tr.listItemType?.isReference) {
+ out += ` data: ${bodyParamName}.map((e) => e.toJson()).toList(),\n`;
+ } else {
out += ` data: ${bodyParamName},\n`;
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/generate-client.ts` around lines 368 - 378, The body
serialization check currently inspects op.requestBodySchema?.$ref and misses
array/oneOf/allOf wrapped refs; replace that raw-schema check with the resolved
type info you already computed via resolveDartType (the variable tr) and
bodyParamName: if tr.isReference OR (tr.isList && tr.listItemType?.isReference)
(and similarly handle any resolved union/allOf indicators from tr if present)
emit a serialized form (call toJson on the model or map list items to toJson)
instead of emitting the raw body, otherwise emit data: bodyParamName as before;
update the logic around resolveDartType/tr and bodyParamName so multipart
handling remains unchanged.
Comment on lines
+31
to
+50
| export function sanitizeTagName(tag: string): string { | ||
| return tag | ||
| .replace(/[^a-zA-Z0-9\s]/g, '') | ||
| .trim() | ||
| .split(/\s+/) | ||
| .map((w) => w.charAt(0).toUpperCase() + w.slice(1).toLowerCase()) | ||
| .join(''); | ||
| } | ||
|
|
||
| /** | ||
| * Sanitize a schema name for use as a Dart class name. | ||
| * Handles names like "Body_bulk_upload_api_flat_bulk_upload__post". | ||
| */ | ||
| export function sanitizeClassName(name: string): string { | ||
| return name | ||
| .split(/[_\s]+/) | ||
| .filter(Boolean) | ||
| .map((w) => w.charAt(0).toUpperCase() + w.slice(1)) | ||
| .join(''); | ||
| } |
Contributor
There was a problem hiding this comment.
sanitizeTagName / sanitizeClassName can produce identifiers starting with a digit.
A tag like "123 Stuff" becomes "123Stuff", which is not a valid Dart identifier and will fail to compile when used as a class name (class 123StuffApi { ... }). Prefix a leading underscore (or other safe prefix) when the first char is a digit.
🛠️ Suggested fix
export function sanitizeTagName(tag: string): string {
- return tag
+ const out = tag
.replace(/[^a-zA-Z0-9\s]/g, '')
.trim()
.split(/\s+/)
.map((w) => w.charAt(0).toUpperCase() + w.slice(1).toLowerCase())
.join('');
+ return /^[0-9]/.test(out) ? `_${out}` : out;
}
@@
export function sanitizeClassName(name: string): string {
- return name
+ const out = name
.split(/[_\s]+/)
.filter(Boolean)
.map((w) => w.charAt(0).toUpperCase() + w.slice(1))
.join('');
+ return /^[0-9]/.test(out) ? `_${out}` : out;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/utils.ts` around lines 31 - 50, sanitizeTagName and
sanitizeClassName can return identifiers that start with a digit (e.g.,
"123Stuff"), which is invalid in Dart; update both functions (sanitizeTagName
and sanitizeClassName) to detect if the resulting string's first character is a
digit and, if so, prepend a safe prefix (for example an underscore) before
returning, ensuring the rest of the existing transformation logic remains
unchanged so existing capitalization/formatting is preserved.
Comment on lines
+120
to
+125
| /** | ||
| * Escape a Dart reserved word by appending an underscore suffix. | ||
| */ | ||
| export function escapeDartReserved(name: string): string { | ||
| return DART_RESERVED.has(name) ? `${name}_` : name; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether escapeDartReserved is wired into the Dart generator.
rg -nP '\bescapeDartReserved\b' --type=ts -C2Repository: orval-labs/orval
Length of output: 400
🏁 Script executed:
# Check all TypeScript files in the Dart package for imports of escapeDartReserved
find packages/dart/src -name "*.ts" -type f | head -20Repository: orval-labs/orval
Length of output: 220
🏁 Script executed:
# Search for any import statements that reference escapeDartReserved
rg "import.*escapeDartReserved|from.*utils" packages/dart/src --type=ts -nRepository: orval-labs/orval
Length of output: 333
🏁 Script executed:
# Check if DART_RESERVED is used anywhere (the constant defined alongside the function)
rg "DART_RESERVED" packages/dart/src --type=ts -n -C1Repository: orval-labs/orval
Length of output: 795
🏁 Script executed:
# Check where escapeDartFieldName is called within dart-types.ts
rg "escapeDartFieldName" packages/dart/src/dart-types.ts -n -C2Repository: orval-labs/orval
Length of output: 406
Remove unused escapeDartReserved function and DART_RESERVED constant from utils.ts—they are exported but never imported.
Dart reserved word handling already exists via escapeDartFieldName() in dart-types.ts (line 205), which is actively used and contains a duplicate DART_RESERVED set. Consolidate by removing the dead code from utils.ts to avoid maintenance confusion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dart/src/utils.ts` around lines 120 - 125, Remove the dead exports
in utils.ts: delete the escapeDartReserved function and the DART_RESERVED
constant since Dart reserved handling is already implemented in
escapeDartFieldName in dart-types.ts; update any exports list in utils.ts to not
export these symbols and run project-wide search to confirm no remaining imports
reference escapeDartReserved or DART_RESERVED before committing.
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
2 participants
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.
Add a new
@orval/dartpackage that generates Dart model classes andDio-based API client code from an OpenAPI specification.
Summary by CodeRabbit
New Features
Documentation
Packaging