-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[HybridWebView] Fix some issues with the typescript #29873
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
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.
Pull Request Overview
This pull request addresses TypeScript issues in the HybridWebView implementation to improve compatibility with older Android versions that exhibit unusual Chrome behavior. The changes include replacing optional chaining with explicit property checks and refactoring the public API functions from inline object literal methods to standalone function declarations that are then assembled into the HybridWebView object.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Core/src/Handlers/HybridWebView/HybridWebView.ts | Replaced optional chaining with explicit null checks and refactored inline method declarations into standalone functions for clarity and compatibility. |
| src/Core/src/Handlers/HybridWebView/HybridWebView.js | Applied similar changes as in TypeScript for consistent behavior on multiple platforms, with improved compatibility checks and API refactoring. |
Comments suppressed due to low confidence (1)
src/Core/src/Handlers/HybridWebView/HybridWebView.js:117
- [nitpick] Moving from inline method definitions to standalone functions improves the structure of the API. Verify that the new function names are used consistently throughout the code.
function sendRawMessage(message) {
| if (window.chrome && window.chrome.webview && window.chrome.webview.addEventListener) { | ||
| // Windows WebView2 | ||
| window.chrome.webview.addEventListener('message', (arg: any) => { | ||
| dispatchHybridWebViewMessage(arg.data); | ||
| }); | ||
| } else if (window.webkit?.messageHandlers?.webwindowinterop) { | ||
| } else if (window.webkit && window.webkit.messageHandlers && window.webkit.messageHandlers.webwindowinterop) { |
Copilot
AI
Jun 6, 2025
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.
[nitpick] Explicitly checking for each nested property improves compatibility with older platforms. If this pattern recurs, consider extracting it into a helper function to improve readability and maintainability.
Copilot uses AI. Check for mistakes.
| * | ||
| * @param message The message to send to the .NET host application. | ||
| */ | ||
| function sendRawMessage(message: string) { |
Copilot
AI
Jun 6, 2025
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.
[nitpick] Refactoring inline object literal methods into standalone function declarations enhances clarity and modularity. Please ensure consistency in naming and ordering across similar functions for a uniform API.
| function sendRawMessage(message: string) { | |
| export function sendRawMessage(message: string) { |
Copilot uses AI. Check for mistakes.
| if (window.chrome && window.chrome.webview && window.chrome.webview.addEventListener) { | ||
| // Windows WebView2 | ||
| window.chrome.webview.addEventListener('message', (arg) => { | ||
| dispatchHybridWebViewMessage(arg.data); | ||
| }); | ||
| } | ||
| else if (window.webkit?.messageHandlers?.webwindowinterop) { | ||
| else if (window.webkit && window.webkit.messageHandlers && window.webkit.messageHandlers.webwindowinterop) { |
Copilot
AI
Jun 6, 2025
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.
[nitpick] The explicit property checks here replace optional chaining to support older environments. Consider a utility function if similar nested checks are needed in other parts of the codebase.
Copilot uses AI. Check for mistakes.
Description of Change
The older Android versions have a weird Chrome version with weird JS support.
See also #29829