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

Combine API parameters into a single request object + options #15

Closed
adrianba opened this issue Mar 3, 2016 · 10 comments
Closed

Combine API parameters into a single request object + options #15

adrianba opened this issue Mar 3, 2016 · 10 comments

Comments

@adrianba
Copy link
Contributor

adrianba commented Mar 3, 2016

This issue comes from WICG/paymentrequest#41.

There is an open issue about whether supportedMethods, details, and data should be combined into a single object.

@adrianhopebailie
Copy link
Collaborator

In the TAG review @triblondon said:

I'm strongly in favour of these being merged into a single object, which provides greater extensibility, and is more in line with the direction of travel of JavaScript now that destructuring makes dealing with large object structures easy.

@msporny
Copy link
Member

msporny commented Apr 4, 2016

I agree strongly with @triblondon wrt. merging arguments into a single object. I was going to submit a PR to do just that after FPWD was out, especially since it would be nice if the messages looked the same in the Web Payments HTTP API and the Browser API. You can't easily split arguments out in an HTTP call unless you put some stuff in as URL parameters and the rest in the body.

The Web Payments Community Group specs used the single object pattern so that Web Payments messages could be shared between the Browser API and the HTTP API.

@halindrome
Copy link
Contributor

+1 to having a single object.

@adrianhopebailie
Copy link
Collaborator

@sideshowbarker said:

Thanks @triblondon. This seems like an important issue, so I would like to hear from more of the TAG (e.g., @mnot, @travisleithead) about whether this is how the TAG recommends that groups design JavaScript APIs. Do either https://www.w3.org/TR/api-design/ or https://w3ctag.github.io/design-principles/ provide some relevant guidance on this?

@adrianhopebailie
Copy link
Collaborator

I'd like the inputs to be split between 2 parameters because some are part of the payment request that goes to the app and others are explicitly intended to only be consumed by the browser (such as requestShipping).

Since the line item stuff in PaymentDetails is purely for display purposes by the browser, especially if we pull the total out into it's own item, I'd not include it in the payment request that goes to the payment app.

PROPOSAL

  • Move items and shippingOptions from PaymentDetails into PaymentOptions
  • Drop the supportedMethods parameter.
  • Define a PaymentMethod object which has:
    • A required member called identifiers which is a sequence of strings, each a payment method identifier.
    • A data member which is a generic object and contains the payment method specific data for the payment method.
  • Add a supportedMethods member to PaymentDetails which is a sequence of PaymentMethod objects.

adrianhopebailie added a commit to adrianhopebailie/browser-payment-api that referenced this issue Apr 10, 2016
Merged PaymentRequest constructor params to allow for options (only
processed by the UA) and details (passed to the payment app).
This commit also proposes solutions to:
w3c#4, w3c#15 and w3c#18
@adrianhopebailie
Copy link
Collaborator

PR #133 reflects my proposal above and also has some nice side-effects which solve #3, allow for a simple solution to #4 and #18

@adrianhopebailie adrianhopebailie modified the milestones: Priority: Medium, Priority: Low, Priority: High Apr 10, 2016
@adrianba
Copy link
Contributor Author

I do not support merging these arguments. My reasoning is described in response to PR #133.

@adrianhopebailie adrianhopebailie modified the milestone: Discuss on Call - 28 April Apr 28, 2016
@adrianhopebailie
Copy link
Collaborator

Partially addressed by PR #162.

Is there a desire to still split the contents of details into the other two parameters?

@msporny
Copy link
Member

msporny commented Jun 16, 2016

We're planning on submitting a PR for this, premature to close the issue.

@adrianhopebailie
Copy link
Collaborator

The issue was stale.
Happy to re-open when there is a concrete PR.

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

No branches or pull requests

5 participants