Amam / Add native support bridge

[nijil-deriv]

Description

(Brief description of what this PR does)

Related Issue: Fixes #(issue)

Type of Change

• 🐛 Bug fix
• ✨ New feature
• 💥 Breaking change
• 🔧 Refactoring
• 📚 Documentation
• 🧪 Tests

Testing

• Unit tests pass
• Manual testing completed
• No console errors
• No test needed

Screenshots

(Add screenshots if UI changes)

Before:

After:

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details

Scanned Manifest Files

[claude]

Claude finished @nijil-deriv's task —— View job

---

Code Review Complete

I've reviewed the PR that adds native support bridge integration for mobile apps. Here's my detailed feedback:

Tasks

• Read and analyze the new useMobileBridge.ts hook
• Review header component changes (account-actions, toggle-menu-drawer, brand-short-logo)
• Review test coverage and quality
• Review global type definitions
• Check for correctness, performance, and security issues
• Provide structured feedback with priorities

---

Summary

This PR implements a bridge communication layer between the web app and native mobile containers (likely Flutter), enabling navigation commands like "back to app" and "home". The implementation is generally solid with good test coverage, but there are several issues that should be addressed.

---

High Priority Issues

1. Missing Hook Tests 🔴

Location: packages/core/src/App/Hooks/useMobileBridge.ts

The core useMobileBridge hook has no dedicated test file. This is critical functionality that handles bridge communication and fallback logic.

Recommendation: Create packages/core/src/App/Hooks/__tests__/useMobileBridge.spec.ts with tests for:

• Bridge message sending when available
• Fallback execution when bridge unavailable
• Error handling with both successful and failed fallbacks
• Return value correctness
• isBridgeAvailable logic

2. Security: Insufficient Input Validation 🔴

Location: useMobileBridge.ts:10

const message: DerivAppChannelMessage = { event };
window.DerivAppChannel.postMessage(JSON.stringify(message));

The hook accepts any event type matching the union type but doesn't validate it at runtime. If the event parameter is somehow manipulated or comes from an untrusted source, this could lead to unintended bridge commands.

Recommendation: Add runtime validation:

const validEvents = ['trading:back', 'trading:home'] as const;
if (!validEvents.includes(event)) {
    console.error(`Invalid bridge event: ${event}`);
    return false;
}

3. Race Condition in ToggleMenuDrawer 🔴

Location: toggle-menu-drawer.jsx:86-91

const handleLogout = React.useCallback(async () => {
    toggleDrawer();
    sendBridgeEvent('trading:back', async () => {
        await logoutClient();
    });
}, [logoutClient, toggleDrawer, sendBridgeEvent]);

toggleDrawer() is called before the bridge event completes. If the drawer close animation interferes with the logout flow, this could cause issues. The callback also doesn't await the async logout.

Recommendation:

const handleLogout = React.useCallback(async () => {
    const success = sendBridgeEvent('trading:back', async () => {
        await logoutClient();
    });
    if (success) {
        toggleDrawer();
    }
}, [logoutClient, toggleDrawer, sendBridgeEvent]);

4. Inconsistent Fallback Execution 🟡

Location: useMobileBridge.ts:7-31

The fallback in the catch block (lines 21-29) is executed even when the bridge successfully sent the message but threw an error afterward. This could result in double execution.

Recommendation: Restructure to only execute fallback when bridge is unavailable or fails to send:

if (!isDesktop && window.DerivAppChannel?.postMessage) {
    try {
        const message: DerivAppChannelMessage = { event };
        window.DerivAppChannel.postMessage(JSON.stringify(message));
        return true;
    } catch (error) {
        console.error('Failed to send bridge message:', error);
        // Only fall through to fallback if bridge failed
    }
}

// Execute fallback
if (fallback) {
    try {
        fallback();
        return true;
    } catch (fallbackError) {
        console.error('Fallback execution failed:', fallbackError);
        return false;
    }
}

---

Medium Priority Issues

5. Type Safety: Missing Return Type Annotations 🟡

Location: useMobileBridge.ts:4, 7, 34

The hook and its return functions lack explicit return type annotations, relying on inference.

Recommendation:

