FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(runtime): deliver a posted undefined as undefined, not null by adrian-niculescu · Pull Request #2063 · NativeScript/android · GitHub

  1. NativeScript
  2. android

Repository navigation

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

Open
adrian-niculescu wants to merge 1 commit into
NativeScript:feat/worker-threadsfrom
adrian-niculescu:fix/message-event-undefined-data
ConversationCommits (1)ChecksFiles changed
Open

fix(runtime): deliver a posted undefined as undefined, not null#2063
adrian-niculescu wants to merge 1 commit into
NativeScript:feat/worker-threadsfrom
adrian-niculescu:fix/message-event-undefined-data

Conversation

adrian-niculescu commented Oct 6, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

postMessage(undefined) arrives as null everywhere a message is delivered: a MessagePort, a BroadcastChannel, a Worker and its global scope, and a node:worker_threads parentPort. Node and browsers deliver undefined.

Every delivery path builds its event with new MessageEvent(type, { data, ports }), and the constructor's init dictionary turns an undefined data into null, as Web IDL requires. Delivery now goes through an internal createMessageEvent that stores the payload as given, the same fix as NativeScript/ios#477. The native messageerror paths pass null instead of undefined, so that event keeps its default. A worker's messageerror now carries the deserialization failure as its data, as a port's already does and as NativeScript/ios#489 does on iOS.

Stacked on #2043. The new specs in testMessaging.js fail on that branch and pass with this change, and the full device suite passes.

Summary by CodeRabbit

  • Bug Fixes
    • Message delivery now preserves undefined values across message ports, broadcast channels, and workers instead of converting them to null.
    • Message events consistently include a data property, including when its value is undefined.
    • Failed message deserialization now reports the caught error, or null when no error is available.

fix(runtime): deliver a posted undefined as undefined, not null
4f06805 
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. The native messageerror paths pass null so that event keeps its default data, and a worker's messageerror carries the deserialization failure as its data, as a port's already does.

coderabbitai Bot commented Oct 6, 2026 •
edited
Loading

Copy link
Copy Markdown

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: 093ce25c-a1ad-4573-b6ad-f125002998d4
📥 Commits

Reviewing files that changed from the base of the PR and between 42a8bcf and 4f06805.

📒 Files selected for processing (10)
  • test-app/app/src/main/assets/app/tests/messaging/describeDataWorker.js
  • test-app/app/src/main/assets/app/tests/messaging/parentPortDescribeWorker.js
  • test-app/app/src/main/assets/app/tests/testMessaging.js
  • test-app/runtime/src/main/cpp/Messaging.cpp
  • test-app/runtime/src/main/cpp/WorkerEvents.cpp
  • test-app/runtime/src/main/cpp/js/broadcast-channel.js
  • test-app/runtime/src/main/cpp/js/message-channel.js
  • test-app/runtime/src/main/cpp/js/message-event.js
  • test-app/runtime/src/main/cpp/js/node-worker-threads.js
  • test-app/runtime/src/main/cpp/js/worker-events.js

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Walkthrough

Message delivery now uses a factory to preserve delivered undefined values in message events. Native deserialization failures use null when no exception value is available. Regression tests cover messaging channels, workers, and MessageEvent defaults.

Changes

Message data handling

Layer / File(s) Summary
Create and relay message events
test-app/runtime/src/main/cpp/js/message-event.js, test-app/runtime/src/main/cpp/js/message-channel.js, test-app/runtime/src/main/cpp/js/broadcast-channel.js, test-app/runtime/src/main/cpp/js/node-worker-threads.js, test-app/runtime/src/main/cpp/js/worker-events.js		 The exported createMessageEvent factory sets event data directly. Message relays use it to create events with the delivered type, data, and ports.
Set messageerror data
test-app/runtime/src/main/cpp/Messaging.cpp, test-app/runtime/src/main/cpp/WorkerEvents.cpp		 Deserialization failures use the caught exception as messageerror data, or null if no exception was caught or the value is undefined.
Verify message payload behavior
test-app/app/src/main/assets/app/tests/testMessaging.js, test-app/app/src/main/assets/app/tests/messaging/*		 Regression tests check payload delivery through MessagePort, BroadcastChannel, browser workers, and node:worker_threads. They also check MessageEvent data defaults.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 4f068

The change makes a posted undefined arrive as undefined instead of null across message ports, broadcast channels and workers. Regression tests cover these paths. No merge-blocking risk was found.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 10 files. 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 summarizes the main change: preserving posted undefined values during runtime message delivery.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1 🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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 sends a message through the night,
“Undefined,” it says, and keeps it right.
Ports and channels pass the payload on,
Workers answer with the same data drawn.
Then hops away beneath the moon’s soft light.

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

adrian-niculescu marked this pull request as ready for review October 6, 2026 19:17

adrian-niculescu commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

coderabbitai Bot commented Oct 6, 2026 •
edited
Loading

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

adrian-niculescu mentioned this pull request Oct 6, 2026
Merged

This branch has not been deployed

No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

Back | FazBrowse Home | New Git URL