Conversation
Two things kept `dorval generate` from reading its own config file.
searchPlaces listed orval.config.* and .orvalrc* only, so the dorval.config.ts
the README tells you to write was never found - a config only loaded when its
path was passed with --config. The documented names go first now, with the
orval-style ones kept behind them.
Loading a .ts config then failed outright under TypeScript 7:
TypeScript Error in ./dorval.config.ts:
typescript.findConfigFile is not a function
cosmiconfig's TypeScript loader drives the typescript package, and 7.x moved
the compiler to a native binary - its main entry exports `version` and
`versionMajorMinor`, nothing else. That loader still runs first, so nothing
changes where it works; when it throws, Node's own type stripping takes over.
Also switches the explorer to the async API, which the loader needs and which
loadConfig was already shaped for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Node's type stripping arrived in 22.13, and this package supports >=20.8.1. On an older Node with TypeScript 7 there is nothing left to transpile a .ts config with, so report that plainly along with the ways out instead of letting `typescript.findConfigFile is not a function` through. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@qwlong take a look when you have time. No hurry, it's not urgent. |
`dynamic` already admits null, so `dynamic?` is redundant and the Dart
analyzer reports it as `unnecessary_question_mark`.
Thirteen places appended `?` to a type; three of them guarded against
`dynamic`, the rest did not, and the model path (`getObject`) was among
the unguarded ones. Every site now goes through `TypeMapper.toNullable`,
which is also the only place left that knows how to spell the `?`.
A property reaches `dynamic` through several ordinary specs: `not: {}`
(the never type), `type: 'null'`, a schema with no `type` at all, and a
`oneOf`/`anyOf` too complex to map.
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two separate things stop
dorval generatefrom reading its own config file. Both reproduce in a plain install (npm i dorval@0.10.8 typescript@7) with thedorval.config.tsthe README documents:The file is never searched for.
searchPlaceslistsorval.config.*and.orvalrc*— nodorval.config.*at any extension, and no.mjseither. The README says:so a config only ever loaded when its path was passed explicitly with
--config.A
.tsconfig cannot be loaded under TypeScript 7. cosmiconfig's built-in TypeScript loader calls into thetypescriptpackage:TypeScript 7 moved the compiler to a native binary. Its main entry resolves to
lib/version.cjs:findConfigFile,transpileModule,sys,ScriptTargetare allundefined, so that loader cannot work at all. Upgrading cosmiconfig does not help — v9 loads TypeScript the same way.typescript@7.0.2is whatnpm i -D typescriptinstalls today, so this is not an edge case for long.It reproduces only in a real install: run it from a checkout and
require('typescript')resolves to the repo's own TypeScript 5, which works fine.Change
searchPlacesleads with the documenteddorval.config.{ts,mjs,js,cjs}and.dorvalrc*names, keeping theorval.config.*ones behind them so existing configs keep working. The cosmiconfig module name moves fromorvaltodorval— that only affects whichpackage.jsonkey would be read, and the README does not document configuring throughpackage.json.For
.tsfiles, the loader tries cosmiconfig's TypeScript loader first, so nothing changes on a runtime where it works, and falls back to Node's ownstripTypeScriptTypeswhen it throws. The stripped module is written next to the config — so its relative imports still resolve — imported, and removed.The explorer moves from
cosmiconfigSyncto the async API, which an async loader requires and whichloadConfigwas already shaped for.What this does not fix
Node's
stripTypeScriptTypeslanded in 22.13, and this package supports>=20.8.1. On Node 20 with TypeScript 7 there is no transpiler left, and a.tsconfig genuinely cannot be loaded. What changes there is the message — instead of a baretypescript.findConfigFile is not a function, it says what happened and what to do:So: TypeScript 7 works on Node 22.13+; on Node 20 it degrades to an actionable error. The search-places half of this PR is not affected by any of that.
Tests
packages/dorval: 9 tests pass — loading a.tsconfig from an explicit path, finding it by search under its documented name, and the fallback path with a primary loader that throws exactly what TypeScript 7 throws — asserting the config loads and no temp file is left behind where Node can strip types, and that the error names the way out where it cannot (CI covers the Node 20 branch). The two new behaviours fail without the fix. Lint clean. Monorepoturbo run test: 4/4 tasks pass.Verified in a real install (
npm i dorval@<local pack> typescript@…), which is the only shape where the TypeScript resolution matters:--configfindConfigFile is not a functionNo configuration file found--configNo configuration file foundNotes
stripTypeScriptTypesis marked experimental in Node and prints a warning on the versions that have it. It is only reached when thetypescriptpackage has already failed, so the warning appears exactly where the alternative was a hard error.CI runs Node 20 only, so the loading path is exercised locally and the error path in CI. Adding 22.x to the matrix would cover both, but that is a change to the workflow rather than to this fix.
Reported alongside the
$refalias problem in #10; that one is in@dorval/coreand unrelated to this.Second fix:
dynamic?Carried on the same branch because it surfaced in the same regeneration.
Problem
dynamicalready admits null. A?on it is redundant, anddart analyzesays so:A property lands on
dynamicthrough several ordinary specs, all of them present in a real nestjs-zod spec:Thirteen places in
@dorval/coreappend?to a type. Three guarded againstdynamic—TypeMapper.mapTypeWithNullability,ReferenceResolver.getModelType, and one branch ofresolvePropertyType. The other ten did not, and the one the model path actually runs through is among them:So the guards that existed were the ones not on the path that mattered.
Change
One helper, and every site goes through it:
`${type}?`now appears exactly once inpackages/core/src, inside that helper. TheendsWith('?')idempotence check was already duplicated at seven call sites and comes along for free.The guard is exact equality, not a substring test:
List<dynamic>?is a legitimate type and stays nullable.Tests
models-dynamic-nullable.test.ts, 5 cases: the helper's three behaviours, then a generated model assertingnot: {},type: nulland an untyped property all come out bare whileString?andList<String>?keep their?. The generator case fails without the fix.models-oneof-nullable.test.tsasserteddynamic? unionField— the old output — and is updated todynamic unionFieldplus a blanketnot.toContain('dynamic?').packages/core: 455 tests pass. Lint unchanged at its 3 pre-existing errors, none in a touched file.tsc --noEmitunchanged at its 5 pre-existing errors.End-to-end on a 165-schema production spec, same config both runs:
dynamic?in generated outputclockOut×2,publishedShift,draftShift