export const useMobileBridge = (): {
    sendBridgeEvent: (event: 'trading:back' | 'trading:home', fallback?: () => void) => boolean;
    isBridgeAvailable: () => boolean;
    isDesktop: boolean;
} => {

6. Async Fallback Not Awaited 🟡

Location: useMobileBridge.ts:14, 23

The fallback functions are called but not awaited, even though they can be async (as seen in toggle-menu-drawer.jsx:88).

Recommendation:

const sendBridgeEvent = useCallback(
    async (event: 'trading:back' | 'trading:home', fallback?: () => void | Promise<void>) => {
        // ... existing logic
        if (fallback) {
            await fallback();
            return true;
        }
    }, 
    [isDesktop]
);

7. Missing JSDoc Documentation 🟡

Location: useMobileBridge.ts

The hook lacks documentation explaining its purpose, parameters, and return values, making it harder for other developers to understand the bridge integration.

Recommendation: Add JSDoc:

/**
 * Hook for communicating with native mobile app containers via bridge
 * @returns Object with sendBridgeEvent function, isBridgeAvailable check, and isDesktop flag
 */

8. Accessibility Issue in BrandShortLogo 🟡

Location: brand-short-logo.tsx:22-26

<div onClick={handleLogoClick} style={{ cursor: 'pointer' }}>

Using a div for click handlers is not keyboard accessible or screen reader friendly.

Recommendation: Use a button or add proper ARIA attributes:

<button 
    onClick={handleLogoClick} 
    className="brand-logo-button"
   
   
>
    <BrandDerivLogoCoralIcon width={24} height={24} />
</button>

9. Console Errors in Production 🟡

Location: useMobileBridge.ts:19, 26

console.error calls will appear in production, which might expose implementation details.

Recommendation: Use a logging service or environment-based logging:

if (process.env.NODE_ENV !== 'production') {
    console.error('Failed to send bridge message:', error);
}

---

Low Priority Issues

10. Redundant isDesktop Exposure 🔵

Location: useMobileBridge.ts:41, account-actions.tsx:46

The hook exposes isDesktop from useDevice, but components could import useDevice directly. This creates an unnecessary dependency.

Recommendation: Remove isDesktop from the return value or document why it's included for convenience.

11. Test Mock Inconsistency 🔵

Location: Test files

The useMobileBridge hook is mocked differently across test files. Some tests mock sendBridgeEvent to call the fallback, others don't.

Recommendation: Create a shared test helper for consistent mocking:

// test-utils/mockMobileBridge.ts
export const mockMobileBridge = (options = {}) => ({
    sendBridgeEvent: jest.fn((event, fallback) => {
        if (options.executeFallback && fallback) fallback();
    }),
    isBridgeAvailable: jest.fn(() => options.bridgeAvailable ?? false),
    isDesktop: options.isDesktop ?? true,
});

12. Global Type Extension Could Be More Specific 🔵

Location: types/global.d.ts:16-19

interface DerivAppChannelMessage {
    event: 'trading:back' | 'trading:home';
    // Add other event types as needed
}

The comment suggests more events will be added, but there's no payload structure defined.

Recommendation: Add a more extensible structure:

interface DerivAppChannelMessage {
    event: 'trading:back' | 'trading:home';
    payload?: Record<string, unknown>;
}

13. Unnecessary Fragment in AccountActions 🔵

Location: account-actions.tsx:67, 39-43

const LoggedOutView = () => (
    <>
        <LoginButtonV2 className='acc-info__button' />
    </>
);

Fragment is unnecessary when returning a single element.

Recommendation:

const LoggedOutView = () => <LoginButtonV2 className='acc-info__button' />;

---

Performance Notes

✅ Good: useMobileBridge properly uses useCallback for memoization
✅ Good: AccountActions is wrapped in React.memo
✅ Good: ToggleMenuDrawer uses observer from MobX for optimal re-renders

---

Security Assessment

🟡 Medium Risk:

• Bridge communication lacks input validation (Issue build(deps-dev): bump axios from 1.7.9 to 1.11.0 #2)
• No sanitization of error messages before logging
• window.DerivAppChannel could be overridden by malicious code

Recommendations:

• Add runtime validation for event types
• Consider adding a nonce or signature verification for bridge messages if the native app supports it
• Freeze the DerivAppChannel interface after initialization

---

Test Coverage Analysis

✅ Good Coverage:

• account-actions.spec.tsx: 8 test cases covering bridge and fallback scenarios
• toggle-menu-drawer.spec.jsx: 7 test cases for drawer behavior with bridge
• brand-short-logo.spec.tsx: 7 test cases for logo navigation

❌ Missing:

• No direct tests for useMobileBridge hook (Issue build(deps-dev): bump webpack-dev-server from 5.2.0 to 5.2.1 #1)
• Edge case: What happens if window.DerivAppChannel exists but postMessage is undefined?
• Edge case: What if the bridge message succeeds but the native app doesn't respond?

---

Quick Fixes

Here are some safe patches you can apply immediately:

1. Add runtime validation to useMobileBridge.ts:

const validEvents = ['trading:back', 'trading:home'] as const;
const sendBridgeEvent = useCallback((event: 'trading:back' | 'trading:home', fallback?: () => void) => {
    if (!validEvents.includes(event)) {
        console.error(`Invalid bridge event: ${event}`);
        return false;
    }
    // ... rest of logic
}, [isDesktop]);

2. Fix accessibility in brand-short-logo.tsx:

<button 
    onClick={handleLogoClick} 
    style={{ cursor: 'pointer', border: 'none', background: 'none', padding: 0 }}
   
   
>
    <BrandDerivLogoCoralIcon width={24} height={24} />
</button>

3. Remove unnecessary fragment in account-actions.tsx:

const LoggedOutView = () => <LoginButtonV2 className='acc-info__button' />;

---

Recommendations

Before Merging:

1. ✅ Add dedicated tests for useMobileBridge hook
2. ✅ Fix the race condition in handleLogout
3. ✅ Add input validation for bridge events
4. ✅ Fix accessibility issue in BrandShortLogo

Post-Merge:

1. Add JSDoc documentation
2. Consider async fallback support
3. Improve error logging strategy
4. Add bridge message validation/signing if possible

---

Overall, this is a well-structured PR with good test coverage for the component integrations. The main concerns are the missing hook tests and the potential race condition in the logout flow. The bridge integration pattern is solid and follows React best practices.

[cloudflare-workers-and-pages]

Deploying derivatives-trader with    Cloudflare Pages

Latest commit:	01e06a9
Status:	✅  Deploy successful!
Preview URL:	https://8ab358a7.derivatives-trader.pages.dev
Branch Preview URL:	https://amam-add-native-support-brid.derivatives-trader.pages.dev

View logs