Skip to content
This repository has been archived by the owner on Mar 31, 2018. It is now read-only.

Add a glossary #14

Merged
merged 12 commits into from
Feb 16, 2016
Merged

Add a glossary #14

merged 12 commits into from
Feb 16, 2016

Conversation

chrisdickinson
Copy link
Contributor

This defines common terms for discussion.

/cc'ing @misterdjules, @petkaantonov, @kriskowal for review.

@chrisdickinson
Copy link
Contributor Author

Fixes #11.

@petkaantonov
Copy link
Contributor

To complement the producer defined operational/programmer errors, the consumer defined unexpected/expected errors should be added as well


A function passed to `Promise#then` or `Promise#catch`. Receives as an argument
the parent promise's resolved value or rejected error. Represents a new
`Promise`. May return a value `T` or `Promise<T>`, which resolves the new
Copy link

Choose a reason for hiding this comment

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

This already duplicates the Settling section a bit, but might I suggest:

A handler which returns a value T or Promise<T> will resolve the new promise with T. One which throws an error R, or returns a Promise which itself eventually rejects (or has already rejected) with R, will reject the new promise with R.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Added verbatim.

@chrisdickinson
Copy link
Contributor Author

@petkaantonov updated.


```js
const p1 = new Promise(() => { throw new Error('xyz')})
const p2.catch(() => {})

Choose a reason for hiding this comment

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

Did you mean p2 = p1.catch...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! Thanks!

> be avoided by changing the code. They can never be handled properly (since by
> definition the code in question is broken).

By definition, these errors are unexpected.
Copy link
Member

Choose a reason for hiding this comment

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

It's worth mentioning that these errors are not always synchronous and that operational errors are not always asynchronous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, this definition describes error states, but not their propagation. I'll add a note stating that this section pointedly doesn't mentioned propagation types, and note the recommendations of the error symposium.

const p = new Promise(() => {})

const p2 = p.then(function SuccessHandler(value) {
// fired if p resolves,
Copy link
Member

Choose a reason for hiding this comment

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

"fulfills"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

@benjamingr
Copy link
Member

I left some comments. I also think this is missing a lot of terms from the issue but that can be handled later at a later PR.

@chrisdickinson
Copy link
Contributor Author

@benjamingr: Thank you for the thorough review. How would you feel about merging this as-is, then fixing up the promises section with subsequent PRs?


```js
const p1 = new Promise(() => { throw new Error('xyz')})
const p2 = p1.catch(() => {})
Copy link
Member

Choose a reason for hiding this comment

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

Why not:

const p1 = new Promise(() => { throw new Error('xyz')});
const p2 = p1.catch((err) => {
    // err === [Error: xyz]
});

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. Thanks!


```js
const p1 = new Promise(() => { throw new Error('xyz')})
const p2 = p1.catch(() => { /* err === [Error: xyz] */ })
Copy link
Member

Choose a reason for hiding this comment

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

() => { /* err === [Error: xyz] */ }

should be

(err) => { /* err === [Error: xyz] */ }

;-)

@benjamingr
Copy link
Member

I think another iteration would be useful - and after another rounds of comments we can gladly merge.

@chrisdickinson
Copy link
Contributor Author

Thank you for the review. In the interest of freeing others up to work on this in subsequent PRs (and to free myself to work on the other required bits!), I'm going to merge this.

chrisdickinson added a commit that referenced this pull request Feb 16, 2016
@chrisdickinson chrisdickinson merged commit dea9dc1 into master Feb 16, 2016
@chrisdickinson chrisdickinson deleted the add-glossary branch February 16, 2016 18:10
@petkaantonov
Copy link
Contributor

I strongly object the definition you used for unexpected/expected errors, let's please work it out before merging. See my inline comment

* [Handler](#handler)
* [Settling](#settling)
* [Rejection / reject](#rejection--reject)
* [Resolving / resolve](#resolving--resolve)

Choose a reason for hiding this comment

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

I find it useful to organize this as:

  • promise states: pending or settled
  • settled states: fulfilled or rejected
  • defer (to create a pending promise, or to resolve a pending promise with another pending promise)
  • resolve (to take a pending promise and, having made some progress, to designate another promise (albeit a fulfilled, rejected, or pending promise) as its resolution, responsible for eventually notifying its observers when it is ultimately settled)
  • fulfill (to create a fulfilled promise, or resolve and settle a pending promise with a fulfilled promise, eventually notifying all of its eventual handlers of the fulfillment value)
  • reject (to create a rejected promise, or resolve and settle a pending promise with a rejected promise, eventually notifying all of its eventual handlers of the rejection error)

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

Successfully merging this pull request may close these issues.

8 participants