resolve dom on import issue by moving quill components into their own package#26676
resolve dom on import issue by moving quill components into their own package#26676brrichards wants to merge 11 commits intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR splits Quill-specific exports out of @fluidframework/react's main entry point into a separate @fluidframework/react/quill sub-path export. This prevents consumers who don't use Quill from incurring a DOM dependency at import time, since Quill requires a DOM environment to load.
Changes:
- Added a new
packages/framework/react/src/text/quill.tsbarrel file that re-exports the Quill-related React components (FormattedMainView,PlainTextMainView,PlainQuillView, and associated types). - Removed the Quill-related exports from the main
src/index.ts, isolating them from the primary package entry point. - Added a
./quillexport map entry inpackage.jsonand updated thetext-editorexample to import Quill components from the new path.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
packages/framework/react/src/text/quill.ts |
New barrel file aggregating Quill-specific exports for the ./quill sub-path |
packages/framework/react/src/index.ts |
Removed Quill-related re-exports from the main entry point |
packages/framework/react/package.json |
Added ./quill sub-path to the exports map |
examples/data-objects/text-editor/src/app.tsx |
Updated imports to use @fluidframework/react/quill for Quill components |
|
Adding custom entrypoints like this is generally something we should avoid whenever possible IMO. Our standard entrypoints map to specific stability guarantees we make for our users, which we lose with a pattern like this. I'm not completely up-to-speed on this issue - what is the failure mode users are seeing? What other options have we explored to resolve this? |
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
Summary
Quill in the react package was polluting all users of that package with a need to have a dom on import time. Now the quill specific imports that caused this have been moved to the new package at
/packages/framework/quill-react.Moved
quillFormattedView.tsx,quillView.tsx, andmochaHooksto new package. LeftplainTextView.tsxin the react package as it did not contain any quill components. Separated the plain text tests from quill view tests when moving them over to the new package.Removed the inventory-app JSDOM fix needed when importing
@fluidframework/react/internalwhen it still had quill components.Created
.changesetwith reasoning for why the package was created.Breaking Change
Anyone importing quill components from
@fluidframework/react/internalwill now have to import from@fluidframework/react/quill-react. Since this is still internal and only used in text-editor, which has been updated with new path, this should be fine.