Make example code import and use the right modules in the right way #555
Conversation
The former example where not technically wrong but pretty misleading, since they suggested `Promise` (the one that can be used in `new Promise`) is the default export of the `rsvp` package when it isn't. Ideally when using the default import it should be called `RSVP` rather then `Promise` but in fact neither of the code example require to use that default export at all so I removed using that in the examples entirely.
|
If the imports aren't working, that sounds like a bug. That should be fixed. |
|
@stefanpenner this is not a bug in rsvp but an error in the documentation. This line: https://github.com/tildeio/rsvp.js/blob/master/lib/rsvp.js#L47 |
|
I see, the goal is for the default export to be |
| is fulfilled with an array of fulfillment values for the passed promises, or | ||
| rejected with the reason of the first passed promise to be rejected. It casts all | ||
| elements of the passed iterable to promises as it runs this algorithm. | ||
|
|
||
| Example: | ||
|
|
||
| ```javascript | ||
| import Promise, { resolve } from 'rsvp'; | ||
| import { all, resolve } from 'rsvp'; |
There was a problem hiding this comment.
In our docs we should ensure import { X } from 'rsvp'; should be either Promise or utility methods unique to RSVP. The import { resolve, all , reject, race } from 'rsvp' are odd, and their is no reason for RSVP to encourage non-standard promise usage patterns.
Let's use all from Promise, as that will make it easier for people to move to native.
import { Promise } from 'rsvp';
Promise.allThere was a problem hiding this comment.
@stefanpenner your example and my changes are only about all but in fact your logic applies for reject, resolve, race and such as well, doesn't it? My idea was to make the imports consistent by importing all the same way as the others were already imported in those examples but I see that it would make even more sense the other way around. In fact I haven't even seen something that RSVP exports which is unique to it but I will look into it once more. I guess in any case I should update the docs to be consistent for all native Promise functions, right?
| @@ -28,14 +28,14 @@ import Enumerator from '../enumerator'; | |||
| Example: | |||
|
|
|||
| ```javascript | |||
| import Promise, { resolve, reject } from 'rsvp'; | |||
| import { all, resolve, reject } from 'rsvp'; | |||
| first passed promise to settle. | ||
|
|
||
| Example: | ||
|
|
||
| ```javascript | ||
| import Promise from 'rsvp'; | ||
| import { Promise, race } from 'rsvp'; |
There was a problem hiding this comment.
Let's instead document Promise.race to better spec parity
| settled promise matters. For example, even if other promises given to the | ||
| `promises` array argument are resolved, but the first settled promise has | ||
| become rejected before the other promises became fulfilled, the returned | ||
| promise will become rejected: | ||
|
|
||
| ```javascript | ||
| import Promise from 'rsvp'; | ||
| import { Promise, race } from 'rsvp'; |
There was a problem hiding this comment.
Let's instead document Promise.race to better spec parity
| @@ -65,9 +65,9 @@ import { | |||
| An example real-world use case is implementing timeouts: | |||
|
|
|||
| ```javascript | |||
| import Promise from 'rsvp'; | |||
| import { Promise, race } from 'rsvp'; | |||
There was a problem hiding this comment.
Let's instead document Promise.race to better spec parity
| @@ -24,9 +24,9 @@ import { | |||
| Instead of writing the above, your code now simply becomes the following: | |||
|
|
|||
| ```javascript | |||
| import Promise from 'rsvp'; | |||
| import { reject } from 'rsvp'; | |||
There was a problem hiding this comment.
Let's instead document Promise.reject to better spec parity
| @@ -22,9 +22,9 @@ import { | |||
| Instead of writing the above, your code now simply becomes the following: | |||
|
|
|||
| ```javascript | |||
| import Promise from 'rsvp'; | |||
| import { resolve } from 'rsvp'; | |||
There was a problem hiding this comment.
Let's instead document Promise.resolve to better spec parity
The former example where not technically wrong but pretty misleading,
since they suggested
Promise(the one that can be used innew Promise) is the default export of thersvppackage when it isn't.Ideally when using the default import it should be called
RSVPratherthen
Promisebut in fact neither of the code example require to usethat default export at all so I removed using that in the examples
entirely.