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

docs: add custom useFetch recipe #27208

Merged
merged 7 commits into from
May 16, 2024
Merged

docs: add custom useFetch recipe #27208

merged 7 commits into from
May 16, 2024

Conversation

Atinux
Copy link
Member

@Atinux Atinux commented May 14, 2024

Migrate from https://notes.atinux.com/nuxt-custom-fetch to a Nuxt recipe as it has a lot of traffic and helps many people on this important feature.

Copy link

stackblitz bot commented May 14, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@Atinux Atinux changed the title docs(recipes): Add custom useFetch docs: Add custom useFetch May 14, 2024
@Atinux Atinux changed the title docs: Add custom useFetch docs: add custom useFetch recipe May 14, 2024
@MuhammadM1998
Copy link
Contributor

I'm using a similar setup with both $fetch and useFetch wrappers inside the same composable without plugins and it's working correctly. Is there a drawback of this that I'm missing? Maybe that would be good to clarified in the docs too

Copy link
Member

@DamianGlowala DamianGlowala left a comment

Choose a reason for hiding this comment

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

I would generally suggest replacing vast majority of occurences of 'we' (e.g. 'we created', 'we are storing') with a form more commonly used in the docs.

E.g. given the following sentence:

We are currently discussing with the core team (...)

... who are 'we' to the readers if it isn't the core team?

docs/2.guide/4.recipes/3.custom-usefetch.md Outdated Show resolved Hide resolved
docs/2.guide/4.recipes/3.custom-usefetch.md Outdated Show resolved Hide resolved
docs/2.guide/4.recipes/3.custom-usefetch.md Outdated Show resolved Hide resolved
docs/2.guide/4.recipes/3.custom-usefetch.md Outdated Show resolved Hide resolved
docs/2.guide/4.recipes/3.custom-usefetch.md Outdated Show resolved Hide resolved
@manniL
Copy link
Member

manniL commented May 14, 2024

Might be worth linking https://www.youtube.com/watch?v=jXH8Tr-exhI here too (happy to link back to the blog post instead of your personal one after this was published 👌)

Atinux and others added 4 commits May 15, 2024 09:23
Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

I've pushed a couple of changes, including to the example (for handling different possibilities of HeadersInit). It would be worth checking that everything still works as expected.

what do you think @Atinux?

@maximepvrt
Copy link
Contributor

@Atinux Will this duplicate https://nuxt.com/docs/examples/advanced/use-custom-fetch-composable ? Should this example be deleted ?

Copy link
Member Author

Atinux commented May 16, 2024

@maximepvrt I created a PR on nuxt/examples#75 to update the link

I think we could update the example based on Daniel changes though.

@danielroe it would be nice to have a utility for that because having 3 lines to do the same thing is quite frustrating. @pi0 is this something that ofetch could provide on onRequest hook?

Thinking of:

const api = $fetch.create({
  baseURL: 'https://api.nuxt.com',
  onRequest({ setHeader }) {
    if (session.value?.token) {
      setHeader('Authorization', `Bearer ${session.value?.token}`)
    }
  }
})

Copy link
Member Author

Atinux commented May 16, 2024

@DamianGlowala applied the changes.

@manniL linked your video :)

@pi0
Copy link
Member

pi0 commented May 16, 2024

Having standard way in ofetch context to set/get headers seems good idea. I'm thinking to adopt usage of web Headers (via a lazy getter). (Therefore using ctx.headers instead of destructuring args in example would be more performant suggestion)

@danielroe danielroe merged commit 06e1676 into main May 16, 2024
6 checks passed
@danielroe danielroe deleted the docs/custom-fetch branch May 16, 2024 11:43
@github-actions github-actions bot mentioned this pull request May 16, 2024
2 tasks
@manniL
Copy link
Member

manniL commented May 16, 2024

@Atinux Now that it has been merged, do you think it'd be helpful to link to the post in the docs in your blog post on the top of the article? 😋

@Atinux
Copy link
Member Author

Atinux commented May 24, 2024

@manniL I added a redirect instead :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants