-
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?
| import "../src/styles/fonts.pcss"; | ||
| import "../src/styles/index.pcss"; | ||
| import "../src/styles/themes/index.pcss"; |
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.
do we need to update the README as well?
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 |
@codexteam/ui/package.json
Outdated
| @@ -1,7 +1,11 @@ | |||
| { | |||
| "name": "@codexteam/ui", | |||
| "version": "0.1.1", | |||
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.
| "version": "0.1.1", | |
| "version": "0.2.0", |
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.
| </script> | ||
|
|
||
| <style lang="postcss"> | ||
| <style module lang="postcss"> |
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.
There is an extra space in the style tag opening. It should be <style module lang="postcss"> instead of <style module lang="postcss"> (note the double space).
| <style module lang="postcss"> | |
| <style module lang="postcss"> |
| @@ -1,5 +1,3 @@ | |||
| @import url('../fonts/Inter/inter.css'); | |||
| @import url('../fonts/JetBrainsMono/JetBrainsMono.css'); | |||
|
|
|||
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.
Font imports have been removed from typography.pcss and moved to fonts.pcss, but fonts.pcss is not being imported in the base styles (index.pcss). This means that when consumers import '@codexteam/ui/styles', they will not get the required font files. Either add fonts.pcss to the vite.config.ts rollup input as a separate entry point, or re-add the font imports back to typography.pcss.
| ### 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.
Tree-shaking Support for @codexteam/ui
Description
Implementation package
Example
main.ts
app.vue
Compare the results with and without tree-shaking