Support union parameters #585
Merged
+305
−57
Conversation
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
sjorsdonkers
approved these changes
Apr 30, 2025
| // | ||
| // The way we'll do this is that, if there's a direct match, we'll use it | ||
| // If there's a potential match, we'll keep looking for a direct match | ||
| // and only use the (first) potential match as a fallback. |
There was a problem hiding this comment.
Just the first one as defined in the order of the union variants? No, type based or like an integer is rather a float than a string. I would expect JS to be highly unpredictable instead of something as straightforward as just the first. Dunno :)
I think this is going to be very hard to get exactly right in the first implementation
There was a problem hiding this comment.
Order is used as a tie-breaker only when no exact match is available.
if you have a union:
a: i32,
b: bool
c: f32
And you pass a true, then it'll map to b. If you pass a 1.445, it'll map to c. But if you pass "hello" which, in JavaScript, can safely be coerced to an integer, a boolean and a float, it'll pick a.
krichprollsch
approved these changes
May 1, 2025
There's ambiguity in mapping due to the flexible nature of JavaScript. Hopefully most types are unambiguous, like a string or am *parser.Node. We need to "probe" each field to see if it's a possible candidate for the JS value. On a perfect match, we stop probing and set the appropriate union field. There are 2 levels of possible matches: candidate and coerce. A "candidate" match has higher precedence. This is necessary because, in JavaScript, a lot of things can be coerced to a lot of other, seemingly wrong, things. For example, say we have this union: a: i32, b: bool, Field `a` is a perfect match for the value 123. And field b is a coerce match (because, yes, 123 can be coerced to a boolean). So we map it to `a`. Field `a` is a candidate match for the value 34.2, because float -> int are both "Numbers" in JavaScript. And field b is a coerce match. So we map it to `a`. Both field `a` and field `b` are coerce matches for "hello". So we map it to `a` because it's declared first (this relies on how Zig currently works, but I don't think the ordering of type declarations is guaranteed, so that's an issue).
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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's ambiguity in mapping due to the flexible nature of JavaScript. Hopefully most types are unambiguous, like a string or a
*parser.Node.We need to "probe" each field to see if it's a possible candidate for the JS value. On a perfect match, we stop probing and set the appropriate union field. There are 2 levels of possible matches: candidate and coerce. A "candidate" match has higher precedence. This is necessary because, in JavaScript, a lot of things can be coerced to a lot of other, seemingly wrong, things.
For example, say we have this union:
a: i32,
b: bool,
Field
ais a perfect match for the value 123. And field b is a coerce match (because, yes, 123 can be coerced to a boolean). So we map it toa.Field
ais a candidate match for the value 34.2, because float -> int are both "Numbers" in JavaScript. And field b is a coerce match. So we map it toa.Both field
aand fieldbare coerce matches for "hello". So we map it toabecause it's declared first (this relies on how Zig currently works, but I don't think the ordering of type declarations is guaranteed, so that's an issue).