[WC-3420] Pusher initial implementation

[r0b1n]

Pull request type

---

Description

[iobuhov]

Where is module code?

[github-actions]

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

---

What was reviewed

File	Change
src/Pusher.tsx	Main widget component
src/hooks/useFetchPusherConfig.ts	Fetches Pusher key/config from backend
src/hooks/usePusherListener.ts	Creates and initializes a PusherListener
src/hooks/usePusherSubscribe.ts	Manages subscription lifecycle
src/utils/PusherListener.ts	Pusher connection/subscription class
src/utils/fetchPusherConfig.ts	REST call to get Pusher key/cluster
src/utils/useMxObjectInfo.ts	Extracts GUID + entity name from ObjectItem
src/__tests__/Pusher.spec.tsx	Unit tests
src/Pusher.xml	Widget XML manifest
typings/PusherProps.d.ts	Generated typings
src/Pusher.editorConfig.ts	Studio Pro design-time config
src/Pusher.editorPreview.tsx	Studio Pro preview component
src/ui/Pusher.scss / PusherPreview.css	Styles
package.json, tsconfig.json, eslint.config.mjs	Package config
CHANGELOG.md	Initial entry present

Skipped (out of scope): pnpm-lock.yaml

---

Findings

🔶 Medium — Stale onEvent callback when channel/event name is unchanged

File: src/utils/PusherListener.ts line 62
Problem: subscribe() returns early when channelName and eventName match the current values, even if onEvent has changed. In Pusher.tsx, handleEvent is created with useCallback and closes over notifyEventAction. If the user reconfigures the action in Studio Pro without changing the channel or event, a new subscription object is produced (because handleEvent changed), usePusherSubscribe re-runs the effect, subscribe() is called again — but the early return means the channel stays bound to the old callback. The new action is silently never executed.
Fix:

subscribe(config: SubscriptionConfig): void {
    if (!this.pusher) { throw new Error("..."); }
    const channelName = this.buildChannelName(config.entityName, config.guid);

    if (channelName === this.currentChannelName && config.eventName === this.currentEventName) {
        // Channel/event unchanged — just rebind the handler in case it changed
        if (this.currentChannel) {
            this.currentChannel.unbind(this.currentEventName!);
            this.currentChannel.bind(config.eventName, config.onEvent);
        }
        return;
    }
    // … rest of the method
}

---

🔶 Medium — pusher:subscription_error handler accumulates on every re-subscription

File: src/utils/PusherListener.ts line 78
Problem: Each call to subscribe() binds a new anonymous function to pusher:subscription_error. unsubscribe() only calls unbind(this.currentEventName), so the error handler is never removed. On frequent re-subscriptions (e.g., entity changes) multiple error handlers fire for each error, and references accumulate.
Fix: Track the error handler and unbind it in unsubscribe():

private currentErrorHandler: ((error: unknown) => void) | null = null;

// in subscribe():
this.currentErrorHandler = (error: unknown) => { /* … */ };
this.currentChannel.bind("pusher:subscription_error", this.currentErrorHandler);

// in unsubscribe():
if (this.currentErrorHandler) {
    this.currentChannel.unbind("pusher:subscription_error", this.currentErrorHandler);
    this.currentErrorHandler = null;
}

---

🔶 Medium — Unit tests are a non-functional placeholder

File: src/__tests__/Pusher.spec.tsx line 1
Problem: The test file contains only expect(true).toBe(true). This new widget has real complexity: async config fetching, Pusher connection/subscription lifecycle, React hook cleanup, and ActionValue execution. None of it is covered. Per repo conventions, new features require unit tests.
Fix: At minimum cover:

• PusherListener: initialize(), subscribe(), unsubscribe(), destroy(), duplicate subscription early-return
• useFetchPusherConfig: loading state, successful fetch, network error, unmount abort
• Pusher component: renders without crash, action executed on event, no action when notifyEventAction is undefined

Use @mendix/widget-plugin-test-utils builders for Mendix data mocks; mock pusher-js and fetchPusherConfig.

---

⚠️ Low — Connection event handlers not removed in destroy()

File: src/utils/PusherListener.ts line 108
Note: destroy() calls this.pusher.disconnect() but never calls this.pusher.connection.unbind("error", ...) or this.pusher.connection.unbind("state_change", ...). Pusher's connection object is shared; unbound handlers keep references alive until GC. Add explicit unbinds before disconnect().

---

⚠️ Low — extractEntityName uses brittle private-symbol access and can throw during render

File: src/utils/useMxObjectInfo.ts line 27
Note: Object.getOwnPropertySymbols(object)[0] reads an undocumented private Mendix internal symbol. If the platform changes the internal structure, this silently returns undefined and then throws "Unable to extract entity name" — inside a useMemo call, which propagates as an unhandled render error. Consider wrapping in a try/catch that returns undefined instead of throwing, and logging a clear warning. If the Mendix SDK eventually exposes entity name directly, the TODO comment in Pusher.tsx should be tracked in a follow-up ticket.

---

⚠️ Low — console.debug left in production event handler

File: src/Pusher.tsx line 18
Note: console.debug("[Pusher] Event received:", data) will log every received Pusher event in production. Remove or gate behind a dev-only flag.

---

Positives

• useFetchPusherConfig correctly uses an active guard and AbortController together — both are needed and both are present.
• PusherListener separates connection management from subscription management cleanly; destroy() / unsubscribe() boundary is well-defined.
• CHANGELOG.md entry present for the initial release.
• CSRF token is correctly forwarded both to the key-fetch REST call and to the Pusher auth endpoint.
• useMxObjectInfo correctly guards against missing guid/entityName before constructing the subscription, preventing spurious subscriptions during load.

[github-actions]

AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

---

What was reviewed

File	Change
src/Pusher.tsx	Root widget component — subscription wiring
src/hooks/useFetchPusherConfig.ts	Async hook fetching Pusher credentials
src/hooks/usePusherListener.ts	Hook that owns the PusherListener lifecycle
src/hooks/usePusherSubscribe.ts	Hook that manages channel subscription
src/utils/PusherListener.ts	Class wrapping pusher-js — subscribe/unsubscribe/destroy
src/utils/fetchPusherConfig.ts	Fetches key+cluster from REST endpoint
src/utils/useMxObjectInfo.ts	Extracts GUID and entity name from ObjectItem
src/__tests__/Pusher.spec.tsx	Unit test file
src/Pusher.xml	Widget manifest
typings/PusherProps.d.ts	Generated prop types
src/Pusher.editorConfig.ts	Studio Pro structure preview
src/Pusher.editorPreview.tsx	Design-time preview component
src/ui/Pusher.scss	Widget styles
src/ui/PusherPreview.css	Design-time preview styles
src/package.xml	Package manifest
package.json	Package metadata and scripts
CHANGELOG.md	Changelog

Skipped (out of scope): pnpm-lock.yaml

CI check results could not be retrieved automatically.

---

Findings

🔶 Medium — Placeholder test file with no real coverage

File: src/__tests__/Pusher.spec.tsx lines 1–10
Problem: The entire test suite is a single expect(true).toBe(true) placeholder. There is no coverage for PusherListener (connection, subscribe, unsubscribe, destroy), useFetchPusherConfig (success, network error, abort, invalid JSON), usePusherListener (config loading state, initialization failure), event handling, or executeAction invocation. This is a new widget being added to production — shipping without tests risks regressions going undetected.
Fix: At minimum, add unit tests for the critical paths. Example skeleton:

import { render } from "@testing-library/react";
import { actionValue } from "@mendix/widget-plugin-test-utils";
import { PusherListener } from "../utils/PusherListener";

describe("PusherListener", () => {
    it("throws when subscribe is called before initialize", () => {
        const listener = new PusherListener({ key: "k", cluster: "eu", authEndpoint: "/auth", csrfToken: "t" });
        expect(() => listener.subscribe({ entityName: "E", guid: "1", eventName: "change", onEvent: jest.fn() }))
            .toThrow("not initialized");
    });

    it("calls onEvent when the bound event fires", () => {
        // mock pusher-js, assert bind + trigger
    });
});

