-
Notifications
You must be signed in to change notification settings - Fork 457
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
Gentype: handle null/nullable/undefined from Stdlib #7132
Conversation
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.
This looks great.
Before merging, or perhaps after: we should settle on where these types exist.
Having them exist in 3 or 4 places is an opportunity for issues down the line in tooling (editor, hovering) and compiler updates.
What's the ground truth here?
I guess for each type, we should have a preferred way of expressing that type, at least.
Ideally, a unique way, but that seems difficult to achieve while maintaining backwards compatibility.
Thoughts @zth ?
If we follow my current approach in #7129, the ground truth would be in the In any case, we still have to be able to handle the different aliases. |
Will keep this open until #7129 is done, as we'll run into the same issue with any other types we alias in Pervasives (e.g. |
abfe2bd
to
d270512
Compare
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.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Syntax Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05
.
Benchmark suite | Current: d270512 | Previous: 80744a5 | Ratio |
---|---|---|---|
Print HeroGraphic.res - time/run |
13.609067813333333 ms |
12.308145193333333 ms |
1.11 |
This comment was automatically generated by workflow using github-action-benchmark.
Rebased after #7285, please re-review. |
No description provided.