-
Notifications
You must be signed in to change notification settings - Fork 355
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
refactor: extract URL parameter related logic to defaultGlobalService #1103
refactor: extract URL parameter related logic to defaultGlobalService #1103
Conversation
WalkthroughThe changes refactor several components and composable modules to centralize URL and state management via a meta API. Functions previously updating location history, preview, page, and block IDs directly have been replaced with calls to getMetaApi(META_SERVICE.GlobalService). Additionally, redundant methods such as postLocationHistoryChanged have been removed and new utility functions added to the default global service module. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Component
participant M as Meta API
participant G as Global Service
U->>C: Trigger update (edit block/update preview/page)
C->>M: getMetaApi(META_SERVICE.GlobalService)
M->>G: Invoke update method (updateBlockId/updatePreviewId/updatePageId)
G-->>C: Return confirmation/update result
Possibly related PRs
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 (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/canvas/DesignCanvas/src/DesignCanvas.vue (1)
231-237
: Consider simplifying the URL parameters transformation.The current implementation can be made more concise while maintaining the same functionality.
-const postUrlChanged = () => { - getMetaApi(META_SERVICE.GlobalService).postLocationHistoryChanged( - Object.fromEntries( - Array.from(new URLSearchParams(window.location.search)).map(([key, value]) => [replaceKey(key), value]) - ) - ) -} +const postUrlChanged = () => { + const params = Object.fromEntries( + [...new URLSearchParams(window.location.search)].map(([key, value]) => [replaceKey(key), value]) + ) + getMetaApi(META_SERVICE.GlobalService).postLocationHistoryChanged(params) +}packages/plugins/page/src/composable/usePage.js (1)
343-343
: LGTM! Consider enhancing error handling.The updatePageId call has been correctly moved to GlobalService. However, consider resetting the page ID in the catch block to maintain URL consistency on failure.
.catch(() => { + getMetaApi(META_SERVICE.GlobalService).updatePageId('') useNotify({ type: 'error', message: '切换页面失败,目标页面不存在' }) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/canvas/DesignCanvas/src/DesignCanvas.vue
(2 hunks)packages/common/composable/defaultGlobalService.js
(2 hunks)packages/plugins/block/src/Main.vue
(2 hunks)packages/plugins/block/src/composable/useBlock.js
(0 hunks)packages/plugins/materials/src/composable/useResource.js
(2 hunks)packages/plugins/page/src/composable/usePage.js
(2 hunks)
💤 Files with no reviewable changes (1)
- packages/plugins/block/src/composable/useBlock.js
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (11)
packages/plugins/block/src/Main.vue (2)
136-144
: Use of additional import statements for metadata services looks aligned with the refactor goal.
These imports centralize operations (e.g., block ID updates) under the metadata service, keeping domain logic consistent.
314-314
: Refactor to metadata service properly ensures centralized block ID updates.
This line updates the block ID via the shared service, removing direct URL parameter manipulation. This is consistent with the refactor objective.packages/common/composable/defaultGlobalService.js (5)
62-63
: Publishing 'locationHistoryChanged' fosters better decoupling.
Maintaining location state changes through messaging is a clean approach for event-driven updates.
64-70
: updatePageId method introduced for central page ID updates.
This approach ensures any page-related refinements (e.g., logging, analytics) can be hooked here without scattering across the codebase.
72-78
: updateBlockId method introduced for block ID coordination.
Similar to updatePageId, it simplifies future expansions (like logging usage or concurrency control) in one central place.
80-96
: updatePreviewId method centralizes preview ID logic.
This effectively replaces arbitrary direct URL updates. The early return for matching IDs helps avoid unnecessary history records. Nicely done!
160-164
: Expanded set of exported APIs is consistent with the new architecture.
Exposing these functions in the service collectively supports a unified approach for location updates, consistent with the PR objectives.packages/plugins/materials/src/composable/useResource.js (2)
26-26
: Importing META_SERVICE ensures direct access to the global service.
Removing unused imports and opting for a single reference to the new centralized implementation improves clarity.
45-45
: Replacing direct URL mutation with updatePageId is a clean solution.
Leveraging the global service for page ID updates is more maintainable and simplifies the logic in this module.packages/canvas/DesignCanvas/src/DesignCanvas.vue (1)
267-267
: LGTM! Consistent with centralization effort.The updatePreviewId method has been correctly moved to GlobalService, aligning with the PR's objective of centralizing URL parameter logic.
packages/plugins/page/src/composable/usePage.js (1)
324-326
: LGTM! Maintains edge case handling.The updatePageId call has been correctly moved to GlobalService while preserving the special handling for pageId === 0.
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
抽取设置url参数相关的逻辑到
defaultGlobalService
defaultGlobalService
新增4个接口:postLocationHistoryChanged
通知url变化updatePageId
更新应用的pageIdupdateBlockId
更新应用的blockIdupdatePreviewId
更新应用的previewIdWhat is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information