Accept custom context in ThemeProvider #1650
Conversation
💥 No ChangesetLatest commit: e8ecc3e Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂 If these changes should be published to npm, you need to add a changeset. Click here to learn what changesets are, and how to add one. Click here if you're a maintainer who wants to add a changeset to this PR |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit e8ecc3e:
|
|
@mitchellhamilton could u take a look at this one? |
|
I'm not really big on this given how easy it is to implement yourself and it makes explaining the API harder(and I imagine it would be harder to type correctly) |
|
My main motivation for this is to make it easier to use custom themes, the existing merging behavior of nested providers is good & quite intuitive - I won't insist at getting this merged right now as it can always be added later but is something that I thought was worth presenting as a PR. While this is easy to implement if one wants to have the same logic as builtin ThemeProvider for a custom theme they need to replicate it closely, won't even be able to copy-paste the builtin implementation as it requires subtle tweaks to support a custom theme. Explaining this would be a separate section in the docs so I don't feel like this would make it particularly harder to understand and as to the types - I think it's possible to support this with TS conditional types. Feel free to close this PR or comment back to get this a green light, both work for me. |
|
@mitchellhamilton should I close this? |
It's a followup to #1577 - it probably indeed didn't make much sense, but I believe there is some benefit in accepting a custom context prop in
ThemeProviderIf we are going to decide to merge this in then I'd like to wait for #1609 to land first, so I can add final TS types in here - based on the work from that other PR.