Allow multiple RequestEditorFns in Client and Request Methods#228
Allow multiple RequestEditorFns in Client and Request Methods#228deepmap-marcinr merged 4 commits intooapi-codegen:masterfrom
Conversation
|
This would be super useful. I just now have to complicate my code because I want to use different access tokens for some requests. With this PR I could just pass |
|
@deepmap-marcinr any chance we could get this in? It's a non-breaking change and it's been proven useful. |
| // A list of callbacks for modifying requests which are generated before sending over | ||
| // the network. | ||
| RequestEditor RequestEditorFn | ||
| RequestEditors []RequestEditorFn |
There was a problem hiding this comment.
This is a breaking change. Anyone accessing the previous RequestEditor will get a compile error now.
We don't need a list of RequestEditors on the client, could you leave it as a single RequestEditor?
Your per-handler request editors supercede this global per-client editor.
There was a problem hiding this comment.
We don't need a list of RequestEditors on the client, could you leave it as a single RequestEditor?
But how would then one register multiple RequestEditors with a client?
I mean, modifying RequestEditors directly should be a bad pattern anyway, no? You have to modify it through constructor options.
But if this is really the problem, we could have RequestEditor and RequestEditors, where first the RequestEditor is run an then new RequestEditors in order.
There was a problem hiding this comment.
I'm guessing the breakage from this will be minimal, I will merge as-is.
The reason that I originally had a single request editor is that you yourself can call a bunch from within it, so it seemed redundant.
Your code never uses that editor array on the client, as you use the list specified on the handler, and only of present, do you append. So, the change from 1 to many doesn't really gain much.
|
I've made one small comment in there to avoid breaking backward
compatibility, and I'll happily merge and cut a new release once you submit
this.
…On Thu, Jan 28, 2021 at 11:29 PM Pierre R ***@***.***> wrote:
@deepmap-marcinr <https://github.com/deepmap-marcinr> any chance we could
get this in? It's a non-breaking change and it's been proven useful.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#228 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALKC5DFLA2RYNP7CQXAPZDTS4JPXFANCNFSM4RAYGFHA>
.
|
…odegen#228) * Allow multiple RequestEditorFns and custom ones * Re-generate templates * Fix tests * Update README
4 participants
I usually find myself having to combine multiple editors into one, or change the client in order to customize a request with e.g. special headers.
This changes that with
RequestEditorFns in the Client, with eachWithRequestEditorFncall appending one to the list.RequestEditorFns as a (spread) parameter in every Client method. They are run after the default ones.This change is