- Notifications
You must be signed in to change notification settings - Fork202
refactor response create function outside of promise handler#132
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This will depend on the size impact - I've updated the repo to include a size check, we'll see what it nets. |
Ah - it looks like this adds a named export to the actual unfetch package, which means it won't work - folks using CommonJS will have to do Instead, if we could retrofit your work here into proper support for theResponse interface, that would definitely be a justifiable export! It would be a nice step towards supporting |
|
@kalisjoshua that'll be the added export. One quick solution to check this would be to move the |
not sure that this is favorable or not but thought that it might be and it allowsfor easier testing of the creation of responses and their properties.
1477655
toac63b91
CompareThis is done now.
I will look into theResponse interface compatibility. |
|
Interesting that the numbers are still so increased. My guess is that's because we're ending up with a closure around the two top-level functions, whereas currently there is no closure required since all functionality exists within the single |
It's interesting to look at the compiled output diff of |
I think the biggest contributor to the bloat that I created is because of the lifting of the |
673f183
to3986993
Compare
|
not sure that this is favorable or not but thought that it might be and it allows
for easier testing of the creation of responses and their properties.