-
Notifications
You must be signed in to change notification settings - Fork 63
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
PROPOSAL: The PaymentRequest object SHOULD NOT expose internal state information to the developer. Any design that requires developers to manage or understand the request state is a compromise in the API design that should be avoided where possible. #64
Comments
There wasn't enough time to discuss this proposal in the 21 January meeting. |
I don't think this is a helpful way to frame things. If we have use cases that need state, then we need to manage the state. Even in a mostly-stateless API like Fetch, you will notice that the request does have methods. And even then, let's not forget that in JS, attributes can be backed by code:
So the distinction you're talking about is not really well-defined. You could perhaps require that things work for the "POJSO" case, but trying to rule out code in the object is quixotic. |
Agreed that in particular is quixotic. Let me think for a bit and try to rephrase. The general point is "Let's not make this browser API more complicated than it needs to be", and when focusing specifically on just the PaymentRequest object, it feels more complicated than it needs to be because: 1) it requires the developer to be keenly aware of state changes, and 2) it hangs functionality off of the PaymentRequest object that, arguably, complicates how a developer has to manage it. Clearly if we have use cases that need state, then we need to manage state. The assertion is to try to avoid making developers manage that state in the first place and have the implementations do it in the background, if possible. The Web Payments Browser API (from the Web Payments CG) proposes one such way to do that. In that proposal all objects passed to navigator.payments.* are: 1) plain 'ol Javascript Objects, and 2) no state information needs to be exposed to the developer. |
As an aside, @bifurcation is Richard B.? - kinda looks like it from his repos? Just checking we have IPR properly handled because @bifurcation is providing a bunch of good technical feedback that will likely be integrated and we need to make sure IPR is clean (since we've asked other non-WG members not to participate in discussion if they're not members). |
Not really. In most implementations, the developer doesn't even really need to check it. It actually emerged because there are certain actions you just can't take in particular states. For example, let's say that we've now opened up a third party application and the developer tries to abort or cancel. This isn't really possible, as it's now outside the control of the user-agent, at least on particular platforms. So a developer could, if they wanted, check the state before calling cancel, but if they don't, the system will just throw an exception.
Not sure I understand how this is a point separate from point 1. What it does provide is functionality that is necessary if we support the collection of shipping address and the necessary complexity that comes with that.
Yep. |
This is exactly my point. Why do you need to expose the state information in this case if you can get an answer to the question by just trying to cancel and catching the exception? The developers code gets more complicated because now they have to check the state before canceling (note - they have to be aware of the state and knowing when to check if they should cancel or can't cancel - added complexity), and they may even still need to do a try/catch block anyway because other things can cause an exception. This is exactly the sort of stuff that XMLHttpRequest got wrong (be aware of state before calling certain methods, check state, but even when you do check state, things can still go wrong (race conditions) so you still have to try/catch - all resulting in more code and complexity than is necessary). |
This feels like we are developing an API that will encourage developers to use exceptions for flow control. i.e. A violation of the Principal of Least Astonishment |
This proposal has been put on the agenda for tomorrow (28 Jan) but it seems there is not agreement on the wording. @msporny can you try to address @bifurcation's concerns around phrasing before the meeting? |
I've tried to narrow the proposal down to one of the problems (state machine management) w/ the PaymentRequest object in the Google/Microsoft proposal. The Web Payments CG spec doesn't suffer from this problem. There are other issues, but I'll try to address those through other proposals. I have no idea if this addresses @bifurcation's concerns with this particular proposal. |
@zkoch wrote:
I'm asserting that not only do you not need to expose state information on the PaymentRequest (point #1), but you also don't need methods on it like .show(), .complete(), and .abort() (point #2) |
@msporny - thanks for the updates. I suspect that an argument against the proposal from @zkoch and @adrianba will be that the state is a requirement. I would assert that this requirement comes from the assumption that other features are also required like capturing shipping address data. Since, that issue is still open I would suggest we re-word this proposal to the following to have a higher chance of resolving it on the call today: The PaymentRequest object SHOULD NOT expose internal state information to the developer. Any design that requires developers to manage or understand the request state is a compromise in the API design that should be avoided where possible. I would consider there to be enough evidence to support this proposal (XmlHttpRequest vs fetch). Resolving this would be taking a step toward establishing a set of design goals for our final deliverable, which I think is valuable, without making assumptions that I think are premature. It's your proposal so I'll leave it with you. |
@ianbjacobs has been soliciting feedback on both APIs from various people with appropriate experience and qualification within W3C and will be able to provide some useful input on the call. |
+1 to your revision, @adrianhopebailie, the proposal now uses your text. |
The counterargument to that is that you can gather data like shipping address in a declarative way - you don't need to track state and fire events to do so. I'm hesitant to add an example of how to do this to the Credentials CG specs because I don't think gathering a shipping address should be a part of the API, but if we do it declaratively, we can always switch to the "right way to do it" in the future w/o affecting how developers use the Web Payments Browser API. |
+1. I made an argument for a different approach here: https://github.com/WICG/paymentrequest/issues/42#issuecomment-173628517 |
Right, so there are at least two mechanisms that have been identified that would address a number of the events+state+imperative issues in the Payment Request API. Ultimately, I think @dlongley's proposal is a better generalized approach. |
The group didn't get to this proposal on the 28 January call. |
Are conflating the API with the messages that are passed to the API? My assumption, when we talk about a payment request object in the context of the browser API, is a JSON object that contains all the data specified by the payee. This object is passed as a parameter to the browser API when a website requests a payment. On the other hand there is a payment request API which accepts the payment request object as one of perhaps many parameters. There are 2 proposals on the table about the shape of the API itself:
Assuming the payment request object is defined as follows: var pr = {
"currency" : "EUR",
"amount" : "100"
}; A payment using 1. is made as follows: var pay = new PaymentRequest(pr, ...);
pay.show(); whereas a payment using 2 is made as follows: navigator.payments.requestPayment(pr, ...); So it would appear to me that this proposal should read: PROPOSAL: The browser API SHOULD NOT expose state information to the developer. Any design that requires developers to manage or understand the state of a payment request is a compromise in the API design that should be avoided where possible. @msporny Does that sound like a reasonable amend to your proposal? As an thought experiment, one could take this a step further and consider the UX implications of the API raising events and requiring the website to respond to these at all. When the website calls the API it is expecting to lose focus and for the user to begin interacting with a native browser UI, platform UI or a payment app UI (less likely - see #55). The website will only get focus again when it receives the payment response. Should we be defining a mechanism whereby the browser passes the execution flow control back to the website but not the user focus? I have heard an argument that this is all done asynchronously so it won't affect the UI but that assumes the event handled by the website and the subsequent response to the browser will not materially impact the UI. Otherwise we have a terrible user experience where either the user interface is regularly locked in a "waiting" state while the browser waits for the website to respond to the event or the UI is not locked but anything the user does after the event is fired may be lost if the response from the website renders their actions invalid. This proposal is an argument against doing the former and here's an example of the latter:
There are many places in that flow where the UX could be severely compromised by trying to make async requests to the website through events without returning the focus to the site or locking up the UI. That begs the question, shouldn't we be avoiding events AT ALL if we can? And if we consider events useful for their designed purpose - notifying the website of an event as opposed to making an sync request disguised as an event - we can include them then? An example of a legitimate event might be; notifying the website that an out-of-band request to the web server was made to get an updated payment request therefor the website may want to fetch the latest payment request from the server. So a more bold but architecturally sound proposal would be: PROPOSAL: The browser API SHOULD NOT raise events as a means of making asynchronous requests to the website. Events should only be used if there are actual events that it is valuable for the website to be aware of, and the only expected behavior of the website should be to ignore the event, process it or cancel the payment request. |
I'm not sure what this proposal means in practice. For the PaymentRequest API, does it just mean delete the line If so, @zkoch and I have discussed this in the past and I don't have strong feelings either way about exposing it in the API. It's low cost and might be useful but if it is seldom or never used then it isn't necessary. On the other hand, if this proposal is meant to say that the API shouldn't be based on a state machine, well that's a much broader topic and I don't believe it can be determined in isolation (and I disagree with that suggestion for the current feature set we've proposed). |
@adrianba it means "delete the line readonly attribute PaymentRequestState state; from the PaymentRequest interface WebIDL". It does not mean the API shouldn't be based on a state machine, clearly there is state here and you need to track it, but you should do it internally (if possible, and we're asserting it's possible). |
Okay. There's a difference between exposing the state and requiring a developer to use the state. A design that requires code that depends on the state value is one thing (and perhaps/probably would be a poorer design). But it doesn't follow that exposing the state does require depending on it. As I said, we've discussed this and I don't have a strong feeling about including it or not - in similar APIs we've found this type of attribute to be useful for debugging though that might not be the case here. |
Agreed with @adrianba here. State isn't required, either from an implementation perspective or from a web developer perspective. But it is very useful from an implementation perspective (try writing a polyfill without it). But like Adrian, I am not married to the idea. |
My concern is that by exposing internal state, Web developers will start depending on it and it's been made clear, at least in this case, that you can't depend on that variable (due to the aforementioned race conditions).
The specs are supposed to expose the public-facing bits, not "debugging info". We're all in agreement that the API implementation will most likely need to track state to some degree, polyfill implementations included. The proposal in this thread is attempting to say: "Yes, track state internally, but don't expose the internal implementation bits to Web developers because they'll likely think they can depend on it (which they can't due to race conditions)." An alternative is to stick the "state" bits in a "debug" variable, but I don't really like that approach either because I don't know of any other (successful) API that does that. |
@adrianhopebailie wrote:
Yes, I'd be a +1 for that. In an attempt to bring @zkoch and @adrianba 's concerns in, I'd also be a +1 for this: PROPOSAL: The browser API SHOULD NOT expose state information to the developer in a way that is not strictly for debugging purposes. Any design that requires developers to manage or understand the state of a payment request is a compromise in the API design that should be avoided where possible. |
@adrianhopebailie wrote:
This question is part of the basis for @dlongley's Checkout API proposal here: https://github.com/w3c/webpayments/wiki/Checkout-API |
@adrianhopebailie wrote:
While I'm supportive of this direction, it's a bit too broad-brush and thus makes me nervous. Not all events are bad, and we may find that an event may be our only option in certain scenarios. Rather that approve this proposal, I'd rather we focus on writing a mechanism up that doesn't depend on events (for the main flow) and see how that looks. I think @dlongley has put together such a proposal here: https://github.com/w3c/webpayments/wiki/Checkout-API |
I'm for removing the line |
Great, thanks @rsolomakhin - we can always add it back in if we find a good use for it. This addresses my concern w/ the Microsoft/Google proposal wrt. state. Happy to close the issue at this point. @adrianhopebailie do you want to close this now, or resolve the proposal on the call and record it as a RESOLUTION? |
Let's close this on the call per our process. Should be quick if everyone is already in agreement. |
This was not discussed on the call. |
@adrianhopebailie yes, happy to close this proposal as the Google/Microsoft spec has removed the state variable. |
Proposed by @msporny
Related to the discussion in #36 and #41
XMLHttpRequest is a problematic API partially due to the fact that it stores state (like the PaymentRequest API proposal) and managing that state machine has led to developer code being brittle (due to race conditions when the state machine transitions from one state to the next).
It is possible (and beneficial) to build a Web Payments Browser API that doesn't expose any state information to the developer, and thus eliminates one source of race conditions and complex code flow management: http://wicg.github.io/web-payments-browser-api/#processing-a-payment-request
PROPOSAL: The PaymentRequest object MUST NOT expose internal state information to the developer.
The text was updated successfully, but these errors were encountered: