-
Notifications
You must be signed in to change notification settings - Fork 109
Add support for requestOptions as a method to Poll #113
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! Thanks for you contribution, just a little edgecase and we are good :)
src/Poll.tsx
Outdated
{...contextProps} | ||
{...props} | ||
requestOptions={{ | ||
...contextRequestOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost perfect! But you need to use merge
from lodash here, so we can add a header without erase these set in the Provider
Example:
https://github.com/contiamo/restful-react/blob/master/src/useGet.tsx#L87
and the associate test:
restful-react/src/useGet.test.tsx
Lines 512 to 533 in eaa37d2
it("should merge headers with providers", async () => { | |
nock("https://my-awesome-api.fake", { reqheaders: { foo: "bar", bar: "foo" } }) | |
.get("/") | |
.reply(200, { oh: "my god 😍" }); | |
const MyAwesomeComponent = () => { | |
const { data, loading } = useGet<{ oh: string }>({ path: "/", requestOptions: { headers: { foo: "bar" } } }); | |
return loading ? <div data-testid="loading">Loading…</div> : <div data-testid="data">{data.oh}</div>; | |
}; | |
const { getByTestId } = render( | |
<RestfulProvider base="https://my-awesome-api.fake" requestOptions={() => ({ headers: { bar: "foo" } })}> | |
<MyAwesomeComponent /> | |
</RestfulProvider>, | |
); | |
await waitForElement(() => getByTestId("data")); | |
expect(getByTestId("data")).toHaveTextContent("my god 😍"); | |
}); | |
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, good catch, thanks!
Fixed and added a test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to me! Thank again for this beautiful PR 👌
@vxsx, you're a legend. Thanks for your contribution! 🎉 |
Why
Fixes #112
I'm not sure this is a proper way, but it works. Get / Poll seem to be quite different and sadly I didn't have much time to dig into the source code.