fix(devtools): add shadowDOMTarget property to react-query-devtool #7005
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
|
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 0b4e792:
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 0b4e792. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution
✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
github-actions
bot
added
documentation
|
@TkDodo hey there 👋🏾 |
michelleoj
changed the title
allow devtool to attach styles to the shadow DOM instead of the default head tag of the light DOM
|
@ardeora any chance I can get some eyes on this soon please? |
|
Apologies for the delay here. I'll be taking a look at this in an hour or two! |
| const lightStyles = (css) => stylesFactory('light', css) | ||
| const darkStyles = (css) => stylesFactory('dark', css) |
There was a problem hiding this comment.
| const lightStyles = (css) => stylesFactory('light', css) | |
| const darkStyles = (css) => stylesFactory('dark', css) | |
| const lightStyles = (css: typeof goober['css']) => stylesFactory('light', css) | |
| const darkStyles = (css: (typeof goober)['css']) => stylesFactory('dark', css) |
Types are erroring could you fix the types like so
|
Also could you run pnpm test before committing the changes? You would also notice some formatting errors. If you run pnpm prettier:write it should fix those |
|
@ardeora added your suggestions and now all tests are passing ✅ |
There was a problem hiding this comment.
These are looking perfect! Thank you for working on this. One other small addition (last one I promise) would be if we can make this feature available in other framework devtools also. You don't need to know Solid, Svelte or Angular. We already have optional props like styleNonce in other framework devtools adapter. We can follow the same convention to add an optional shadowDomTarget prop to all other devtools too. Once that is done, I think we are good to merge.
|
@ardeora just added your suggestions and it's ready for another review! 🙏🏾 |
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #7005 +/- ##
==========================================
- Coverage 41.29% 41.03% -0.26%
==========================================
Files 180 182 +2
Lines 7193 7220 +27
Branches 1454 1490 +36
==========================================
- Hits 2970 2963 -7
- Misses 3836 3859 +23
- Partials 387 398 +11 |
|
@michelleoj Thank you so much for working on this! Everything looks great and should be included in the next minor version! |
|
@ardeora thanks so much for taking the time to review my PR! this is my first time contributing to an open source project so thanks for helping me through it 🎉 ❤️ |
This feature stems from this conversation with @ardeora
The original issue was that if you rendered the devtools in an component that lives in the shadow dom, the styles would be appended to regular DOM's head tag. Since the shadow DOM encapsulates everything, the devtools shows up unstyled. The fix to this is to pass in the shadow root as a target so that goober library will append the styles to the ShadowRoot element.
I've gone ahead and added an example and updated the docs. Let me know if there's anything else I'm missing