-
Notifications
You must be signed in to change notification settings - Fork 324
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
Add abstractions for creating Request and Response objects #1157
Conversation
|
||
<li><p><a>Abort fetch</a> with <var>p</var>, <var>request</var>, and | ||
<var>responseObject</var>. | ||
<li><p><a>Abort fetch</a> with <var>p</var>, <var>request</var>, and <var>responseObject</var>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this works as at this point responseObject is null and later it might get a value at which point we'd want this to operate on that value I think. Is there a better way to tackle this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you check the signal right before creating the response object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only minor things, otherwise LGTM
For the eventual commit message:
|
@@ -6633,7 +6653,8 @@ method steps are: | |||
<li><p>If <var>response</var> is a <a>network error</a>, then <a for=/>reject</a> <var>p</var> | |||
with a {{TypeError}} and terminate these substeps. | |||
|
|||
<li><p>Associate <var>responseObject</var> with <var>response</var>. | |||
<li><p>Set <var>responseObject</var> to the result of <a for=Response>creating</a> a {{Response}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're worried about races between this and the "Abort fetch" line above, you could check the state locallyAborted
and abort these steps if it's true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's step 1 here so I think we're good, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Closes #771.
Preview | Diff