feat: remove webComponent I18n host #1438
Conversation
WalkthroughThe changes remove all references to Changes
Sequence Diagram(s)sequenceDiagram
participant AppVue
participant VueComponents
participant i18n
AppVue->>i18n: Provide i18n context via provide()
AppVue->>VueComponents: Render TestVueUse and TestVueInject
VueComponents->>i18n: Inject and use i18n instance
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/i18n/package.json (1)
7-7: Update package description to reflect webcomponent removal.The package description still mentions "webcomponent vue i18n host" but the webcomponent functionality has been removed. Consider updating the description to reflect the current functionality.
- "description": "webcomponent vue i18n host", + "description": "vue i18n host",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/canvas/package.json(0 hunks)packages/canvas/render/src/canvas-function/custom-renderer.ts(1 hunks)packages/common/js/import-map.json(0 hunks)packages/design-core/package.json(0 hunks)packages/i18n/package.json(1 hunks)packages/i18n/src/App.vue(1 hunks)packages/i18n/src/I18nHost.vue(0 hunks)packages/i18n/src/lib.ts(0 hunks)packages/i18n/vite.config.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/common/js/import-map.json
- packages/i18n/src/lib.ts
- packages/i18n/src/I18nHost.vue
- packages/design-core/package.json
- packages/canvas/package.json
🧰 Additional context used
🧠 Learnings (1)
packages/canvas/render/src/canvas-function/custom-renderer.ts (1)
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/canvas-function/custom-renderer.ts:28-32
Timestamp: 2025-01-14T07:11:44.138Z
Learning: The locale and webComponent wrapper in `packages/canvas/render/src/canvas-function/custom-renderer.ts` are part of migrated code and will be improved in a future PR.
🧬 Code Graph Analysis (1)
packages/canvas/render/src/canvas-function/custom-renderer.ts (1)
packages/canvas/render/src/render.ts (1)
renderer(228-321)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (5)
packages/i18n/vite.config.ts (1)
42-48: LGTM! Consistent removal of webcomponent dependency from build configuration.The changes correctly remove both the external dependency and its corresponding global variable mapping for
@opentiny/tiny-engine-webcomponent-core, maintaining consistency in the Vite build configuration.packages/i18n/package.json (1)
9-10:✅ Verification successful
Verify if removed scripts are still needed.
The removal of
lint,format, andpublish:npmscripts may impact development workflow and CI/CD processes. Please verify that these scripts are not required elsewhere in the build pipeline.
🏁 Script executed:
#!/bin/bash # Description: Check if removed scripts are referenced in CI/CD files or other scripts # Expected: Find any references to the removed scripts that might break workflows echo "Searching for references to removed scripts..." rg -i "lint|format|publish:npm" --type yaml --type json --type js --type ts .github/ scripts/ || echo "No CI/CD references found" # Check for package.json files that might depend on these scripts rg -A 5 -B 5 "packages/i18n.*lint|packages/i18n.*format|packages/i18n.*publish" --type jsonLength of output: 1187
🏁 Script executed:
#!/bin/bash # Refined search for direct invocations of the removed scripts in CI/CD and helper scripts echo "Searching for direct invocations of removed scripts in .github workflows..." rg -i 'npm\s+run\s+(lint|format|publish:npm)' .github/ || echo "No direct npm run invocations found in .github/" rg -i 'yarn\s+(lint|format|publish:npm)' .github/ || echo "No yarn invocations found in .github/" rg -i 'pnpm\s+run\s+(lint|format|publish:npm)' .github/ || echo "No pnpm invocations found in .github/" echo "Searching for direct invocations of removed scripts in scripts/ directory..." rg -i 'npm\s+run\s+(lint|format|publish:npm)' scripts/ || echo "No npm run invocations found in scripts/" rg -i 'yarn\s+(lint|format|publish:npm)' scripts/ || echo "No yarn invocations found in scripts/" rg -i 'pnpm\s+run\s+(lint|format|publish:npm)' scripts/ || echo "No pnpm invocations found in scripts/"Length of output: 1193
Removed scripts are not referenced elsewhere
No CI/CD workflows or helper scripts invokelint,format, orpublish:npmin packages/i18n. The removal is safe.packages/canvas/render/src/canvas-function/custom-renderer.ts (1)
28-32: LGTM! Simplified rendering logic by removing web component wrapper.The direct rendering approach correctly maintains the original logic while removing the
tiny-i18n-hostwrapper. This should resolve the Vue 3.5 compatibility issues mentioned in the PR objectives.Based on retrieved learnings, this migrated code will be further improved in future PRs, which is appropriate for maintaining the current functionality while addressing the immediate compatibility concern.
packages/i18n/src/App.vue (2)
9-12: LGTM! Simplified template by removing web component wrapper.The direct rendering of Vue components instead of using the
tiny-i18n-hostweb component wrapper correctly addresses the Vue 3.5 compatibility issue.
28-28: Verify if locale change from 'en_US' to 'zh_CN' is intentional.The default locale was changed from
'en_US'to'zh_CN'. Please confirm if this change is intentional or if it should remain'en_US'for consistency.
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
修复问题：#1433 (升级到 vue 3.5 版本之后，画布空白，不显示)
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
New Features
Bug Fixes
Refactor
Chores