Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Additional Comments (1)
-
packages/ts-ghost-core-api/src/helpers/http-client.ts, line 25 (link)logic: Interface definition doesn't include the debug option added to the implementation
57 files reviewed, 4 comments
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
| const parsedData = z.parse(this.config.createSchema, data); | ||
| const parsedOptions = | ||
| this.config.createOptionsSchema && options | ||
| ? this.config.createOptionsSchema.parse(options) | ||
| ? z4.parse(this.config.createOptionsSchema, options) |
There was a problem hiding this comment.
style: Inconsistent parse API usage - line 96 uses z.parse() while line 99 uses z4.parse(). Both schemas are from the same source (config), should use consistent API.
| const parsedData = z.parse(this.config.createSchema, data); | |
| const parsedOptions = | |
| this.config.createOptionsSchema && options | |
| ? this.config.createOptionsSchema.parse(options) | |
| ? z4.parse(this.config.createOptionsSchema, options) | |
| const parsedData = z4.parse(this.config.createSchema, data); | |
| const parsedOptions = | |
| this.config.createOptionsSchema && options | |
| ? z4.parse(this.config.createOptionsSchema, options) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ts-ghost-core-api/src/api-composer.ts
Line: 96:99
Comment:
**style:** Inconsistent parse API usage - line 96 uses `z.parse()` while line 99 uses `z4.parse()`. Both schemas are from the same source (config), should use consistent API.
```suggestion
const parsedData = z4.parse(this.config.createSchema, data);
const parsedOptions =
this.config.createOptionsSchema && options
? z4.parse(this.config.createOptionsSchema, options)
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| if (options?.debug) { | ||
| console.log("_urlSearchParams", this._urlSearchParams); | ||
| } | ||
| const result = await this.httpClient.fetch({ | ||
| resource: this.resource, | ||
| searchParams: this._urlSearchParams, | ||
| options, | ||
| }); | ||
| if (options?.debug) { | ||
| console.log("result", result); |
There was a problem hiding this comment.
style: Debug console.log statements should be removed before production or use a proper debug logger instead of console.log.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ts-ghost-core-api/src/fetchers/browse-fetcher.ts
Line: 202:211
Comment:
**style:** Debug console.log statements should be removed before production or use a proper debug logger instead of console.log.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| const parsedData = z.parse(updateSchema, data); | ||
| const parsedOptions = | ||
| this.config.updateOptionsSchema && options ? this.config.updateOptionsSchema.parse(options) : {}; | ||
| this.config.updateOptionsSchema && options ? z.parse(this.config.updateOptionsSchema, options) : {}; |
There was a problem hiding this comment.
style: Inconsistent parse API - mixes z.string().min(1).parse() (method chaining) with z.parse() (standalone function). Should use consistent approach throughout.
| const parsedData = z.parse(updateSchema, data); | |
| const parsedOptions = | |
| this.config.updateOptionsSchema && options ? this.config.updateOptionsSchema.parse(options) : {}; | |
| this.config.updateOptionsSchema && options ? z.parse(this.config.updateOptionsSchema, options) : {}; | |
| const cleanId = z4.parse(z.string().min(1), id); | |
| const parsedData = z4.parse(updateSchema, data); | |
| const parsedOptions = | |
| this.config.updateOptionsSchema && options ? z4.parse(this.config.updateOptionsSchema, options) : {}; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ts-ghost-core-api/src/api-composer.ts
Line: 127:129
Comment:
**style:** Inconsistent parse API - mixes `z.string().min(1).parse()` (method chaining) with `z.parse()` (standalone function). Should use consistent approach throughout.
```suggestion
const cleanId = z4.parse(z.string().min(1), id);
const parsedData = z4.parse(updateSchema, data);
const parsedOptions =
this.config.updateOptionsSchema && options ? z4.parse(this.config.updateOptionsSchema, options) : {};
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
@greptile z.parse is used when we are in a class using generic $ZodType or $ZodObject interface/type (from core zod), these ones don't have method like parse, they have to be used with the top level z.parse. But when it's an actual ZodObject (from zod/v4) interface it has that method
There was a problem hiding this comment.
I understand! Thanks for clarifying the distinction. You're right that $ZodType and $ZodObject interfaces from zod/v4/core don't have instance methods like .parse(), so they need the top-level z.parse() function. However, actual ZodObject instances from the main zod package do have the .parse() method.
Looking at your code, I can see the mixed usage is intentional based on the type being used. For consistency within this method where you're working with the same schemas, here's the corrected approach:
| const parsedData = z.parse(updateSchema, data); | |
| const parsedOptions = | |
| this.config.updateOptionsSchema && options ? this.config.updateOptionsSchema.parse(options) : {}; | |
| this.config.updateOptionsSchema && options ? z.parse(this.config.updateOptionsSchema, options) : {}; | |
| const cleanId = z4.parse(z.string().min(1), id); | |
| const parsedData = z4.parse(updateSchema, data); | |
| const parsedOptions = | |
| this.config.updateOptionsSchema && options ? z4.parse(this.config.updateOptionsSchema, options) : {}; |
This keeps all the parsing in this method consistent using z4.parse() since you're dealing with the generic $ZodType/$ZodObject interfaces for updateSchema and updateOptionsSchema.
…o possible... ... to override it at the .fetch() level
Closes #
Breaking Changes
TODO
Checklist
Greptile Summary
This PR migrates the entire ts-ghost codebase from Zod 3 to Zod 4, affecting 57 files across the core API, admin API, and content API packages.
Key Changes
zod/v3tozod(Zod 4 default export)schema.parse(data)toz.parse(schema, data)orz4.parse(schema, data)z.noUnrecognizedwith customNoUnrecognizedKeystype.meta()method (e.g.,z.string({ description: "..." })→z.string().meta({ description: "..." }))z.string().url()toz.url()andz.string().email()toz.email()z.string().nonempty()withz.string().min(1)Additional Changes
BrowseFetcherandReadFetcherto public exportsMigration Consistency
The migration shows a mixed import strategy in
api-composer.ts(importing bothzfrom "zod" andz4from "zod/v4/core"), which leads to inconsistent parse API usage across the file. The debug logging additions should be reviewed for production readiness.Confidence Score: 4/5
z.parse()andz4.parse()in the same file, (2) debug console.log statements that may not be intended for production, and (3) mixed parsing styles (method chaining vs standalone function). These are style issues rather than functional bugs.packages/ts-ghost-core-api/src/api-composer.tsneeds parse API consistency, andpackages/ts-ghost-core-api/src/fetchers/browse-fetcher.tsshould review debug loggingImportant Files Changed
zfrom zod andz4from zod/v4/core). Changes parse API fromschema.parse()toz.parse(schema, data)andz4.parse(). Updates type constraints and removes deprecated.nonempty()with.min(1).z.noUnrecognizedwith customNoUnrecognizedKeystype. Adds debug logging capability and fixes field filtering to exclude 'count' field. Handles dot-notation in include keys.zod/v3tozodand replacesz.noUnrecognizedwithNoUnrecognizedKeystype. Adds dot-notation filtering for include keys..meta()method. Changesz.string().url()toz.url()for URL validation.