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

Clarify promise rejection value #76

Closed
LinusU opened this issue Apr 3, 2018 · 4 comments
Closed

Clarify promise rejection value #76

LinusU opened this issue Apr 3, 2018 · 4 comments

Comments

@LinusU
Copy link
Contributor

LinusU commented Apr 3, 2018

I'm a bit unsure of what to expect back from the async clipboard API when there is a failure. The explainer.adoc here doesn't use the rejection value at all, and the spec simply says "reject |p|".

1. If |r| is not "granted", then reject |p|

Is the intention that the Promise should simply reject with the value undefined instead of an Error instance?

This is how we have interpreted it here: feross/clipboard-copy#24

I feel that this goes against best practices, and I think there is a ton of code in the wild that does something similar to:

runAllMythings().catch((err) => {
  console.log(err.stack)
})

Wouldn't it be nice to reject with an error, maybe reject(new Error('Persmission for writing to clipboard was not granted'))?

@gked
Copy link

gked commented May 9, 2018

@LinusU clipboard API should probably be able to relay PermissionState value back to the developer. @garykac what do you think if we added new property to DataTransfer interface? Something like readonly attribute PermissionState state; with some default value.

@garykac
Copy link
Member

garykac commented May 9, 2018

We should definutely return better DOMExceptions, but I don't think we want to touch DataTransfer.

E.g., the spec should be updated to read:

   1. If |r| is not granted, then reject |p| with a "NotAllowedError" DOMException.

While the spec currently uses a DataTransfer object for read/write, I believe that we'll need to change that to a more appropriate object because of recent issues that were raised. Specifically #41 and #44 and this comment from the TAG review.

@gked
Copy link

gked commented May 9, 2018

@garykac NotAllowedError seems a bit ambiguous. But I guess for the lack of a better alternative, I think, this should work. I am checking with Travis about why DataTransfer was questioned during TAG review.

@garykac
Copy link
Member

garykac commented May 10, 2018

Re: "NotAllowedError", see the comments here for the recommendation to use this for denied requests.

Questions about the appropriateness of DataTransfer came about before the TAG review because it doesn't quite match what we'll need to do for the clipboard:

  • Images can be on the clipboard multiple times (in different formats)
  • We'll need placeholders for delay-generated data

We could hack these things on top of DataTransfer, but that's not necessarily the best approach.

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

No branches or pull requests

3 participants