Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(useFetch): new function #330
feat(useFetch): new function #330
Changes from 6 commits
47dec5c
0ffbe6c
38a9a94
f41c653
954a037
9680574
83b82ef
8f9d850
fc387cb
b56d392
d692c28
0d96f10
b9a8e2d
237724c
374ed94
cc8edad
34e8fed
345d5de
cf21b90
8530f98
ae739be
2c5092a
acb7bf0
9e07337
5ad7f98
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 we make this usage simpler with some API?
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.
Hmm, is there something out there that would allow us to simplify it? I'm not aware of anything, but if you know I'll update it
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.
Not really. But maybe keep your implementation and make a
useFetchJson
wrapper with preconfigured header? Moreover, something likeusePost
andusePostJson
?Another approach might be similar to
ky
WDYT?
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.
Hmm doing more research into the fetch API they actually have more than just
text()
andjson()
, https://developer.mozilla.org/en-US/docs/Web/API/Body#methodsIf we were to make a wrapper for all of these we would have
useFetch
,useFechJson
,useFetchBlob
,useFetchFormData
, anduseFetchArrayBuffer
. But I feel like most of these would not be used very often and it would create quite a few methods. Perhaps another option would be to have an option in the config to set how the data is parsed. Something like this perhaps?I do like the idea of wrappers for
usePost
andusePostJson
. If we do that would we also want ones for form-data and multipart form data?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.
Sorry for the formatting
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.
No problem, I can fix it in a sec
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 doesn't actually seem to work. Can you watch an
unref
? I was under the impression it just gave you back the raw value if it was a refThere 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.
Did you test it? It's not watching an
unref
but the "accessing".unref
is just a unified way to accessing here, the return value doesn't matter. Should work I think.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.
Ok it looks like it does work, but the watch needs to have
deep: true
setThere 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.
Em? I don't think it's necessary... 🤔
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.
Weird, it only seems to work when I set
deep: true
. Not really sure whyThere 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 just created a repo to demonstrate the issue. It is a bit of a contrived example, but it should show the issue. It is in vite so you should be able to just clone it and run it https://github.com/jacobclevenger/vue3-watch-bug/blob/master/src/App.vue