-
Notifications
You must be signed in to change notification settings - Fork 28
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
cell destructuring #88
base: main
Are you sure you want to change the base?
Conversation
This initial implementation looks good, and the functionality it provides is highly desirable. While there are going to be a few small considerations to keep in mind in the UI when the adopt destructuring (permalink names for cells, display of multiple names in the cell menu, embed code generation for a cell that exposes multiple values...), those are all fairly easily surmountable. I'd still like to register my (already verbally discussed) mild objection to this syntax. We’re appropriating the parenthetical declaration of destructured function arguments, instead of the JavaScript canonical parenthetical wrapping of the entire destructuring assignment. I'd prefer that we hew to JavaScript if we’re going to add this. For example, JavaScript: let a, b;
...
({a, b} = expression); But in this proposal: ({a, b}) = expression I'd prefer: ({a, b} = expression) ... and of course, dropping the parentheses altogether would be best. There’s quite a bit more discussion about the ES6 parse choices of |
Thanks for the references and feedback! It feels unfair (to me) to say that one of these approaches “hews to JavaScript” while the other doesn’t. Named cells are not assignment expressions; they declare a binding between one or more identifiers and a function body, the latter being invoked automatically by the runtime. In my view cells are closest to arrow functions. ({foo, bar}) => body The difference is that the name of the cell binds to the output rather than the input. Hence, my proposal to put the parens around the binding. ({foo, bar}) = body The other (lexical binding) precedent to consider is a variable declaration. let {foo, bar} = expression; We could use a keyword to make it immediately clear whether a cell is named and avoid ambiguity. That could be a new keyword (such as const {foo, bar} = body Simple names could be written this way, too, if desired. const foo = body And you could possibly have a const mutable, which is a little weird. const mutable foo = body The reason I’ve avoided a keyword (from the beginning) is that it feels like a lot of noise for very little signal. Cells are already delineated as separate scripts, and in the common case only a couple tokens are needed to determine whether a cell is named. Introducing a new keyword to support destructuring feels more disruptive than necessary. |
I realized that both of these proposals (foo) = body (foo = body) are potentially not backwards-compatible with mutable assignments, though my guess there are few (if any) cells that would break. In the parens-around-binding proposal, a mutable assignment could change to a mutable declaration. (mutable foo) = body In the parens-around-cell proposal, a mutable assignment could change to a mutable declaration, too. (mutable foo = body) Also, in the parens-around-binding proposal, previously invalid syntax (assignment to constant variable foo) could become a valid cell declaration. (foo) = 42 This is true of the parens-around-cell proposal, too. (foo = 42) Personally, I find the parens-around-cell proposal looks more like an assignment expression and less like a lexical binding, and hence I prefer the parens around the binding. 😄 |
186e141
to
c3145db
Compare
c3145db
to
7f5a259
Compare
716309a
to
98467c8
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.
Amazing! All the test output yielded what I would expect. 👍 Glanced at the related compiler updates you're working on, but curious to dig more into that, too.
Any chance that this can be merged? Just going through the feedback repo and I think this would close observablehq/feedback#48 |
Honestly can't wait for this feature to be released. |
Destructuring allows a cell to expose multiple values.
This PR allows the cell.id to be an ObjectPattern or an ArrayPattern. There’s still a fair amount of work on the compiler side to turn this into the corresponding set of variables, but this serves as the language proposal.
TODO