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.
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
feat: support more input parsers #1061
feat: support more input parsers #1061
Changes from 17 commits
63372a5
6b05be4
42f7e66
c4f4a34
238a32a
b6417f9
8fe3b71
55f8352
0bf97a1
e7d0b49
41a4ef1
3c58112
752bbc5
19af73f
300a503
d65112b
434097c
2dbadf4
b179776
fe22f96
36acdf3
8b92ab0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@tim-smart is there some lower-level subtype of schema we should use that would allow these as well?
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.
optional<Schema>
is a property signature (only found inside of Schema.Struct etc), so not something you would pass directly.Is there an usage example somewhere?
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.
hmm that's not what I can see: https://effect.website/play#d5ee870d7ca7
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.
https://github.com/pingdotgg/uploadthing/pull/1061/files#diff-6d27c61bead2229bd300fc7cb86004f70cdf7530fac338099880dc4e82307cf4R55-R77
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.
ah
Schema.Any
might work! https://effect.website/play#dbc0aa398730 - just gotta see how to connect that to what the other parsers we useThere 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.
or
Schema.Schema.Variance<Out,In,never>
- but that doesn't pass the runtime checkSchema.isSchema
https://effect.website/play#539624469d65
or is that what you were saying with
Schema.optional
being on properties only? How do you do optoinal objects then?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.
There are modifiers like Schema.NullOr() etc.
You can use Schema.AnyNoContext if you want to prevent requirements from being used.
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.
hmm i struggle getting this to work so I think we'll just ship it with the restriction that the outer type must be a
Schema.Schema
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.
💡 Codebase verification
Effect Schema optional type wrapping requires different implementation approach
The FIXME comment is valid. The codebase shows that Effect Schema is used differently in the handler implementation, where optional fields are handled through
Schema.optional
andSchema.optionalWith
in a piped manner. The test's current approach of wrapping the entire object schema doesn't align with this pattern. This is a legitimate technical limitation that requires a different implementation approach rather than a documentation update.packages/uploadthing/src/internal/handler.ts
demonstrates the correct pattern:SomeType.pipe(Schema.optional)
for optional fields🔗 Analysis chain
Address Effect Schema optional type compatibility
The FIXME comment indicates that Effect Schema's optional type wrapping is incompatible with the current approach. This limitation should be:
Let's verify if this is a known issue:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 476
Script:
Length of output: 2154
Script:
Length of output: 2056