-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Improve performance with inline typecheck for string parsing #4058
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: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes update the validation logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant ZodString as ZodString
participant Context as ParsingContext
Caller->>ZodString: _parse(input)
alt input.data is string
ZodString->>ZodString: Validate string properties
ZodString-->>Caller: Return parsed string
else input.data is not string
ZodString->>Context: _getOrReturnCtx(input)
Context-->>ZodString: Return error context with type details
ZodString->>Context: Record type mismatch issue
ZodString-->>Caller: Return error context
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code Definitions (2)src/types.ts (1)
deno/lib/types.ts (1)
🔇 Additional comments (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for guileless-rolypoly-866f8a ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Deployment failed with the following error:
|
This small change improves the benchmarked speed of string parsing by up to ~70% and realworld by ~20%.
Benchmark
These were run on my Macbook Pro M3.
Before the change:
After the change:
Explanation
In the parse function for ZodString, we are calling
getParsedType
(utils.ts) through a layer of indirection (this._getType
->getParsedType
) in order to determine whether the data type is not a string. Since we don't actually need the full range of responses from getParsedType, we can simplify the logic by performing this negative typecheck inline.The same is true for most other parse methods (we perform a
!= type
check which can be moved inline), although making this change for strings is the simplest and probably the largest impact.Summary by CodeRabbit