-
Notifications
You must be signed in to change notification settings - Fork 2
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
What happens if the deep path does not exist in the value that is spread? #4
Comments
This would also need to apply in the reverse: const rec = #{ a: 1 };
const two = #{ [0]: 2, ...rec }; // TypeError, as you described const rec = #{ a: #[1,2,3] };
const two = #{ ...rec, a[0]: 2 }; // two === # { a: #[2,2,3] }
const three = #{ ...rec, a.foo }; // TypeError, because we can't add a non number-like key to a tuple |
const rec = #{ a: 1 };
const two = #{ [0]: 2, ...rec }; // TypeError, as you described I don't understand this case. This one is clear: The record will have a property named "0". This is just the direct semantics of the record proposal. Remember, in all these cases, that you can use deep paths without spread. (There should probably be examples of this in the explainer.) So, #{ a[0]: 1 }
// Two possibilities for expansion:
#{ a: #{ [0]: 1 } }
#{ a: #[ 1 ] } I agree that adding a non-number-like property to a Tuple also needs to be a TypeError. |
I've added a couple of FAQ entries to clarify this, let me know if you think they are sufficient. |
The FAQ entry looks good, but I'd suggest we ban all non-existent paths, not just numeric-like ones (just to keep it simple and consistent, but I'd be open to reconsidering). |
Good point, that does simplify things somewhat. I've updated it again to reflect that. |
Minor suggestion: I think it'd be clearest if you opened with an example of such a manipulation (doesn't matter whether it's numeric or not) and state that it's a TypeError. Then, the rationale can be below it. Materialization is never a thing, so we can start by saying that, rather than getting into the nitty-gritty of how it would work if it did exist. |
good idea! I've moved the examples ahead of the details. |
looks good; made some minor edits in #6. |
thank you, the edits look great, merged |
If reconsidered, a possibility would be to use |
@ByteEater-pl In the Records and Tuples proposal, the syntax for accessing Tuples is |
There's no difference between
Indeed, and I don't propose to change it, nor to prevent |
I find this syntax for explicitly asking for materialization to be very confusing. I'd suggest that people use (nested) record literal syntax in this case. |
That also works. And since it's a corner case anyway, nothing else seems indispensable.
Do I understand correctly that you suggest to allow only |
Any materialization would have to be done by the programmer; nothing will come implicitly in this proposal. |
@littledan @rickbutton I've had a look at the FAQ and this issue and I'm still a little confused. Couldn't the number key ambiguity be avoided by just... picking a behaviour and including it in the spec? e.g. "if a deep path with a number key does not already exist, a tuple will be materialized"? People who really want to use number keys in records (pretty rare IRL, I suspect) could just use a slightly more verbose syntax. To use the examples in the explainer: const one = #{ a: #{} };
#{ ...one, a.b.c: "foo" }; // returns #{ a: #{ b: #{ c: "foo" } } }
#{ ...one, a.b[0]: "foo" }; // returns #{ a: #{ b: #["foo"] }
#{ ...one: #{ b: #{ [0]: "foo" } } // what you have to do if you're a weirdo who likes number keys in records I might be missing something though. I'm also confused by this:
Does this mean |
If we restrict deep paths to Records and Tuples, then the difficult part of this question is: If there's a number-like key used in the path, then is a Record or Tuple materialized? Either would be valid--Records can have number-like keys.
Because of this ambiguity, I'd argue we should throw a TypeError in this case. It's always possible to use spread to insert the key first, as well as other operations to test for its presence. But I'd be interested in more feedback on whether this is a very common issue which really needs addressing.
The text was updated successfully, but these errors were encountered: