- 
                Notifications
    
You must be signed in to change notification settings  - Fork 4
 
WIP effection retry blogpost #374
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
base: production
Are you sure you want to change the base?
WIP effection retry blogpost #374
Conversation
          👷 Deploy request for frontside pending review.Visit the deploys page to approve it 
  | 
    
e7d2cf2    to
    e4ea906      
    Compare
  
    | const response = yield* call(fetch("https://foo.bar"), { signal }); | ||
| 
               | 
          ||
| if (response.ok) { | ||
| return yield* call(response.json()); | 
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.
Let's use call(() => response.jon()) here instead of call(response.json). We're probably going to change this upstream, so just to make it forward compatible.
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.
Done
| } | ||
| } | ||
| 
               | 
          ||
| main(function* () { | 
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.
Let's use run in the example here because main is only for scripts.
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.
This one's done too
| let attempt = -1; | ||
| while (true) { | ||
| const signal = yield* useAbortSignal(); | ||
| const response = yield* call(fetch("https://foo.bar"), { signal }); | 
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.
We should take take argument from fetchWithBackoff.
function* fetchWithBackoff(url: URL | string, init?: RequestInit) {
    const signal = yield* useAbortSignal();
    const response = yield* call(() => fetch(url, { ...init, signal }));
}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 added the arguments for all the examples except the last one. If we're surfacing the function to the top, those don't need to be an argument, right?
| 
           Looks good. Should we start working on the text now?  | 
    
          
 👌  | 
    
d8de1f5    to
    2463ade      
    Compare
  
    | 
           @taras I might need some help with the intro and outro. And let me know what you think of the draft.  | 
    
WIP
Motivation
Approach
Alternate Designs
Possible Drawbacks or Risks
TODOs and Open Questions
Learning
Screenshots