Should PaymentMethodData be converted at construction time?

[marcoscaceres]

Related to #753

Given the following:

var details = {
  total: {
    label: "Total",
    amount: {
      currency: "USD",
      value: "0.00",
    },
  },
};
new PaymentRequest(
  [
    { supportedMethods: "basic-card", data: { supportedNetworks: "☠️" } },
  ],
  details
);

Firefox currently throws, because it tries to do the IDL conversion:

TypeError: Fail to convert methodData.data to BasicCardRequest.

Chrome doesn't throw.

Per spec, Chrome is right... but it not doing the conversion seems not great.

[marcoscaceres]

Checked Safari also (using Apple Pay as PMI), it doesn't throw.

@rsolomakhin, @aestes, @zouhir, what are your preferences?

[rsolomakhin]

I'm OK not throwing, but it would be good to print a warning in the developer console. We can put this guidance in the spec.

[zouhir]

I don't have strong opinion/preference on this, we don't throw today but I agree it's not great. Console warning sounds like a good suggestion.

[aestes]

If we convert at construction time and only console.warn(), does that mean we then won't throw at show() time because we've already converted? Demoting an exception to a console warning doesn't seem great to me.

Edit: I guess show() would have to throw something if the conversion failed, because the transaction couldn't proceed otherwise. But without extra effort that exception likely wouldn't be the same one that the browser's IDL conversion routine would've thrown, and might be less useful/informative.

[marcoscaceres]

If we convert at construction time and only console.warn(), does that mean we then won't throw at show() time because we've already converted? Demoting an exception to a console warning doesn't seem great to me.

I agree. Seems kinda pointless to go through the conversion steps, warn, then go through the conversion steps again in .show() to throw. Irrespective that Firefox already throws early, I think it's cleaner just to throw during construction time (and saves doing a conversion twice).

[aestes]

Converting at construction time would also allow for a more direct IDL conversion. Browsers could convert from a JS object directly to a IDL dictionary rather than the current JS object > JSON string > JS object > IDL dictionary conversion algorithm.

[marcoscaceres]

Converting at construction time would also allow for a more direct IDL conversion. Browsers could convert from a JS object directly to a IDL dictionary rather than the current JS object > JSON string > JS object > IDL dictionary conversion algorithm.

Yes, precisely :) That's ultimately where I'd like to get too. It gets rid of a whole bunch of complexity.

[rsolomakhin]

If show() will throw anyway, then might as well throw at construction time.

[marcoscaceres]

Ok, will move the conversion logic from show to the constructor. That makes much more sense.