describe("Pusher component", () => {
    it("renders without crashing when objectSource is unavailable", () => {
        // render with loading/unavailable mxObjectInfo
    });
});

---

🔶 Medium — Stale onEvent callback when action prop changes

File: src/utils/PusherListener.ts lines 62–64
Problem: subscribe() bails out early when channelName and eventName are unchanged:

if (channelName === this.currentChannelName && config.eventName === this.currentEventName) {
    return; // does NOT re-bind onEvent
}

In Pusher.tsx, handleEvent is recreated by useCallback whenever notifyEventAction changes. That triggers subscription (via useMemo) to be recreated, which triggers usePusherSubscribe's useEffect to call listener.subscribe(newSubscription) — but PusherListener.subscribe returns early, leaving the old onEvent callback bound to the channel. If a developer reconfigures the action in Studio Pro at runtime, the stale action is called instead of the new one.
Fix: Update the bound handler when only the callback changes, or unbind and rebind the event even when the channel stays the same:

// If same channel but callbacks may have changed, unbind the old event and rebind
if (channelName === this.currentChannelName && config.eventName === this.currentEventName) {
    this.currentChannel!.unbind(config.eventName);
    this.currentChannel!.bind(config.eventName, config.onEvent);
    return;
}

---

⚠️ Low — extractEntityName relies on undocumented internal Mendix Symbol

File: src/utils/useMxObjectInfo.ts lines 27–31
Note: Accessing the internal mxObject via Object.getOwnPropertySymbols(object)[0] is not part of the public Mendix Pluggable Widgets API. This is fragile — it will silently break if the runtime changes its internal object shape. Consider filing a request for an official API (object.getMetaObject?.() or similar), and at minimum add a comment explaining why this approach is used and what Mendix version it was verified against.

---

⚠️ Low — Empty PR description

Note: The PR body has no description, no selected PR type, and no testing instructions. For a brand-new widget integration (real-time Pusher connectivity), reviewers and QA need to know: what endpoints must exist on the backend, what Studio Pro configuration is required, and how to verify events are received end-to-end. Please fill in the template.

---

⚠️ Low — as any cast in useMxObjectInfo with open-ended TODO

File: src/Pusher.tsx line 13
Note: useMxObjectInfo(objectSource as any) is called because the generated typing has objectSource: ListValue but the implementation expects DynamicValue<ObjectItem>. The // TODO: fix typings when PWT updated comment has no ticket reference or timeline. This should either reference the PWT issue being tracked or be explained in a comment so future maintainers understand the mismatch.

---

Positives

• useFetchPusherConfig correctly implements the active guard and AbortController cleanup pattern for async effects — this matches the repo standard exactly.
• PusherListener.destroy() unbinds connection listeners before disconnecting, avoiding potential memory leaks on widget unmount.
• fetchPusherConfig validates the response fields (key, cluster) before returning — prevents silent misconfiguration.
• CHANGELOG entry present for the initial release with the correct [Unreleased] format.
• handleEvent and handleError are stable via useCallback, and subscription is memoized — dependency arrays are correct throughout.

[iobuhov]

I don't think we need active here. We can use ctrl.signal.aborted flag

[iobuhov]

I think usePusherListener and useFetchPusherConfig should be merge into single factory. They have dependency that seems separated just for seek of hooks, don't think it adds lot of value. useFetchPusherConfig don't have any args. We can create one clean factory that fetches config, init the instance and returns it wrapped in promise. This way whole factory can be passed to useState and we have ether instance or null. This way we have less coupling with react.

[iobuhov]

I think this code belongs to constructor. Init just adds extra step for no reason.

[iobuhov]

If we move init code to constructor this prop can be set to not-nullable

[iobuhov]

I think this should be a getter

[iobuhov]

Are we going to keep this any?

[iobuhov]

NIT: I would extract this block to small function for better testing