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

fix(useFetch)!: allow setting response type before doing request #454

Merged
merged 4 commits into from May 11, 2021

Conversation

isgj
Copy link
Contributor

@isgj isgj commented Apr 15, 2021

fixes #442

Now there is only one interface UseFetchReturn that is returned, when you call useFetch the first time, set the method or update the type. Of course you cannot change the method or the type while it's making a request.

In addition this fixes (unless it was intended) the loading state when the request is cancelled in beforeFetch .

I didn't touch the tests nor the docs. before let me know what you think about this approach.

@antfu
Copy link
Member

antfu commented Apr 19, 2021

I still not quite understand the context here. useFetch is designed to be used for one response type at once. Which gives better code organization and better typescript support. For requests that need to have difference types on each call, I'd think it's better to use two separate useFetch to do that

@isgj
Copy link
Contributor Author

isgj commented Apr 20, 2021

useFetch is designed to be used for one response type at once.

This is still true, you can't change the type nor the method (and data) while making a call.

More than different types, this is about different data as explained in the linked issue. I can update this PR so for the functions that set the type the returned interface is base + method functions (but it wont change much).

@@ -361,9 +362,9 @@ export function useFetch<T>(url: MaybeRef<string>, ...args: any[]): UseFetchRetu

function setType(type: DataType) {
return () => {
if (!initialized) {
if (!isFetching.value) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this should still be !initalized, right? Changing the payload seems fine to me, but we don't want to change the type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this PR initialized is not used. It was set to true the first time execute was called and stayed so.
Now in master now you can do

const {post} = useFetch(..., {immediate: false})
// somewhere else
post(...).json().execute() // of course only once

If you'll allow more calls json will return undefined and you cannot chain execute and ... (you got the idea).

In the last commit I changed the types so you can do

const {post} = useFetch(..., {immediate: false}).json()
// somewhere else
post(...).execute() // even more times

To note

This is a breaking change as now the order you chain the method is strict post -> json (method --> type), here instead is the inverse json --> (post X times).
If the order shouldn't be strict then those functions should return the same interface, otherwise keeping track what was called and should be returned would be unnecessary

@antfu
Copy link
Member

antfu commented Apr 21, 2021

I see. That's a good point. Since it's a breaking change, I'd more like to have it in the v5 release #363. Will handle it soon

@schelmo
Copy link
Contributor

schelmo commented May 3, 2021

👍
the next cool thing would be refetching when the the payload is changing!
like

const payload = ref({some: 'data'})
useFetch('/', {refetch: true}).post(payload)
payload.value.more = 'moar' // -> refetches

maybe even watch the whole config object? (so, changes to type, method, payload, payloadType will trigger a refetch)

@wheatjs
Copy link
Member

wheatjs commented May 3, 2021

👍
the next cool thing would be refetching when the the payload is changing!
like

const payload = ref({some: 'data'})
useFetch('/', {refetch: true}).post(payload)
payload.value.more = 'moar' // -> refetches

maybe even watch the whole config object? (so, changes to type, method, payload, payloadType will trigger a refetch)

Hmm probably would just want to watch the payload for refetching. I don't think we really want to be changing methods

@isgj
Copy link
Contributor Author

isgj commented May 4, 2021

Hmm probably would just want to watch the payload for refetching.

I agree. And resending some data with POST (or PUT, ...) you're not refetching but changing something on the server.

@antfu antfu changed the base branch from main to next May 11, 2021 18:24
@antfu antfu changed the title Update useFetch to allow more calls fix(useFetch)!: allow setting response type before doing request May 11, 2021
@antfu antfu merged commit 2d6e330 into vueuse:next May 11, 2021
This was referenced May 25, 2021
@isgj isgj deleted the more-fetch branch June 5, 2021 10:06
antfu added a commit that referenced this pull request Jun 6, 2021
* feat(useTransition): support for vectors (#376)

* refactor(useTransition): cleaning up (#385)

* refactor(useWebWorkerFn): Small doc and type improvements (#382)

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>

* feat: pwa reload prompt

* chore: update docs

* refactor(useWebWorkerFn): Small doc and type improvements (#382)

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>

* chore: update docs

* test: simpilfy tests for useTransition

* chore: fix tests

* feat(useTransition): support for delayed transitions (#386)

* feat(useTransition): support for disabled transitions (#436)

* feat!: introduce `controls` option

* chore: update

* chore: update

* refactor(useRafFn): remove depreacted APIs

* chore: enabled tests for next branch

* fix(useFetch)!: allow setting response type before doing request (#454)

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>

* chore: resolve conflicts

* feat(useMediaControls): expose source types (#495)

* fix(useMediaControls): Removes tracks that have been inserted in html (#493)

* chore: release v4.9.3

* fix(usePermission): tolerate error on FireFox

* fix(useDevicesList): treat as premssion granted after getUserMedia

* chore: release v4.9.4

* chore: fix typo (#502)

* feat(useWebSocket): add immediate option (#503)

* feat(useAxios): bring API into line with useFetch (#499)

* feat(createEventHook): new function (#497)

* chore: release v4.10.0

* fix(useMediaControls): Doesn't rewrite default media properties (#500)

* feat(useMediaControls): add error event (#509)

* feat(useStorage): optimize event handling logic (#505)

* feat(useFetch): add afterFetch option, onFetchResponse, and onFetchError (#506)

* feat(useWebWoker): return worker (#507)

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>

* fix: Change `onMediaError` to `onSourceError` (#510)

* feat(onClickOutside): default to just pointerDown (#508)

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
Co-authored-by: sibbng <sibbngheid@gmail.com>

* chore: update docs

* chore: release v4.11.0

* fix(onClickOutside): duplicate code (#519)

Co-authored-by: Nurettin Kaya <sibbngheid@gmail.com>

* feat(createEventHook): added interface (#531)

* feat(createEventHook): added interface

* added types for EventHookOn, EventHookOff, and EventHook trigger

* feat(useStorage): allow custom serializer (#528)

* feat(useStorage): allow custom serializer

* update test

* refactor(useMediaControls): Deprecate options that can simply be set as attributes (#514)

* useMediaControls: Add `volumechange` event listener

* fix: `mute` returned

* feat: Deprecate video options:
* `poster`
* `autoplay`
* `preload`
* `loop`
* `controls`
* `playsinline`
* `autoPictureInPicture`

* fix: Fix deprecated behaviour in demo

* fix: Remove deprecated usage from doc

* refactor: More polite messages

* fix: Remove `console.warn`s

* chore: release v4.11.1

* refactor!: remove deprecated apis

* chore: add next tag

* chore: release v5.0.0-beta.1

* feat: introduce `components` & `directives` (#486)

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>

* docs: re-organize

* chore: fix lint

* docs: about components

* chore: include directives

* chore: release v5.0.0-beta.2

* chore: rollback jest

* chore: fix docs build

* docs: readme for components

* docs: add @vueuse/gesture

* chore: ship indexes.json

* chore: release v5.0.0-beta.3

* feat(typedef): add return typedefs (#543) (#544)

* refactor!: change publish strcture and support submodules, close #469

* chore: cleanup stories.tsx

* docs: update docs about submodules

* chore: fix docs

* chore: release v5.0.0-beta.4

* chore: update lock

* chore: release v5.0.0-beta.5

* chore: update deps and extend publish memory

* refactor: remove `useDeviceLight`

* chore: update

* chore: fix tests

* chore: release v5.0.0-beta.7

* refactor(useWebSocket)!: change immediate default for 5.0.0 (#545)

* feat(useIpcRenderer): new add-one & new functions (#547)

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>

* chore: update deps

* chore: release v5.0.0-beta.8

* chore: fix docs build

* chore(usePointerSwipe): fix typo (#557)

* fix(useAuth): now reqiures the auth instance, close #538

* chore: update deps

* docs(biSyncRef): fix console output comment (#555)

* docs: removed deprecated value from example (#556)

* docs(guidlines): added guidelines (#535)

* docs: update guidelines

* chore: update guidelines

Co-authored-by: Scott Bedard <scottbedard@users.noreply.github.com>
Co-authored-by: Fabian <donskelle@googlemail.com>
Co-authored-by: Ismail Gjevori <isgjevori@protonmail.com>
Co-authored-by: Alex Kozack <cawa-93@users.noreply.github.com>
Co-authored-by: Shinigami <chrissi92@hotmail.de>
Co-authored-by: wheat <jacobrclevenger@gmail.com>
Co-authored-by: sibbng <sibbngheid@gmail.com>
Co-authored-by: JserWang <jserwang@gmail.com>
Co-authored-by: Pig Fang <g-plane@hotmail.com>
Co-authored-by: ArcherGu <34826812+ArcherGu@users.noreply.github.com>
Co-authored-by: Ilya Komichev <hello@ilko.me>
Co-authored-by: Daiki Ojima <daiking.ca2@gmail.com>
Co-authored-by: Manaus <manaustransez@hotmail.com>
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.

useFetch cannot post with different data
4 participants