-
Notifications
You must be signed in to change notification settings - Fork 448
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
Record representation is unstable #7031
Comments
One possible fix is to add entry sorting at runtime, which will add serious performance degradation wherever optional fields are used (e.g. JSXv4) Another way is to initialize optional fields to |
Coercion for records also break this assumption (more fields can be on the object than what's represented by the record). Honestly I think these are just quirks we'll have to live with in some form if we want optional record fields (which we do) and coercion (which we also do). Semi-unrelated, but one thing I've thought about before is whether one could have an annotation for a record definition type that disallows optional fields and/or coercion. Or one annotation for each thing. That way you could set up a "protected" record that you know will have the exact runtime shape you expect (unless it's from an external, but 🤷 ). |
I think we should mention this caveat in the docs for optional fields. I rarely use record as the key (or a complex type), but who knows. |
Actually that was never true because of FFI. |
IMO it should have runtime effects, not just be a simple type assertion.
Output should be I know it has a tradeoff, but we need to care a little more for integrity, even if it needs runtime. |
At least when using FFI explicitly, users can be quite careful about it. But in pure ReScript-produced code, this behavior is somewhat surprising to me. |
If so, we should deprecate supporting comparison operations ( |
This is definitely a good idea. I wonder however if anyone is using them. That type of comparison could be quite powerful ( |
OCaml std users may be using it, but there's no guarantee that it will work without bugs. We're dropping OCaml std, and Belt is a little better because it makes users explicitly build their own But still, using However, the advantage of module functions is that it makes it easy to replace the implementation. Users can get both correctness and performance by replacing in a custom implementation. Then we could extend language support something like |
Since we added support for optional fields on records, a crucial assumption was broken: up until then, records can be expected to have a fixed layout, as its definition.
So records with the same values for the same keys always had the same hash.
However, by introducing optional fields, we can easily create values that pass the equality check but have different hashes.
https://rescript-lang.org/try?version=v11.1.4&code=C4TwDgpgBMULxQN4CgpQIYC4oGdgCcBLAOwHMAaVKAIwH5s8izK0BjBgki5AX2WQA2EWADcAjPCRUsUAETpZLKOzmtFvfkNEAmSSjQA6I+KXVss6ur6DhUEQGY908wtPnLSlbLWVr6HDgQ+MAAFAAS-gAWwNQCBpFRIeIAlPAIETjRsfGJDsnJyEA
v2
andv3
have the same values for the same keys but in different order.Obj.entries(v2)
=>[['a', 'a'], ['c', 'c'], ['b', 'b']]
Obj.entries(v3)
=>[['a', 'a'], ['b', 'b'], ['c', 'c']]
This literally means that users can't use a record as a key in a hashtable or other data structure. In other words, it's not a record.
The text was updated successfully, but these errors were encountered: