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

Make example code import and use the right modules in the right way #555

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/rsvp/hash-settled.js
Expand Up @@ -87,14 +87,14 @@ HashSettled.prototype._setResultAt = setSettledResult;
Example:

```javascript
import Promise, { hashSettled, resolve } from 'rsvp';
import { hashSettled, resolve } from 'rsvp';

function MyConstructor(){
this.example = resolve('Example');
}

MyConstructor.prototype = {
protoProperty: Promise.resolve('Proto Property')
protoProperty: resolve('Proto Property')
};

let myObject = new MyConstructor();
Expand Down
4 changes: 4 additions & 0 deletions lib/rsvp/promise.js
Expand Up @@ -56,6 +56,8 @@ function needsNew() {
------------

```js
import { Promise } from 'rsvp';

let promise = new Promise(function(resolve, reject) {
// on success
resolve(value);
Expand All @@ -78,6 +80,8 @@ function needsNew() {
`XMLHttpRequest`s.

```js
import { Promise } from 'rsvp';

function getJSON(url) {
return new Promise(function(resolve, reject){
let xhr = new XMLHttpRequest();
Expand Down
10 changes: 5 additions & 5 deletions lib/rsvp/promise/all.js
@@ -1,22 +1,22 @@
import Enumerator from '../enumerator';

/**
`Promise.all` accepts an array of promises, and returns a new promise which
`all` accepts an array of promises, and returns a new promise which
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';
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.all

Copy link
Author

Choose a reason for hiding this comment

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

@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?


let promise1 = resolve(1);
let promise2 = resolve(2);
let promise3 = resolve(3);
let promises = [ promise1, promise2, promise3 ];

Promise.all(promises).then(function(array){
all(promises).then(function(array){
// The array here would be [ 1, 2, 3 ];
});
```
Expand All @@ -28,14 +28,14 @@ import Enumerator from '../enumerator';
Example:

```javascript
import Promise, { resolve, reject } from 'rsvp';
import { all, resolve, reject } from 'rsvp';
Copy link
Collaborator

Choose a reason for hiding this comment

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

^


let promise1 = resolve(1);
let promise2 = reject(new Error("2"));
let promise3 = reject(new Error("3"));
let promises = [ promise1, promise2, promise3 ];

Promise.all(promises).then(function(array){
all(promises).then(function(array){
// Code here never runs because there are rejected promises!
}, function(error) {
// error.message === "2"
Expand Down
16 changes: 8 additions & 8 deletions lib/rsvp/promise/race.js
Expand Up @@ -7,13 +7,13 @@ import {
} from '../-internal';

/**
`Promise.race` returns a new promise which is settled in the same way as the
`race` returns a new promise which is settled in the same way as the
first passed promise to settle.

Example:

```javascript
import Promise from 'rsvp';
import { Promise, race } from 'rsvp';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's instead document Promise.race to better spec parity


let promise1 = new Promise(function(resolve, reject){
setTimeout(function(){
Expand All @@ -27,20 +27,20 @@ import {
}, 100);
});

Promise.race([promise1, promise2]).then(function(result){
race([promise1, promise2]).then(function(result){
// result === 'promise 2' because it was resolved before promise1
// was resolved.
});
```

`Promise.race` is deterministic in that only the state of the first
`race` is deterministic in that only the state of the first
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';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's instead document Promise.race to better spec parity


let promise1 = new Promise(function(resolve, reject){
setTimeout(function(){
Expand All @@ -54,7 +54,7 @@ import {
}, 100);
});

Promise.race([promise1, promise2]).then(function(result){
race([promise1, promise2]).then(function(result){
// Code here never runs
}, function(reason){
// reason.message === 'promise 2' because promise 2 became rejected before
Expand All @@ -65,9 +65,9 @@ import {
An example real-world use case is implementing timeouts:

```javascript
import Promise from 'rsvp';
import { Promise, race } from 'rsvp';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's instead document Promise.race to better spec parity


Promise.race([ajax('foo.json'), timeout(5000)])
race([ajax('foo.json'), timeout(5000)])
```

@method race
Expand Down
8 changes: 4 additions & 4 deletions lib/rsvp/promise/reject.js
Expand Up @@ -4,11 +4,11 @@ import {
} from '../-internal';

/**
`Promise.reject` returns a promise rejected with the passed `reason`.
`reject` returns a promise rejected with the passed `reason`.
It is shorthand for the following:

```javascript
import Promise from 'rsvp';
import { Promise } from 'rsvp';

let promise = new Promise(function(resolve, reject){
reject(new Error('WHOOPS'));
Expand All @@ -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';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's instead document Promise.reject to better spec parity


let promise = Promise.reject(new Error('WHOOPS'));
let promise = reject(new Error('WHOOPS'));

promise.then(function(value){
// Code here doesn't run because the promise is rejected!
Expand Down
8 changes: 4 additions & 4 deletions lib/rsvp/promise/resolve.js
Expand Up @@ -4,11 +4,11 @@ import {
} from '../-internal';

/**
`Promise.resolve` returns a promise that will become resolved with the
`resolve` returns a promise that will become resolved with the
passed `value`. It is shorthand for the following:

```javascript
import Promise from 'rsvp';
import { Promise } from 'rsvp';

let promise = new Promise(function(resolve, reject){
resolve(1);
Expand All @@ -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';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's instead document Promise.resolve to better spec parity


let promise = RSVP.Promise.resolve(1);
let promise = resolve(1);

promise.then(function(value){
// value === 1
Expand Down
2 changes: 1 addition & 1 deletion lib/rsvp/rethrow.js
Expand Up @@ -11,7 +11,7 @@
error again so the error can be handled by the promise per the spec.

```javascript
import { rethrow } from 'rsvp';
import { Promise, rethrow } from 'rsvp';

function throws(){
throw new Error('Whoops!');
Expand Down