-
Notifications
You must be signed in to change notification settings - Fork 18
Merge origin/main into origin/feat/2.6 #1104
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
Conversation
Merge origin/main into origin/dev
fix: touchcancel event unbind
fix: implemented a check for the BOM object in SSR environments before usage
Merge origin/dev into origin/main
docs: changelog 2.5.6
WalkthroughThis update increments the version to 2.5.6 across all relevant packages and updates changelogs. It introduces a check for the Browser Object Model (BOM) before usage in SSR environments, centralizes the BOM check utility, and improves event handling by properly managing the touchcancel event in the interaction system. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EventSystem
participant DeviceUtils
participant GLContext
User->>EventSystem: Initiates interaction (touch/mouse)
EventSystem->>EventSystem: Listen for touchend, touchcancel, mouseleave
EventSystem-->>User: Handles touchend/touchcancel/mouseleave events
User->>GLContext: Requests GL context initialization
GLContext->>DeviceUtils: Check canUseBOM
DeviceUtils-->>GLContext: Returns BOM availability
GLContext-->>User: Throws error or initializes context based on environment
Estimated code review effort🎯 2 (Simple) | ⏱️ ~9 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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
Documentation and Community
|
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: 1
🧹 Nitpick comments (4)
CHANGELOG-zh_CN.md (1)
11-17
: Chinese changelog entries are partly written in English – please localize for consistency
Fix: implemented a check …
和Fix: touchcancel event unbind
等描述仍为英文,而同文件其余条目全部使用中文表述。建议统一语言,避免中英文混用造成阅读割裂。CHANGELOG.md (1)
14-15
: Tighten wording of the second bullet
touchcancel event unbind
可读性略差,建议改成 “Fixed incorrect un-binding of the touchcancel event” 以点明 “fixed” 与 “event” 的关系并保持与其它条目语气一致。plugin-packages/alipay-downgrade/src/native-log.ts (2)
26-28
: Message concatenation drops separators
args.join('')
会把参数黏连在一起 (foo
和bar
→foobar
)。
将分隔符设为空格能保持可读性:- 'message': `${prefix} ${msg} ${args.join('')}`, + 'message': `${prefix} ${msg} ${args.join(' ')}`,
20-41
: Optional: short-circuit when Alipay bridge unavailable当前即便
ap
为undefined
仍注册nativeLogger
,后续每次写日志都会进入函数并做空检查。
若希望在非 Alipay 环境完全跳过,可在准备阶段直接返回 early 以省一次函数调用:const ap = isAlipayMiniApp() ? my : window.AlipayJSBridge; + if (!ap) { + // not running in Alipay container, no native log needed + return; + } const nativeLogger = (type: string, msg: string, ...args: string[]) => {仅作性能微调参考。
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
CHANGELOG-zh_CN.md
(1 hunks)CHANGELOG.md
(1 hunks)packages/effects-core/package.json
(1 hunks)packages/effects-core/src/gl/index.ts
(2 hunks)packages/effects-core/src/plugins/interact/event-system.ts
(3 hunks)packages/effects-core/src/utils/device.ts
(1 hunks)packages/effects-helper/package.json
(1 hunks)packages/effects-threejs/package.json
(1 hunks)packages/effects-webgl/package.json
(1 hunks)packages/effects/package.json
(1 hunks)plugin-packages/alipay-downgrade/package.json
(1 hunks)plugin-packages/alipay-downgrade/src/native-log.ts
(1 hunks)plugin-packages/alipay-downgrade/src/utils.ts
(1 hunks)plugin-packages/downgrade/package.json
(1 hunks)plugin-packages/editor-gizmo/package.json
(1 hunks)plugin-packages/model/package.json
(1 hunks)plugin-packages/multimedia/package.json
(1 hunks)plugin-packages/orientation-transformer/package.json
(1 hunks)plugin-packages/rich-text/package.json
(1 hunks)plugin-packages/spine/package.json
(1 hunks)plugin-packages/stats/package.json
(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
packages/effects-helper/package.json (2)
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in plugin-packages/model/src/utility/plugin-helper.ts
do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in plugin-packages/model/src/utility/plugin-helper.ts
do not need to be merged as per project preference or coding standards.
plugin-packages/rich-text/package.json (2)
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in plugin-packages/model/src/utility/plugin-helper.ts
do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in plugin-packages/model/src/utility/plugin-helper.ts
do not need to be merged as per project preference or coding standards.
plugin-packages/model/package.json (2)
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in plugin-packages/model/src/utility/plugin-helper.ts
do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in plugin-packages/model/src/utility/plugin-helper.ts
do not need to be merged as per project preference or coding standards.
plugin-packages/alipay-downgrade/src/native-log.ts (3)
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in plugin-packages/model/src/utility/plugin-helper.ts
do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in plugin-packages/model/src/utility/plugin-helper.ts
do not need to be merged as per project preference or coding standards.
Learnt from: wumaolinmaoan
PR: #691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts
, when accumulating color components, avoid using clone()
methods because it can cause additional garbage collection overhead.
plugin-packages/alipay-downgrade/src/utils.ts (3)
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in plugin-packages/model/src/utility/plugin-helper.ts
do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in plugin-packages/model/src/utility/plugin-helper.ts
do not need to be merged as per project preference or coding standards.
Learnt from: wumaolinmaoan
PR: #691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts
, when accumulating color components, avoid using clone()
methods because it can cause additional garbage collection overhead.
packages/effects-core/src/gl/index.ts (1)
Learnt from: wumaolinmaoan
PR: #691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts
, when accumulating color components, avoid using clone()
methods because it can cause additional garbage collection overhead.
🧬 Code Graph Analysis (1)
packages/effects-core/src/gl/index.ts (1)
packages/effects-core/src/utils/device.ts (2)
canUseBOM
(14-14)isIOS
(16-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (22)
plugin-packages/editor-gizmo/package.json (1)
3-3
: Version bump aligns with the coordinated 2.5.6 patch release
No issues spotted. Make sure the workspace’s lock-file is regenerated before publishing.plugin-packages/model/package.json (1)
3-3
: Consistent version increment to 2.5.6
Looks correct and in sync with the other packages.plugin-packages/spine/package.json (1)
3-3
: Spine plugin version updated correctly
Nothing else to flag.plugin-packages/orientation-transformer/package.json (1)
3-3
: Orientation-transformer plugin now at 2.5.6
Change is straightforward and safe.plugin-packages/downgrade/package.json (1)
3-3
: Downgrade plugin version bump verified
All good; no additional adjustments required.packages/effects-webgl/package.json (1)
3-3
: Version bump looks goodPatch-level bump aligns this package with the rest of the workspace. No other issues spotted.
packages/effects-core/package.json (1)
3-3
: Version bump acknowledged
effects-core
is now 2.5.6 – consistent with other packages. ✔️packages/effects-threejs/package.json (1)
3-3
: Version bump only – okNo additional metadata changed; nothing to flag.
plugin-packages/rich-text/package.json (1)
3-3
: Rich-text plugin version increment approvedSynchronises with the 2.5.6 release train. Looks good.
packages/effects-helper/package.json (1)
3-3
: Version bump LGTMThe package has been bumped to 2.5.6 in sync with the rest of the mono-repo. No other changes detected.
packages/effects/package.json (1)
3-3
: Version bump LGTMThe root runtime package correctly reflects the 2.5.6 patch release.
plugin-packages/multimedia/package.json (1)
3-3
: Version bump LGTMMultimedia plugin version aligned to 2.5.6; no further adjustments necessary.
plugin-packages/stats/package.json (1)
3-3
: Version bump LGTMStats plugin now points to 2.5.6 in lockstep with other packages.
plugin-packages/alipay-downgrade/package.json (1)
3-3
: Version bump LGTMAlipay-downgrade plugin version updated to 2.5.6; consistent with the overall release.
plugin-packages/alipay-downgrade/src/native-log.ts (1)
13-13
: Import path looks good—verify thatcanUseBOM
is re-exported替换为统一入口可减少代码重复 👍。请确认
@galacean/effects
已在 2.5.6 正确 re-exportcanUseBOM
,以免出现打包时 tree-shaking / d.ts 缺失问题。plugin-packages/alipay-downgrade/src/utils.ts (2)
1-1
: LGTM! Good refactoring to centralize BOM availability check.The addition of
canUseBOM
to the import statement properly centralizes the BOM availability check instead of defining it locally, improving code consistency across the codebase.
18-25
: Excellent SSR environment handling.The early return when
canUseBOM
is false properly prevents errors in server-side rendering environments wherewindow
is not available. The returned downgrade result is appropriately structured withdowngrade: true
and a clear reason message.packages/effects-core/src/plugins/interact/event-system.ts (2)
52-52
: Proper touchcancel event mapping for both mouse and touch scenarios.The implementation correctly maps
touchcancel
to'mouseleave'
for mouse events and'touchcancel'
for touch events, following the established pattern of other touch event variables. This ensures consistent event handling across different input methods.Also applies to: 87-87
136-136
: Smart reuse of touchend handler for touchcancel events.Assigning the
touchend
handler to handletouchcancel
events is appropriate since both scenarios require the same cleanup logic: resetting thecurrentTouch
state and dispatching the final touch events. This ensures consistent behavior when touches are canceled.packages/effects-core/src/gl/index.ts (3)
2-2
: LGTM! Consistent with BOM availability centralization.Adding
canUseBOM
to the import properly utilizes the centralized BOM availability check from the utils module, maintaining consistency across the codebase.
10-11
: Clear and descriptive error message constants.The new error constants provide specific, descriptive messages that distinguish between iOS 16 lockdown mode issues and general WebGL environment problems. This improves debugging capability and code maintainability.
25-32
: Enhanced error handling with proper environment context.The improved error handling logic appropriately distinguishes between different failure scenarios:
- BOM available environments get specific error messages (iOS 16 lockdown mode vs. general WebGL issues)
- SSR environments (no BOM) get the general WebGL constants error
This provides better debugging information while properly handling server-side rendering scenarios.
// window 对象不存在时需要判断 | ||
export const canUseBOM = typeof window !== 'undefined'; | ||
|
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.
🛠️ Refactor suggestion
Use canUseBOM
inside the helper predicates to avoid SSR-time ReferenceError
s
Exporting canUseBOM
is a solid first step, but helpers such as isWechatMiniApp
, isAndroid
, isIOS
, etc. still dereference window
/navigator
un-guarded. When these helpers are invoked in an SSR context they will still throw, defeating the purpose of the new flag.
Apply the flag internally (or document the contract) so that the helpers are safe by default:
-export function isWechatMiniApp () {
- return window.__wxjs_environment === 'miniprogram';
-}
+export function isWechatMiniApp (): boolean {
+ return canUseBOM && (window as any).__wxjs_environment === 'miniprogram';
+}
Repeating the same pattern for the other predicates keeps the public API ergonomic while guaranteeing crash-free SSR.
🤖 Prompt for AI Agents
In packages/effects-core/src/utils/device.ts around lines 13 to 15, the helpers
like isWechatMiniApp, isAndroid, and isIOS directly access window or navigator
without checking canUseBOM, causing ReferenceErrors during SSR. To fix this,
update these helper functions to internally check canUseBOM before accessing
window or navigator, returning false or a safe default if canUseBOM is false.
This ensures the helpers are safe to call in SSR contexts without throwing
errors.
Summary by CodeRabbit
Bug Fixes
Chores