- 
                Notifications
    
You must be signed in to change notification settings  - Fork 576
 
feat(cli-zod): use light weight check mode if not on the happy path #4946
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat(cli-zod): use light weight check mode if not on the happy path #4946
Conversation
| const userZodMajor = semver.major(semver.coerce(userZodVersion) || '0.0.0'); | ||
| const nangoZodMajor = semver.major(semver.coerce(nangoZodVersion) || '0.0.0'); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[BestPractice]
Potential error handling issue: The semver.coerce() call may return null for malformed version strings, but the fallback '0.0.0' could mask version parsing failures and lead to incorrect major version comparisons. Consider adding explicit validation:
const userZodMajor = semver.major(semver.coerce(userZodVersion) || '0.0.0');
const nangoZodMajor = semver.major(semver.coerce(nangoZodVersion) || '0.0.0');
if (userZodMajor === 0 || nangoZodMajor === 0) {
    // Log warning about unparseable versions
    console.log(chalk.yellow(`⚠️  Could not parse version strings: user=${userZodVersion}, nango=${nangoZodVersion}`));
}Context for Agents
[**BestPractice**]
Potential error handling issue: The `semver.coerce()` call may return `null` for malformed version strings, but the fallback `'0.0.0'` could mask version parsing failures and lead to incorrect major version comparisons. Consider adding explicit validation:
```typescript
const userZodMajor = semver.major(semver.coerce(userZodVersion) || '0.0.0');
const nangoZodMajor = semver.major(semver.coerce(nangoZodVersion) || '0.0.0');
if (userZodMajor === 0 || nangoZodMajor === 0) {
    // Log warning about unparseable versions
    console.log(chalk.yellow(`⚠️  Could not parse version strings: user=${userZodVersion}, nango=${nangoZodVersion}`));
}
```
File: packages/cli/lib/zeroYaml/check.ts
Line: 47There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds reasonable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like a lot of complexity for diminished guaranteed. The 4.0 -> 4.1 OOM issue already shows that we cannot even rely on zod minor versions. To please a few customers we are going to make our life way more difficult and introduce tons of complexity. Maybe relying on zod for the typing was not such a good idea after all
| const userZodMajor = semver.major(semver.coerce(userZodVersion) || '0.0.0'); | ||
| const nangoZodMajor = semver.major(semver.coerce(nangoZodVersion) || '0.0.0'); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds reasonable
| if (userZodMajor !== nangoZodMajor) { | ||
| // Different major version - force update | ||
| updated = true; | ||
| packageJson.devDependencies['zod'] = nangoZodVersion; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should warn about updating. no?
This is up for discussion if we want to allow this at all. See https://linear.app/nango/issue/NAN-4255/investigate-zod-dependency-improvement again for info.
This essentially short circuits the full compilation check for a surface level check otherwise the zod mismatch triggers an OOM error. Leaves the happy path unchanged
Test against zod versions
Add lightweight TypeScript/Zod type-checking fallback to prevent OOM on dependency mismatch
Introduces a fallback “lightweight” compilation path for the CLI that is activated when the user’s
zodversion differs from the version bundled with the Nango CLI. Instead of full semantic type-checking (which can trigger out-of-memory errors with Zod ≥4.1), the fallback performs syntax-only diagnostics and custom AST-based Zod-API validation. Normal (full) type-checking remains unchanged when versions match. The PR also refinespackage.jsonsync logic to warn on minor/patch Zod differences and force-update on major differences.Key Changes
• Added new module
packages/cli/lib/zeroYaml/lightweightTypecheck.tsexposingshouldUseLightweightMode()andrunLightweightTypecheck()with AST parsing to catch common Zod mistakes while avoiding semantic checking• Integrated fallback into
packages/cli/lib/zeroYaml/compile.ts; compile flow now decides between fulltypeCheck()andrunLightweightTypecheck()based onshouldUseLightweightMode()• Enhanced dependency sync in
packages/cli/lib/zeroYaml/check.ts: now usessemverto compare Zod versions, warns on minor/patch mismatch, auto-updates on major mismatch, and adds missing Zod dev-dependency• Imported
semver(new runtime dep) and adjusted import list• Removed rigid Zod pinning logic; replaced with flexible, version-aware handling
Affected Areas
•
zeroYamlcompile pipeline•
check.tsdependency validation• New
lightweightTypecheckutility• CLI dev-dependency management logic
This summary was automatically generated by @propel-code-bot