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

editorial: add example of .toJSON()+fetch POST #583

Merged
merged 3 commits into from Aug 16, 2017

Conversation

marcoscaceres
Copy link
Member

@marcoscaceres marcoscaceres commented Aug 15, 2017

Copy link
Collaborator

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM with potential improvements

index.html Outdated
try {
const httpResponse = await fetch("/process-payment", {
method: "POST",
headers: new Headers({ "Content-Type": "application/json" }),
Copy link
Collaborator

Choose a reason for hiding this comment

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

new Headers is not necessary; you can pass the object literal directly.

});
result = httpResponse.ok ? "success" : "fail";
} catch (err) {
console.error(err);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this set result = "fail"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably... but wanted to show the "unknown" state somehow.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe do a quick parse of response.json() ? I dunno. I have a hard time understanding "unknown" in real-world systems...

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, me too :( Ok, will just add a comment about "unknown" that it's an option that people can use if they need it... however, we should try to come up with something real or we should mark it "at risk".

Chrome currently does nothing with "unknown" (behaves the same as "success", in that no error is shown to the user). I don't know if we (Firefox) will do anything with "unknown" at this point.

Copy link
Member Author

Choose a reason for hiding this comment

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

Filed #583

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed mention of "unknown", as I have a feeling it's it won't survive CR.

@marcoscaceres marcoscaceres merged commit 88d2836 into gh-pages Aug 16, 2017
@marcoscaceres marcoscaceres deleted the toJSON_example2 branch August 16, 2017 04:22
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

Successfully merging this pull request may close these issues.

Need short example of serializing response
2 participants