feat: external most dependencies of core package#1236
feat: external most dependencies of core package#1236chilingling wants to merge 5 commits intoopentiny:developfrom
Conversation
WalkthroughThe Vite build configurations across multiple packages were updated to expand the list of external dependencies in their Rollup options. These changes introduce additional explicit package names and regex patterns, ensuring a broader set of modules are treated as external and excluded from the bundled output, affecting how dependencies are resolved during the build process. Changes
Sequence Diagram(s)sequenceDiagram
participant ViteConfig
participant Rollup
participant ExternalModule
participant Bundle
ViteConfig->>Rollup: Provide external dependencies list
Rollup->>ExternalModule: Check if module matches external pattern
alt Module is external
ExternalModule-->>Bundle: Exclude from bundle
else Module is not external
ExternalModule-->>Bundle: Include in bundle
end
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
chilingling
force-pushed
the
feat/externalAllDepsFromCore
branch
from
3624474 to
da90453
Compare
chilingling
force-pushed
the
feat/externalAllDepsFromCore
branch
from
da90453 to
d986aa0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/layout/vite.config.ts (1)
35-35: Consider consistent externalization pattern across packagesWhile this change is good, I notice that different packages are externalizing different dependencies. Consider standardizing a common set of core dependencies that should be externalized across all packages to ensure consistency.
For example, if '@vueuse/core' is being externalized here, should it also be externalized in other packages where it might be used?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
packages/canvas/init-canvas/init-canvas.ts(1 hunks)packages/canvas/scripts/vite-plugin-separate-build.ts(2 hunks)packages/canvas/vite.config.ts(1 hunks)packages/common/vite.config.ts(1 hunks)packages/design-core/vite.config.js(1 hunks)packages/layout/vite.config.ts(1 hunks)packages/plugins/block/vite.config.ts(1 hunks)packages/plugins/datasource/vite.config.js(1 hunks)packages/plugins/i18n/vite.config.ts(1 hunks)packages/plugins/schema/vite.config.ts(1 hunks)packages/plugins/tutorial/vite.config.ts(1 hunks)packages/register/vite.config.ts(1 hunks)packages/settings/props/vite.config.ts(1 hunks)packages/settings/styles/vite.config.ts(1 hunks)packages/toolbars/lang/vite.config.ts(1 hunks)packages/toolbars/view-setting/vite.config.ts(1 hunks)packages/utils/vite.config.ts(1 hunks)packages/vue-generator/vite.config.js(1 hunks)packages/webcomponent/vite.config.js(1 hunks)
🔇 Additional comments (20)
packages/settings/props/vite.config.ts (1)
35-35: Good externalization of @vueuse/core dependency.Adding
'@vueuse/core'to the externals list is consistent with the PR's objective to externalize all core package dependencies. This ensures the package won't bundle @vueuse/core, reducing the final bundle size and preventing duplicate code when multiple packages use the same dependency.packages/plugins/schema/vite.config.ts (1)
35-35: Good externalization of @vueuse/core dependency.Adding
'@vueuse/core'to the externals list for this package maintains consistency with the other package configurations and aligns with the PR's goal of externalizing all core dependencies.packages/toolbars/view-setting/vite.config.ts (1)
32-32: Good externalization of @vueuse/core dependency.Adding
'@vueuse/core'to the externals list is consistent with the other packages and properly supports the PR's objective to externalize all core package dependencies.packages/utils/vite.config.ts (1)
28-28: Good addition of jszip as external dependencyAdding 'jszip' to the external dependencies list is consistent with the PR objective to externalize all core package dependencies. This ensures that jszip isn't bundled with the utils package, which should lead to smaller bundle sizes.
This change is part of a breaking change as noted in the PR objectives, so consumers of this package will now need to provide jszip themselves.
packages/register/vite.config.ts (1)
29-29: Correctly externalizing lodash-esAdding 'lodash-es' to the external dependencies is appropriate and aligns with the PR objective. Lodash is a commonly used utility library that's better served as an external dependency to avoid duplication across packages.
Note that this change is part of a breaking change as mentioned in the PR objectives, so consumers will now need to provide lodash-es themselves.
packages/plugins/tutorial/vite.config.ts (1)
35-35: Good addition of marked as external dependencyAdding 'marked' to the external dependencies list is consistent with the PR's goal to externalize all core package dependencies. This markdown parser library is commonly used and makes sense as an external dependency.
This is part of the breaking change mentioned in the PR objectives, so consumers will now need to ensure the 'marked' package is available.
packages/layout/vite.config.ts (1)
35-35: Correctly externalizing @vueuse/core and vue-draggable-nextAdding '@vueuse/core' and 'vue-draggable-next' to the external dependencies list is consistent with the PR objective. These are commonly used Vue ecosystem libraries that are sensible choices for externalization.
As part of the breaking change noted in the PR, consumers will need to provide these dependencies themselves going forward.
packages/plugins/i18n/vite.config.ts (1)
35-35: LGTM! New external dependency is properly added.Adding 'vue-clipboard3' to the external dependencies list is consistent with the PR objective of externalizing all dependencies of the core package. This will prevent bundling this dependency, reducing the package size and avoiding potential duplicate dependencies in the final application.
Just ensure that 'vue-clipboard3' is properly documented as a peer dependency in the package.json file.
packages/plugins/block/vite.config.ts (1)
35-35: LGTM! External dependencies properly expanded.Adding '@vue/shared', 'html2canvas', and 'pinyin-pro' to the external dependencies list is aligned with the PR objective of externalizing core package dependencies. This approach reduces bundle size and gives consumers more flexibility in managing dependency versions.
Ensure these newly externalized dependencies are documented as peer dependencies in the package.json file.
packages/plugins/datasource/vite.config.js (1)
35-43: LGTM! External dependencies reformatted and expanded.The reformatted and expanded external dependencies list with additions of '@vue/shared', 'axios', 'vue-clipboard3', and 'xlsx' improves readability and aligns with the PR objective. Externalizing these dependencies provides better separation of concerns and more flexibility for consumers of this package.
Remember to update the package.json to include these dependencies as peer dependencies with appropriate version constraints.
packages/vue-generator/vite.config.js (1)
47-55: LGTM! Comprehensive external dependencies configuration.The expanded external dependencies list now properly includes Vue packages and namespace patterns. Using regex patterns for scoped packages (
/@vue\/.*/and/@opentiny\/tiny-engine.*/) is a good approach that avoids listing individual packages under these namespaces.This change will significantly impact how this package is consumed, so ensure that:
- These externalized dependencies are documented as peer dependencies in package.json
- Documentation is updated to inform consumers about these new external requirements
packages/toolbars/lang/vite.config.ts (1)
35-35: External dependency '@vueuse/core' added correctly.Adding '@vueuse/core' to the external dependencies list aligns with the PR objective of externalizing all dependencies of the core package. This ensures '@vueuse/core' is not bundled with the output, reducing bundle size and avoiding potential duplicate dependencies.
packages/settings/styles/vite.config.ts (1)
35-35: External dependencies '@vueuse/core' and 'postcss' added correctly.Adding '@vueuse/core' and 'postcss' to the external dependencies list ensures these modules won't be bundled with the output. This change is consistent with the PR objective of externalizing all dependencies of the core package.
packages/common/vite.config.ts (1)
69-76: External dependencies expansion looks good.The additions to the external dependencies list are comprehensive and well-organized:
- Added regex patterns for '@babel.', '@eslint.', '@vueuse.', and '@VUE.' packages
- Added specific modules: 'axios', 'css-tree', 'eslint-linter-browserify', and 'vue-draggable-next'
This implementation correctly fulfills the PR objective of externalizing all dependencies.
packages/canvas/vite.config.ts (1)
54-66: External dependencies structured clearly and comprehensively.The external dependencies array has been well-structured with:
- Core Vue-related packages ('vue', '@vueuse/core', 'vue-i18n')
- Organization-specific packages via regex patterns (OpenTiny packages)
- Tooling-related packages ('@babel.', '@VUE.')
- Specific utility packages ('diff-match-patch', 'jsondiffpatch', 'postcss', 'postcss-selector-parser')
This comprehensive externalization aligns with the PR objective and follows a consistent pattern with other package configurations.
packages/design-core/vite.config.js (1)
82-98: Expanded external dependencies list aligns with PR objectiveThe updated
externalarray in rollup options now includes a comprehensive list of modules to be treated as external during bundling. This change properly externalizes all dependencies of the core package as intended in the PR objectives.The externalization strategy uses both specific module names and regex patterns, which is an effective approach to cover related packages under common namespaces.
packages/canvas/scripts/vite-plugin-separate-build.ts (2)
8-8: Type annotation added for improved type safetyAdding the string type annotation to the
cleanUrlparameter enhances code readability and type safety.
91-92: Simplified code export formatThe previous implementation that encoded bundled code as a base64 data URI has been replaced with a direct JSON-stringified export. This simplification:
- Makes debugging easier since the code is no longer base64-encoded
- Coordinates with the changes in the canvas initialization that conditionally injects scripts
- Aligns with the overall externalization strategy in the PR
This change streamlines how bundled code is included in the build output.
packages/canvas/init-canvas/init-canvas.ts (2)
5-5: Improved code structure with local variableUsing a local variable
resbefore returning improves code structure and readability without changing functionality.Also applies to: 20-21
12-18: Environment-specific script loading strategyThe implementation now uses different script embedding strategies based on environment:
- Development: External script tag with src attribute
- Production: Inline script with properly escaped content
This approach optimizes for both development (easier debugging) and production (fewer HTTP requests) while properly handling escaping issues by doubling
$characters in the inline script.This change works well with the simplified export format in the vite-plugin-separate-build.ts file.
packages/webcomponent/vite.config.js
Outdated
| rollupOptions: { | ||
| // 确保外部化处理那些你不想打包进库的依赖 | ||
| external: ['vue', 'vue-i18n'], | ||
| external: ['vue', 'vue-i18n', '@vue/shared'], |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Good externalization of @vue/shared dependency.
Adding '@vue/shared' to the externals list is appropriate for this package. However, I notice that you've updated the globals mapping for 'vue' and 'vue-i18n' but not for '@vue/shared'.
Consider whether you need to add '@vue/shared' to the globals object for UMD builds:
🏁 Script executed:
#!/bin/bash
# Check if @vue/shared is used in UMD builds and needs a global variable mapping
# Search for imports of @vue/shared in the webcomponent package
rg -A 2 "from ['\"]\@vue/shared['\"]" packages/webcomponent/Length of output: 167
I’ll check the Vite config and scan for any actual imports of @vue/shared before deciding if a UMD global is needed.
🏁 Script executed:
#!/bin/bash
set -e
echo "1) Locate Vite config for webcomponent package"
rg -l "vite.config.js"
echo -e "\n2) Show externals & globals in vite.config.js"
sed -n '1,200p' packages/webcomponent/vite.config.js | sed -n '30,60p'
echo -e "\n3) Find any usage of @vue/shared in source files"
rg -n "@vue/shared" packages/webcomponentLength of output: 1407
Add missing UMD global for @vue/shared
The webcomponent bundle imports utilities from @vue/shared (e.g. in src/defineCustomElement.js), and you’ve externalized it in Rollup, but you haven’t provided a corresponding global in the UMD build. This will lead to a runtime “undefined” when consumers use the UMD script.
• File: packages/webcomponent/vite.config.js
• Location: under rollupOptions.output.globals
Suggested diff:
output: {
globals: {
vue: 'Vue',
'vue-i18n': 'VueI18n',
+ '@vue/shared': 'VueShared'
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| external: ['vue', 'vue-i18n', '@vue/shared'], | |
| // packages/webcomponent/vite.config.js | |
| export default defineConfig({ | |
| build: { | |
| rollupOptions: { | |
| external: ['vue', 'vue-i18n', '@vue/shared'], | |
| output: { | |
| globals: { | |
| vue: 'Vue', | |
| 'vue-i18n': 'VueI18n', | |
| '@vue/shared': 'VueShared' | |
| } | |
| } | |
| } | |
| } | |
| }) |
chilingling
force-pushed
the
feat/externalAllDepsFromCore
branch
from
d986aa0 to
50e44f1
Compare
chilingling
force-pushed
the
feat/externalAllDepsFromCore
branch
from
50e44f1 to
5cb2abc
Compare
chilingling
changed the title
chilingling
force-pushed
the
feat/externalAllDepsFromCore
branch
from
54ccc3a to
616f266
Compare
2 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit