Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Editorial: create closures using the standard, verbose ES pattern #29

Closed
domenic opened this issue Jun 19, 2017 · 6 comments
Closed

Editorial: create closures using the standard, verbose ES pattern #29

domenic opened this issue Jun 19, 2017 · 6 comments

Comments

@domenic
Copy link
Member

domenic commented Jun 19, 2017

Although I've filed an issue to make this easier (tc39/ecma262#933), in the meantime, to get this spec ready for advancement, it should really use the standard style for closures.

You can see some examples in e.g. https://tc39.github.io/proposal-async-iteration/#await

@domenic
Copy link
Member Author

domenic commented Jun 19, 2017

They should also be nested as sub-clauses underneath the Promise.prototype.finally clause, similar to how Promise.all works

@domenic
Copy link
Member Author

domenic commented Jun 19, 2017

This may also be required for the valueThunk/thrower "closures". Maybe @bterlson can comment (since his review is required anyway) as to whether we can let those slip by...

@domenic domenic changed the title Create closures using the standard, verbose ES pattern Editorial: create closures using the standard, verbose ES pattern Jun 19, 2017
@ljharb
Copy link
Member

ljharb commented Jun 19, 2017

The style I'm using now was in the spec until a recent Promise refactor; iow, it's how ES2015 worded it. The refactor wasn't about closure-styles; why wouldn't we be able to continue using that style prior to tc39/ecma262#933 being resolved?

(I'll definitely fix the nesting regardless)

@domenic
Copy link
Member Author

domenic commented Jun 19, 2017

There has never been anything in the spec that creates closures using the words "Return a function that takes one argument, value, and when invoked, performs the following steps:" and implicitly closes over variables in the algorithm (onFinally).

@ljharb
Copy link
Member

ljharb commented Jun 20, 2017

I'm referring to ES2015, prior to my refactor - it definitely didn't have a closure, but used "equivalent to" language.

I can certainly change it to use the more verbose form that uses an internal slot instead of closure semantics to provide onFInally, it just seems strictly less valuable and readable than this proposal's current language.

@domenic
Copy link
Member Author

domenic commented Jun 20, 2017

I don't think consistency is strictly less valuable.

ljharb added a commit that referenced this issue Jun 20, 2017
 - Use SpeciesConstructor to ensure subclasses are respected
 - Observably invoke `.then` with the result of the `onFinally` function
 - Refactor closures to use the more verbose form the spec currently favors.

Fixes #29. Fixes #32. Fixes #33.
ljharb added a commit that referenced this issue Jun 21, 2017
 - Use SpeciesConstructor to ensure subclasses are respected
 - Observably invoke `.then` with the result of the `onFinally` function
 - Refactor closures to use the more verbose form the spec currently favors.

Fixes #29. Fixes #32. Fixes #33.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants