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

feat: have both local and global interceptors #357

Closed
wants to merge 1 commit into from

Conversation

enkot
Copy link

@enkot enkot commented Feb 2, 2024

🔗 Linked issue

Closes

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Currently interceptors specified in $fetch() override the interceptors in $fetch.create.
This PR implements:

  1. The interceptor "merge strategy" options that allow you to have both default and per-request interceptors. You can add interceptor as an object with handler and strategy fields when creating new client using $fetch.create. Available strategies:
  • 'overwrite' - current behaviour
  • 'after' - interceptor in $fetch will be called after the interceptor in $fetch.create
  • 'before' - interceptor in $fetch will be called before the interceptor in $fetch.create
  • 'manual' - interceptor in $fetch.create decides how/when/if call interceptor from $fetch

In all cases interceptor has second parameter which can be:

  • response data
  • data return by the previous interceptor

Names are subject to change 👀

  1. It is now allowed to return new response data from the onResponse, onResponseError interceptors.
    If the function doesn't return anything or returns undefined - the response data will not be overwritten.

Example:

const myFetch = $fetch.create({
  onResponse: {
    strategy: "after",
    handler(ctx, data) {
      return 'global: ' + data
    },
  }
})

const r = await myFetch<string>("https://httpstat.us/200", {
  onResponse(ctx, data) {
    return 'local -> ' + data
  }
})

console.log(r) // will print "local -> global: 200 OK"

The open question is how to type the "data" parameter, currently it has type any.

We can also introduce the defaultStrategy option.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@enkot enkot closed this Mar 14, 2024
@enkot enkot mentioned this pull request Mar 14, 2024
8 tasks
@enkot enkot reopened this Mar 28, 2024
@pi0
Copy link
Member

pi0 commented Aug 28, 2024

Thank you for PR dear @enkot while some changes are nice, i found it little over complex and some against the design patterns of ofetch.

For the initial step, I went with #353 which allows an array of interceptor options.

A quick explanation of other parts:

  • ofetch does not allow any kind of global behavior change (this is by-design). we might (rightfully) need global hooks that can log requests but they shouldn't be able to block or change global fetch behavior so they are not exactly hooks.
  • with the array format, the order of hooks is already explicit
  • we can support returning custom Responses by all hooks, as a followup of feat: support interceptors arrays #353 in callHook internal util.

@pi0 pi0 closed this Aug 28, 2024
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.

2 participants