fix(runtime): deliver a posted undefined as undefined, not null - #477
adrian-niculescu wants to merge 1 commit into
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe 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. ChangesMessaging data preservation
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to 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)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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.)
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. A rabbit carried messages light, Comment |
worker.postMessage(undefined)arrives withevent.data === null. The same happens on aMessagePort, on aBroadcastChanneland on thenode:worker_threadsparentPort. Browsers deliverundefined;null,false,0and""already arrive unchanged.Every delivery path built its event with
new MessageEvent(type, { data, ports }). The constructor is right to turn an absent or undefineddataintonull, 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.jsnow exports an internalcreateMessageEvent(type, data, ports)that stores the payload as given, and the four delivery sites use it. The public constructor is unchanged.The two native
messageerrorpaths relied on that defaulting to turn "nothing to carry" intonull, so they now passnullthemselves and that event keeps its defaultdata.New specs in
MessagingTests.jscover each channel kind and fail without the fix.Summary by CodeRabbit
Bug Fixes
messageerrorevents now usenullfor unavailable or invalid message data instead ofundefined.undefined, without altering their values.nullwhen appropriate.Tests