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

Allow overriding credentials via rest directive #214

Closed
stramel opened this issue Jun 3, 2019 · 3 comments
Closed

Allow overriding credentials via rest directive #214

stramel opened this issue Jun 3, 2019 · 3 comments

Comments

@stramel
Copy link

stramel commented Jun 3, 2019

It would be great to be able to specify an override for credentials via the @rest directive. The majority of my requests need same-origin but I have a handful that need omit specified. Currently, I can't figure out a way to handle this use-case.

@fbartho
Copy link
Collaborator

fbartho commented Jun 3, 2019

I would write a customFetch implementation then. (It's a thin wrapper around the node-fetch instance).

Alternatively, you can use a custom context link to mess with the headers before they get sent.

@stramel
Copy link
Author

stramel commented Jun 3, 2019

@fbartho Would you be willing to accept a PR for this as a feature/enhancement?

@fbartho
Copy link
Collaborator

fbartho commented Jun 14, 2019

@stramel I keep starting a response to your request, and losing track of the browser tab before submitting.

I wanted to think about it carefully before responding with a reflex gut reaction.

Right now, I feel like the @rest( directive already has too many "optional" arguments with defaults. (Each additional argument is a significant amount of code & testing & documentation & examples -- and each one makes things harder for newbies to know how to use this library). When you asked me about this issue, that's was my immediate thought. Then today, this ticket got opened: #215 and that cemented my feeling.

I really don't think that we should do this as a new optional @rest() parameter. Instead if you implement the customFetch. It would be straight forward for you to create your own fetch with a registry of special overrides:

// Somewhere:
const credentialsRegistryByPathPrefix = {
  "https://example.com/must-omit": { omit: true }
}

// Where you configure RestLink:
function myFetch(path, options) {
  for(const p in credentialsRegistryByPathPrefix) {
    if (path.startsWith(p)) {
      options.credentials = { ...credentialsRegistryByPathPrefix[p], ...(options.credentials || {} ) };
    }
  }
  return fetch(path, options);
}

const restLink = new RestLink({ customFetch: myFetch });

Obviously this is pseudo-code that I just wrote in my browser, but I think it would work.

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

No branches or pull requests

2 participants