feat: go through the mobile packaging build and release process and fix the bugs found during the build and release #2780
Conversation
…ix the bugs found during the build and release
WalkthroughThis pull request involves modifications across several files in the mobile package. The primary changes include updating type import statements in various Vue components to use local imports instead of imports from the renderless package. Additionally, the package.json file has been updated with a new version, publish configuration, and a new dependency. The utility files have seen the removal of version-related exports, and the Vite configuration has been enhanced with new plugins for build processing. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
WalkthroughThis pull request addresses the mobile packaging build and release process by fixing bugs encountered during these processes. The changes involve updating import paths for type definitions, removing unused variables, and enhancing the build configuration with new plugins. Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| @@ -262,8 +262,6 @@ export const CASCADER = { | |||
| MenuConnector: 'cascader-menu-' | |||
| } | |||
|
|
|||
| export const version = process.env.RUNTIME_VERSION | |||
|
|
|||
| export const log = (data, type = 'log') => { | |||
There was a problem hiding this comment.
The removal of version might affect parts of the codebase that rely on this variable. Ensure that this change does not break any functionality.
| @@ -41,8 +41,6 @@ import validate from './validate' | |||
| import memorize from './deps/memorize' | |||
| import * as common from '.' | |||
|
|
|||
| const version = process.env.RUNTIME_VERSION | |||
|
|
|||
| const Renderless = { | |||
There was a problem hiding this comment.
The removal of version might affect parts of the codebase that rely on this variable. Ensure that this change does not break any functionality.
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
zzcr
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
packages/mobile/utils/runtime.ts (3)
Line range hint
15-18: Consider translating Chinese comments to English for better maintainability.The comment block explains that these are common methods exposed to users at runtime, and dependencies should be imported using package paths. Consider translating it to English to maintain consistency and improve accessibility for non-Chinese speakers.
-/* - * 以下是能对外抛出给用户使用的通用方法（运行时用） - * 涉及以下的依赖统一使用包名路径的形式引入 - * 导出对象命名规范：以文件名称的驼峰格式 - */ +/* + * The following are common methods exposed to users (for runtime use) + * All dependencies should be imported using package paths + * Export object naming convention: camelCase format of the filename + */
Line range hint
42-43: Consider removing or updating the TINY_NO_NEED comment.The comment indicates that the
vue-popuppackage is only used internally by dialog-box. Consider:
- Moving this internal dependency to the dialog-box component if it's truly only used there
- Documenting why it needs to be in the shared utils if it must remain
-/** TINY_NO_NEED 这个包只是dialog-box 自己使用的一个中间包，暴露给用户，用户也不会使用它呀 */ +/** + * Internal dependency used by dialog-box component. + * @internal + */
Line range hint
19-41: Consider organizing imports into logical groups.While the imports are somewhat grouped, they could be better organized for improved readability and maintainability. Consider grouping them into:
- Core utilities
- DOM-related utilities
- Vue-specific utilities
+// Core utilities import * as array from './array' import browser from './browser' import * as date from './date' import * as decimal from './decimal' import * as object from './object' import * as string from './string' import * as type from './type' import * as dataset from './dataset' + +// DOM-related utilities import afterLeave from './deps/after-leave' import clickoutside from './deps/clickoutside' import debounce from './deps/debounce' import * as dom from './deps/dom' -import popper from './deps/popper' -import popupManager from './deps/popup-manager' import * as resizeEvent from './deps/resize-event' import * as scrollbarWidth from './deps/scrollbar-width' import throttle from './deps/throttle' + +// Vue-specific utilities +import popper from './deps/popper' +import popupManager from './deps/popup-manager' import vueEmitter from './deps/vue-emitter' import vuePopper from './deps/vue-popper' import vuePopup from './deps/vue-popup'packages/mobile/vite.config.ts (1)
12-12: Configure DTS plugin optionsThe DTS plugin would benefit from explicit configuration options for better TypeScript declaration file generation.
- dts(), + dts({ + outDir: 'dist/types', + skipDiagnostics: false, + logDiagnostics: true + }),packages/mobile/package.json (1)
28-30: Consider expanding the files arrayThe files array should include other essential files like README, LICENSE, and changelog.
"files": [ - "dist" + "dist", + "README.md", + "LICENSE", + "CHANGELOG.md" ],packages/mobile/components/tab-item/src/mobile.vue (1)
29-29: Overall Pattern: Localizing Type ImportsThe consistent pattern across all components of moving type imports from
@opentiny/vue-renderless/types/*to local files aligns well with the PR's objective of fixing mobile packaging build issues. This change:
- Reduces external dependencies
- Simplifies the build process
- Improves maintainability
- Follows the existing pattern of local prop imports
Consider documenting this architectural decision to maintain consistency for future component additions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/mobile/components/form/src/mobile.vue(1 hunks)packages/mobile/components/tab-item/src/mobile.vue(1 hunks)packages/mobile/components/tag/src/mobile.vue(1 hunks)packages/mobile/components/toast/src/mobile.vue(1 hunks)packages/mobile/package.json(3 hunks)packages/mobile/utils/index.ts(0 hunks)packages/mobile/utils/runtime.ts(2 hunks)packages/mobile/vite.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/mobile/utils/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR Unit Test
🔇 Additional comments (5)
packages/mobile/utils/runtime.ts (1)
67-67: Verify version information management after removal.The removal of the
versionconstant suggests that version information might be managed differently now. Please ensure that:
- Version information is properly managed elsewhere in the codebase
- Any features depending on this version constant have been updated
- Documentation reflects the new way of accessing version information
Let's verify the version management changes:
Also applies to: 95-95
✅ Verification successful
Version constant removal verified as safe
The version information is properly managed through package.json files and component-level version imports. No dependencies on the removed version constant were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for version-related changes in the codebase echo "Searching for version-related files and imports..." # Look for version management in package.json echo "Checking package.json files:" fd package.json | xargs rg "version" # Look for version imports/usage echo "Checking version imports and usage:" rg -g '!package.json' -g '!package-lock.json' "RUNTIME_VERSION|version" # Look for version-related configuration echo "Checking version configuration:" fd "vite.config|vue.config" | xargs rg "version"Length of output: 66464
packages/mobile/components/toast/src/mobile.vue (1)
18-18: LGTM! Verify local type definition.The change to use a local import for
IToastApiis consistent with the build process improvements.✅ Verification successful
Type definitions verified successfully
The local import of
IToastApiis properly supported with complete type definitions intoast.ts.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify IToastApi type exists in local file ast-grep --pattern 'export interface IToastApi { $$$ }' packages/mobile/components/toast/src/toast.tsLength of output: 391
Script:
#!/bin/bash # Verify IToastState type exists ast-grep --pattern 'export interface IToastState { $$$ }' packages/mobile/components/toast/src/toast.tsLength of output: 438
packages/mobile/components/tag/src/mobile.vue (1)
26-26: LGTM! Verify local type definition.The change to use a local import for
ITagApiis consistent with the build process improvements.✅ Verification successful
✓ Local type definitions verified
The
ITagApiinterface and its dependencyITagStateare properly defined in the local file with all necessary type definitions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify ITagApi type exists in local file ast-grep --pattern 'export interface ITagApi { $$$ }' packages/mobile/components/tag/src/tag.tsLength of output: 448
Script:
#!/bin/bash # Verify ITagState type exists and is properly defined ast-grep --pattern 'interface ITagState { $$$ }' packages/mobile/components/tag/src/tag.tsLength of output: 668
packages/mobile/components/form/src/mobile.vue (1)
29-29: LGTM! Verify local type definition.The change to use a local import for
IFormApiis consistent with the build process improvements.✅ Verification successful
✓ Local type definition verified
The
IFormApiinterface is properly defined in the localform.tsfile with all necessary form-related types and methods.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify IFormApi type exists in local file ast-grep --pattern 'export interface IFormApi { $$$ }' packages/mobile/components/form/src/form.tsLength of output: 2040
packages/mobile/components/tab-item/src/mobile.vue (1)
29-29: LGTM! Verify local type definition.The change to use a local import for
ITabItemApiis consistent with the build process improvements.✅ Verification successful
Type definition verified successfully
The
ITabItemApiinterface is properly defined in the localtab-item.tsfile with all required properties.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify ITabItemApi type exists in local file ast-grep --pattern 'export interface ITabItemApi { $$$ }' packages/mobile/components/tab-item/src/tab-item.tsLength of output: 766
| replace({ | ||
| '.less': '.css' | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add preventAssignment option to replace plugin configuration
The replace plugin configuration should include preventAssignment: true and use string values for replacements to avoid potential build issues.
replace({
- '.less': '.css'
+ preventAssignment: true,
+ '.less': '.css',
})📝 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.
| replace({ | |
| '.less': '.css' | |
| }) | |
| replace({ | |
| preventAssignment: true, | |
| '.less': '.css', | |
| }) |
| "publishConfig": { | ||
| "main": "dist/index.js" | ||
| }, |
There was a problem hiding this comment.
Resolve inconsistent main field declarations
There's a conflict between:
- publishConfig.main: "dist/index.js"
- main: "index.ts"
This could cause module resolution issues for consumers.
"publishConfig": {
"main": "dist/index.js"
},
+ "main": "dist/index.js",
+ "module": "dist/index.js",
+ "types": "dist/types/index.d.ts",
- "main": "index.ts",Also applies to: 31-31
| "main": "index.ts", | ||
| "scripts": { | ||
| "build": "vite build" | ||
| "build": "vite build", | ||
| "pub": "pnpm publish --no-git-checks --access=public" |
There was a problem hiding this comment.
Reconsider using --no-git-checks in publish script
The --no-git-checks flag bypasses important safety checks and could lead to publishing uncommitted or untagged changes.
- "pub": "pnpm publish --no-git-checks --access=public"
+ "pub": "pnpm publish --access=public"📝 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.
| "pub": "pnpm publish --no-git-checks --access=public" | |
| "pub": "pnpm publish --access=public" |
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "name": "@opentiny/vue-mobile", | |||
| "type": "module", | |||
| "version": "3.21.0", | |||
| "version": "1.0.0-alpha.4", | |||
There was a problem hiding this comment.
💡 Codebase verification
Version downgrade creates dependency version mismatch
The version change to 1.0.0-alpha.4 creates a mismatch with your dependencies that use ~3.21.0:
- @opentiny/vue-icon
- @opentiny/vue-locale
- @opentiny/vue-theme-mobile
Consider either:
- Updating these dependencies to match the new versioning scheme, or
- Maintaining the current 3.x version line if backward compatibility is needed
🔗 Analysis chain
Version downgrade requires careful consideration
Changing version from 3.21.0 to 1.0.0-alpha.4 is a major breaking change that requires:
- Clear communication to users
- Migration guide
- Changelog updates
Let's verify the impact:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for dependents that might be affected
npm info "@opentiny/vue-mobile" dependencies
Length of output: 250
Script:
#!/bin/bash
# Search for dependencies on @opentiny/vue-mobile in package.json files
rg -l "@opentiny/vue-mobile" -g "package.json"
# Check for potential TypeScript references
fd "\.d\.ts$" | xargs rg -l "@opentiny/vue-mobile"
Length of output: 152
Script:
#!/bin/bash
# Check how examples/sites depends on @opentiny/vue-mobile
cat examples/sites/package.json | grep -A 1 -B 1 "vue-mobile"
Length of output: 206
走通mobile打包构建发布流程，修复构建发布中发现的bug
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
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
Release Notes
Type Imports
Package Configuration
Build Configuration
Utilities