-
Notifications
You must be signed in to change notification settings - Fork 6
feat: implement tree-shaking #337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
neSpecc
left a comment
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.
How tree-shaking works for components?
For Components
When user imports: import { Button, Avatar } from '@codexteam/ui/vue';Bundler analyzes and includes ONLY:
All other components (Editor, Input, Chart, etc.) are completely eliminated from the final bundle. For ThemesSimilar approach - each theme is a separate CSS file: import '@codexteam/ui/styles/themes/pure'; // Only pure theme included
import '@codexteam/ui/styles/themes/grass'; // Only grass |
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.
Pull request overview
This pull request implements tree-shaking support for the @codexteam/ui package, allowing consumers to import only the components and themes they need to reduce bundle size. The implementation restructures the build configuration to split CSS files and preserve module structure.
Changes:
- Enabled CSS code splitting and module preservation in Vite build configuration
- Separated theme CSS files into individual importable modules
- Converted several Vue components to use CSS modules (ThemePreview, Tabbar, Editor, Counter, Chart, ChartLine)
- Updated package.json exports to expose individual theme files and TypeScript types
- Moved font imports to separate fonts.pcss file
- Updated documentation with tree-shaking usage examples
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| @codexteam/ui/vite.config.ts | Enabled CSS code splitting, added individual theme entries, configured module preservation for tree-shaking |
| @codexteam/ui/tsconfig.dts.json | Updated TypeScript declaration output directory structure |
| @codexteam/ui/package.json | Added version bump, sideEffects configuration, individual theme exports, and TypeScript types path |
| @codexteam/ui/src/styles/index.pcss | Removed themes import to enable individual theme imports |
| @codexteam/ui/src/styles/typography.pcss | Removed font imports (moved to separate file) |
| @codexteam/ui/src/styles/fonts.pcss | Created new file with font imports for separate loading |
| @codexteam/ui/src/vue/components/*/**.vue | Converted multiple components to use CSS modules for better encapsulation |
| @codexteam/ui/README.md | Updated documentation with tree-shaking examples and usage instructions |
| @codexteam/ui/.gitignore | Added comprehensive gitignore patterns |
| @codexteam/ui/.npmignore | Added test-app to ignored files |
| @codexteam/ui/dev/index.html | Updated dev environment to import fonts and themes separately |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </script> | ||
|
|
||
| <style> | ||
| <style module> |
Copilot
AI
Jan 15, 2026
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.
The style tag has been changed to use module, but this component still uses hardcoded class names in the template (lines 4-7) instead of $style. When using CSS modules, all class references in the template must use $style or the styles should remain global by removing the module attribute. The classes 'editorjs' and 'editorjs--with-initial-placeholder' will not be properly scoped with this configuration.
@codexteam/ui/src/vue/components/theme-preview/ThemePreview.vue
Outdated
Show resolved
Hide resolved
| ### 1. Import Base Styles (Required) | ||
|
|
||
| ```typescript | ||
| // main.ts | ||
| import '@codexteam/ui/styles'; | ||
| ``` |
Copilot
AI
Jan 15, 2026
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.
The documentation doesn't mention that users need to handle font imports separately. Since fonts were moved to a separate fonts.pcss file that is not included in the base styles, users will not have access to the Inter and JetBrains Mono fonts unless they explicitly import them. The README should be updated to document how fonts should be imported, or fonts should be included in the base styles export.
| preserveModules: true, | ||
| preserveModulesRoot: 'src', |
Copilot
AI
Jan 15, 2026
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.
With preserveModules: true and preserveModulesRoot: 'src', the output structure will mirror the source structure. This means the output will be in dist/vue/index.js, dist/vue/components/button/Button.vue.js, etc. However, the package.json exports point to ./dist/vue.js. This mismatch will likely cause import errors. The rollup input should either not include the vue entry if using preserveModules, or the package.json exports path needs to be updated to match the actual output location.
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.
preserveModules: true preserves the module structure for imported modules, not for entry points.
In our configuration:
-
The entry point vue: resolve(__dirname, 'src/vue/index.ts') outputs to dist/vue.js based on the key in the input object
-
preserveModules only affects modules imported from entry points, preserving their source structure (e.g., dist/vue/components/...)
Entry points are always bundled and named according to their keys in rollupOptions.input, regardless of preserveModules. Therefore, dist/vue.js is the correct output,
1259e00 to
3871411
Compare
3871411 to
d74382a
Compare
| formats: ['es'], | ||
| }, | ||
| cssCodeSplit: false, | ||
| cssCodeSplit: true, |
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.
please add comment here
|
|
||
| ```typescript | ||
| // main.ts | ||
| import '@codexteam/ui/styles'; |
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.
You made fonts optional, in this case we need to add a note to readme that fonts are needed to be imported separately.
Tree-shaking Support for @codexteam/ui
Description
Implementation package
Example
main.ts
app.vue
Compare the results with and without tree-shaking