Skip to content

fix(runtime): deliver a posted undefined as undefined, not null - #477

Open
adrian-niculescu wants to merge 1 commit into
NativeScript:mainfrom
adrian-niculescu:fix/message-event-undefined-data
Open

adrian-niculescu wants to merge 1 commit into
NativeScript:mainfrom
adrian-niculescu:fix/message-event-undefined-data

Conversation

@adrian-niculescu

@adrian-niculescu adrian-niculescu commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

worker.postMessage(undefined) arrives with event.data === null. The same happens on a MessagePort, on a BroadcastChannel and on the node:worker_threads parentPort. Browsers deliver undefined; null, false, 0 and "" already arrive unchanged.

Every delivery path built its event with new MessageEvent(type, { data, ports }). The constructor is right to turn an absent or undefined data into null, that is what Web IDL does for its init dictionary, but a delivered message is not built from a dictionary: it carries whatever the payload deserialized to. message-event.js now exports an internal createMessageEvent(type, data, ports) that stores the payload as given, and the four delivery sites use it. The public constructor is unchanged.

The two native messageerror paths relied on that defaulting to turn "nothing to carry" into null, so they now pass null themselves and that event keeps its default data.

New specs in MessagingTests.js cover each channel kind and fail without the fix.

Summary by CodeRabbit

  • Bug Fixes

    • messageerror events now use null for unavailable or invalid message data instead of undefined.
    • Messaging channels and workers preserve falsy payloads, including explicit undefined, without altering their values.
    • Message events continue to default their data to null when appropriate.
  • Tests

    • Added coverage for message payload handling across message ports, broadcast channels, browser workers, and worker threads.

Every delivery path built its MessageEvent through the public constructor, whose init dictionary turns an undefined data into null. Delivery now goes through an internal factory that stores the payload as given, and the native messageerror paths pass null so that event keeps its default data.
@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 8eadd574-6118-400e-997a-f3d638e7610e

📥 Commits

Reviewing files that changed from the base of the PR and between 6221ca8 and b7f17b5.

📒 Files selected for processing (10)
  • NativeScript/runtime/Messaging.cpp
  • NativeScript/runtime/Worker.mm
  • NativeScript/runtime/js/broadcast-channel.js
  • NativeScript/runtime/js/message-channel.js
  • NativeScript/runtime/js/message-event.js
  • NativeScript/runtime/js/node-worker-threads.js
  • NativeScript/runtime/js/worker-events.js
  • TestRunner/app/tests/MessagingTests.js
  • TestRunner/app/tests/messaging/describeDataWorker.js
  • TestRunner/app/tests/messaging/parentPortDescribeWorker.js

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The change adds a message-event factory that preserves undefined payloads, updates messaging relays to use it, normalizes deserialization-error data to null, and adds cross-runtime regression tests for falsy values.

Changes

Messaging data preservation

Layer / File(s) Summary
Message event factory
NativeScript/runtime/js/message-event.js
Adds and exports createMessageEvent, which preserves undefined data while retaining normal MessageEvent constructor defaults.
Message delivery integration
NativeScript/runtime/js/broadcast-channel.js, NativeScript/runtime/js/message-channel.js, NativeScript/runtime/js/worker-events.js, NativeScript/runtime/js/node-worker-threads.js
Message delivery paths lazily load and call the factory with event type, data, and ports.
Deserialization behavior and validation
NativeScript/runtime/Messaging.cpp, NativeScript/runtime/Worker.mm, TestRunner/app/tests/MessagingTests.js, TestRunner/app/tests/messaging/*
Deserialization failures now use null message data. Tests cover undefined, null, false, zero, and empty-string payloads across messaging implementations.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: edusperoni

Merge Risk: ⚪ Minimal · up to b7f17

The messaging update preserves delivered undefined payloads while retaining constructor defaults and explicit messageerror null handling. No merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 9 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: preserving undefined when delivering a posted message instead of converting it to null.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 9 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI

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.

❤️ Share

A rabbit carried messages light,
Undefined stayed undefined in flight.
Null marked errors where values fell,
Ports and workers rang the bell.
Tests watched each payload hop,
And every tiny value did not stop.

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant