-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Make zod types strip (default) instead of passthrough #792
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
Open
ihrpr
wants to merge
7
commits into
main
Choose a base branch
from
ihrpr/zod-types
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
43a7a1e
add strip types
ihrpr 33f69eb
improve example
ihrpr 7012f5d
Merge branch 'main' into ihrpr/zod-types
ihrpr 47c91b2
merge
ihrpr c3cf0f8
lint
ihrpr 7b6060f
BaseRequestParamsSchema, BaseNotificationParamsSchema, RequestMetaSch…
ihrpr 3607385
Tweak strict types setup, fix type errors under strictTypes.ts (#806)
bhosmer-ant File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
#!/usr/bin/env node | ||
import { readFileSync, writeFileSync } from 'fs'; | ||
import { join, dirname } from 'path'; | ||
import { fileURLToPath } from 'url'; | ||
|
||
const __dirname = dirname(fileURLToPath(import.meta.url)); | ||
|
||
// Read the original types.ts file | ||
const typesPath = join(__dirname, '../src/types.ts'); | ||
const strictTypesPath = join(__dirname, '../src/strictTypes.ts'); | ||
|
||
let content = readFileSync(typesPath, 'utf-8'); | ||
|
||
// Remove the @deprecated comment block | ||
const deprecatedCommentPattern = /\/\*\*\s*\n\s*\*\s*@deprecated[\s\S]*?\*\/\s*\n/; | ||
content = content.replace(deprecatedCommentPattern, ''); | ||
|
||
// Add header comment | ||
const header = `/** | ||
* Types remove unknown | ||
* properties to maintaining compatibility with protocol extensions. | ||
* | ||
* - Protocol compatoble: Unknown fields from extended implementations are removed, not rejected | ||
* - Forward compatibility: Works with servers/clients that have additional fields | ||
* | ||
* @generated by scripts/generateStrictTypes.ts | ||
*/ | ||
|
||
`; | ||
|
||
// Replace all .passthrough() with a temporary marker | ||
content = content.replace(/\.passthrough\(\)/g, '.__TEMP_MARKED_FOR_REMOVAL__()'); | ||
|
||
// Special handling for experimental and capabilities that should remain open | ||
// These are explicitly designed to be extensible | ||
const patternsToKeepOpen = [ | ||
// Keep experimental fields open as they're meant for extensions | ||
/experimental: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g, | ||
// Keep _meta fields open as they're meant for arbitrary metadata | ||
/_meta: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g, | ||
// Keep JSON Schema properties open as they can have arbitrary fields | ||
/properties: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g, | ||
// Keep BaseRequestParamsSchema passthrough for JSON-RPC param compatibility | ||
/const BaseRequestParamsSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g, | ||
// Keep BaseNotificationParamsSchema passthrough for JSON-RPC param compatibility | ||
/const BaseNotificationParamsSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g, | ||
// Keep RequestMetaSchema passthrough for extensibility | ||
/const RequestMetaSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g, | ||
// Keep structuredContent passthrough for tool-specific output | ||
/structuredContent: z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\.optional\(\)/g, | ||
// Keep metadata passthrough for provider-specific data in sampling | ||
/metadata: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g, | ||
]; | ||
|
||
// Revert marker back to passthrough for these special cases | ||
patternsToKeepOpen.forEach(pattern => { | ||
content = content.replace(pattern, (match) => | ||
match.replace('.__TEMP_MARKED_FOR_REMOVAL__()', '.passthrough()') | ||
); | ||
}); | ||
|
||
// Remove the temporary marker from all remaining locations (these become no modifier) | ||
content = content.replace(/\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g, ''); | ||
|
||
// Add a comment explaining the difference | ||
const explanation = ` | ||
/** | ||
* Note: The following remain open (using .passthrough()): | ||
* - experimental: Designed for protocol extensions | ||
* - _meta: Designed for arbitrary metadata | ||
* - properties: JSON Schema properties that can have arbitrary fields | ||
* - BaseRequestParamsSchema: Required for JSON-RPC param compatibility | ||
* - BaseNotificationParamsSchema: Required for JSON-RPC param compatibility | ||
* - RequestMetaSchema: Required for protocol extensibility | ||
* - structuredContent: Tool-specific output that can have arbitrary fields | ||
* - metadata: Provider-specific metadata in sampling requests | ||
* | ||
* All other objects use default behavior (no modifier) to remove unknown properties while | ||
* maintaining compatibility with extended protocols. | ||
*/ | ||
`; | ||
|
||
// Insert the explanation after the imports | ||
const importEndIndex = content.lastIndexOf('import'); | ||
const importEndLineIndex = content.indexOf('\n', importEndIndex); | ||
content = content.slice(0, importEndLineIndex + 1) + explanation + content.slice(importEndLineIndex + 1); | ||
|
||
// Write the strict types file | ||
writeFileSync(strictTypesPath, header + content); | ||
|
||
console.log('Generated strictTypes.ts successfully!'); |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
#!/usr/bin/env node | ||
|
||
import { execSync } from 'child_process'; | ||
import { readFileSync, writeFileSync, existsSync } from 'fs'; | ||
import { join, dirname } from 'path'; | ||
import { fileURLToPath } from 'url'; | ||
|
||
const __dirname = dirname(fileURLToPath(import.meta.url)); | ||
const rootDir = join(__dirname, '..'); | ||
|
||
const files = [ | ||
'src/client/index.ts', | ||
'src/server/index.ts', | ||
'src/server/mcp.ts' | ||
]; | ||
|
||
console.log('Testing strict types compatibility...'); | ||
console.log('======================================\n'); | ||
|
||
// Backup original files | ||
const backups = {}; | ||
files.forEach(file => { | ||
const fullPath = join(rootDir, file); | ||
if (existsSync(fullPath)) { | ||
backups[file] = readFileSync(fullPath, 'utf8'); | ||
|
||
// Replace imports | ||
const content = backups[file]; | ||
const newContent = content.replace(/from "\.\.\/types\.js"/g, 'from "../strictTypes.js"'); | ||
writeFileSync(fullPath, newContent); | ||
console.log(`✓ Replaced imports in ${file}`); | ||
} | ||
}); | ||
|
||
console.log('\nRunning TypeScript compilation...\n'); | ||
|
||
try { | ||
// Run TypeScript compilation | ||
execSync('npm run build', { cwd: rootDir, stdio: 'pipe' }); | ||
console.log('✓ No type errors found!'); | ||
} catch (error) { | ||
// Extract and format type errors | ||
const output = error.stdout?.toString() || error.stderr?.toString() || ''; | ||
const lines = output.split('\n'); | ||
|
||
const errors = []; | ||
let currentError = null; | ||
|
||
lines.forEach((line, i) => { | ||
if (line.includes('error TS')) { | ||
if (currentError) { | ||
errors.push(currentError); | ||
} | ||
currentError = { | ||
file: line.split('(')[0], | ||
location: line.match(/\((\d+),(\d+)\)/)?.[0] || '', | ||
code: line.match(/error (TS\d+)/)?.[1] || '', | ||
message: line.split(/error TS\d+:/)[1]?.trim() || '', | ||
context: [] | ||
}; | ||
} else if (currentError && line.trim() && !line.startsWith('npm ')) { | ||
currentError.context.push(line); | ||
} | ||
}); | ||
|
||
if (currentError) { | ||
errors.push(currentError); | ||
} | ||
|
||
// Display errors | ||
console.log(`Found ${errors.length} type error(s):\n`); | ||
|
||
errors.forEach((error, index) => { | ||
console.log(`${index + 1}. ${error.file}${error.location}`); | ||
console.log(` Error ${error.code}: ${error.message}`); | ||
if (error.context.length > 0) { | ||
console.log(` Context:`); | ||
error.context.slice(0, 3).forEach(line => { | ||
console.log(` ${line.trim()}`); | ||
}); | ||
} | ||
console.log(''); | ||
}); | ||
} | ||
|
||
// Restore original files | ||
console.log('Restoring original files...'); | ||
Object.entries(backups).forEach(([file, content]) => { | ||
const fullPath = join(rootDir, file); | ||
writeFileSync(fullPath, content); | ||
}); | ||
|
||
console.log('✓ Original files restored.'); |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
#!/bin/bash | ||
|
||
# Script to test strict types by replacing imports and running tests | ||
|
||
echo "Testing strict types compatibility..." | ||
echo "======================================" | ||
|
||
# Save original files | ||
cp src/client/index.ts src/client/index.ts.bak | ||
cp src/server/index.ts src/server/index.ts.bak | ||
cp src/server/mcp.ts src/server/mcp.ts.bak | ||
|
||
# Replace imports | ||
sed -i '' 's/from "\.\.\/types\.js"/from "..\/strictTypes.js"/g' src/client/index.ts | ||
sed -i '' 's/from "\.\.\/types\.js"/from "..\/strictTypes.js"/g' src/server/index.ts | ||
sed -i '' 's/from "\.\.\/types\.js"/from "..\/strictTypes.js"/g' src/server/mcp.ts | ||
|
||
echo "Replaced imports in:" | ||
echo " - src/client/index.ts" | ||
echo " - src/server/index.ts" | ||
echo " - src/server/mcp.ts" | ||
echo "" | ||
|
||
# Run TypeScript compilation to get type errors | ||
echo "Running TypeScript compilation..." | ||
echo "" | ||
npm run build 2>&1 | grep -E "(error TS|src/)" | grep -B1 "error TS" || true | ||
|
||
# Restore original files | ||
mv src/client/index.ts.bak src/client/index.ts | ||
mv src/server/index.ts.bak src/server/index.ts | ||
mv src/server/mcp.ts.bak src/server/mcp.ts | ||
|
||
echo "" | ||
echo "Original files restored." |
Oops, something went wrong.
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.
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.
Wonder if we could do a deep copy of the current src/, package.json, package-lock and node_modules to a temp directory instead (Then
cwd: tmpDir
below), to avoid in place mutations?