fix(KNO-14012): inject guide toolbar styles at runtime#1043
fix(KNO-14012): inject guide toolbar styles at runtime#1043thomaswhyyou wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: b1a58af The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bundle ReportChanges will increase total bundle size by 43.74kB (7.62%) ⬆️
Affected Assets, Files, and Routes:view changes for bundle: @knocklabs/react-react-esmAssets Changed:
Files in
Files in
view changes for bundle: @knocklabs/react-react-cjsAssets Changed:
Files in
Files in
|
The guide toolbar rendered unstyled unless the consuming app imported @knocklabs/react/dist/index.css, which apps rendering only custom guide components have no reason to do. The toolbar now injects its compiled styles (telegraph tokens + components + toolbar keyframes, via a ?inline css import) into the document head when it becomes visible, so it works without an explicit CSS import. dist/index.css is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4924b4e to
b1a58af
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b1a58af. Configure here.
| @@ -0,0 +1,21 @@ | |||
| import React from "react"; | |||
|
|
|||
| import toolbarStyles from "./styles.css?inline"; | |||
There was a problem hiding this comment.
Inline CSS stripped by build
High Severity
useToolbarStyles imports toolbar CSS via styles.css?inline, but the React package build deletes all *.css.js / *.css.mjs artifacts and strips CSS requires. With vite-plugin-no-bundle (preserveModules), that inline module is emitted as one of those files, so the published package loses the style string and runtime injection cannot work.
Reviewed by Cursor Bugbot for commit b1a58af. Configure here.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1043 +/- ##
==========================================
+ Coverage 66.02% 66.58% +0.55%
==========================================
Files 213 214 +1
Lines 10500 10514 +14
Branches 1509 1529 +20
==========================================
+ Hits 6933 7001 +68
+ Misses 3542 3487 -55
- Partials 25 26 +1
|
1 participant
Description
Currently the guide Toolbar has an implicit dependency on
@knocklabs/react/dist/index.css, because the toolbar component uses Telegraph components and it requires the telegraph styles to be present in the document.If you happen to be not using any of the renderable components from
@knocklabs/react, then you have no need for dist/index.css in your project. With guides, for example, if you are only using your custom guide components and not any of the OOTB components, then you don't need import dist/index.css.One exception here though is the guide toolbar because the component needs certain telegraph and other styles. Currently we flag in the doc to import dist/index.css when using the toolbar, but it's a friction Neo4j have flagged.
This PR explores adding
useToolbarStyleshook that inlines the telegraph css + other styles needed for the toolbar, and injects into the page when the toolbar is used. This adds ~21KB min/~5KB gzip of inline css to the bundle (~2% of the overall size), and in return removing the dependency on@knocklabs/react/dist/index.css.TODO: Update the doc if/when we merge this.