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

Add Promise.allSettled() #4385

Merged
merged 2 commits into from
Jul 9, 2019
Merged

Conversation

a2sheppy
Copy link
Contributor

This patch adds this method to the Promise.json data with what should be
correct version information for Chrome, Edge, and Firefox. There is no
specification URL because currently allSettled() is not yet part of the
spec, and the schema rejects the URL of the proposal.

Sources:

This patch adds this method to the Promise.json data with what should be
correct version information for Chrome, Edge, and Firefox. There is no
specification URL because currently `allSettled()` is not yet part of the
spec, and the schema rejects the URL of the proposal.

Sources:
* https://tc39.es/proposal-promise-allSettled/
* Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1539694
* Chrome status: https://www.chromestatus.com/features/5547381053456384
* Edge: https://developer.microsoft.com/en-us/microsoft-edge/platform/catalog/?page=1&q=allSettled
@a2sheppy a2sheppy requested a review from Elchi3 June 26, 2019 16:32
@a2sheppy a2sheppy changed the title Add Promise.allSettled() DO NOT MERGE - Add Promise.allSettled() Jun 26, 2019
@a2sheppy
Copy link
Contributor Author

Don't merge this. Turns out it's Nightly-only for now.

@a2sheppy a2sheppy removed the request for review from Elchi3 June 26, 2019 20:36
@queengooborg queengooborg added data:js 📟 Compat data for JS/ECMAScript features. https://developer.mozilla.org/docs/Web/JavaScript not ready ⛔ This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. labels Jun 26, 2019
@queengooborg
Copy link
Collaborator

I know we’ve added features and set their notes as “Available only in nightly builds” before. Perhaps we could also do the same here?

@ddbeck
Copy link
Collaborator

ddbeck commented Jul 5, 2019

@vinyldarkscratch what to do about channel-gated features is a long-standing issue: #338. Personally, I've always seen the nightly notes a hack around the lack of consensus on the channel schema question. I agree with not merging this until the feature is actually headed to a release version of the browser

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

We should just set it to false for Firefox for now. We have this documented, so it needs a compat table, so this data should land. For the Nightly case, I think it makes no sense if it will be nightly only for just one release to have it in the data. I wouldn't bother with it in this case.
Generally, I agree with Daniel, the most interesting data is what is shipping to real users and so much like the compile flag cases, my feeling is that Nightly only data is not very useful. We should study this some more and clean the data if necessary in the issue linked from here.

We're also missing a spec_url. Please add it.

"version_added": false
},
"firefox": {
"version_added": "68"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"version_added": "68"
"version_added": false

"version_added": "68"
},
"firefox_android": {
"version_added": "68"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"version_added": "68"
"version_added": false

@Elchi3 Elchi3 changed the title DO NOT MERGE - Add Promise.allSettled() Add Promise.allSettled() Jul 8, 2019
@Elchi3 Elchi3 removed the not ready ⛔ This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. label Jul 8, 2019
@Elchi3
Copy link
Member

Elchi3 commented Jul 8, 2019

There is no specification URL because currently allSettled() is not yet part of the
spec, and the schema rejects the URL of the proposal.

Sorry, didn't see this in my review (the review button is there when you look at the diff and you don't see the thread then, bugs me for longer with GitHub already). Try this URL that has a deep link which spec_url requires: https://tc39.es/proposal-promise-allSettled/#sec-promise.allsettled

(I just noticed we only have spec_url documented in the schema and not in our schema docs. It reads "An optional URL or array of URLs, each of which is for a specific part of a specification in which this feature is defined. Each URL must contain a fragment identifier.". I filed this doc issue as #4466)

Set Firefox to `false` and added a `spec_url` entry referencing the
proposal. This will need to be updated when `allSettled()` is added
to the spec proper.
Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Thank you! 👍

@Elchi3 Elchi3 merged commit da298de into mdn:master Jul 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:js 📟 Compat data for JS/ECMAScript features. https://developer.mozilla.org/docs/Web/JavaScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants