Skip to content
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

Editorial: create EnqueuePromiseReactions abstract operation. #592

Closed
wants to merge 1 commit into from

Conversation

ljharb
Copy link
Member

@ljharb ljharb commented Jun 2, 2016

This unobservable spec refactor allows the Promise#finally proposal to make a much smaller change, so it can reuse some of the logic in PerformPromiseThen.

(following up on #584)

This unobservable spec refactor allows the `Promise#finally` proposal to make a much smaller change, so it can reuse some of the logic in PerformPromiseThen.

(following up on tc39#584)
<h1>EnqueuePromiseReactions ( _promise_, _fulfillReaction_, _rejectReaction_, _resultCapability_ )</h1>
<p>The abstract operation EnqueuePromiseReactions enqueues PromiseJobs with the provided PromiseReaction records on _promise_. The result is _resultCapability_'s promise.</p>
<emu-alg>
1. Assert: IsPromise(_promise_) is *true*.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need type-testing assertions for all arguments? :-/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they're both necessary in the algorithm, and unobservably skippable in all implementations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're definitely not necessary; nothing will change depending on their presence or absence. The question is whether they're helpful or annoying/distracting, and I claim the latter. In general abstract ops don't do type assertions on their arguments, except in cases where it could be potentially unclear (e.g. what type of completion record they will end up getting). This has only one call site so it seems super-unnecessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably #545 will help this :-P

@bterlson
Copy link
Member

bterlson commented Jun 2, 2016

Sympathetic to making the finally spec easier to write but I don't think this will help your spec text be much more understandable and it doesn't stand on its own without P#finally. Let's just include this refactoring with the P#finally proposal!

@bterlson bterlson closed this Jun 2, 2016
@ljharb ljharb deleted the ljharb/enqueue_promise_reactions branch June 2, 2016 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants