Skip to content

worker: populate BroadcastChannel MessageEvent source#64334

Open
SudhansuBandha wants to merge 1 commit into
nodejs:mainfrom
SudhansuBandha:update-broadcast-channel-source
Open

worker: populate BroadcastChannel MessageEvent source#64334
SudhansuBandha wants to merge 1 commit into
nodejs:mainfrom
SudhansuBandha:update-broadcast-channel-source

Conversation

@SudhansuBandha

@SudhansuBandha SudhansuBandha commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Populate the MessageEvent source property with the sender's worker thread ID for BroadcastChannel messages originating from worker threads.

Fixes: #59053

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. worker Issues and PRs related to Worker support. labels Jul 7, 2026
@SudhansuBandha SudhansuBandha mentioned this pull request Jul 7, 2026
Open
@SudhansuBandha
worker: populate BroadcastChannel MessageEvent source
98aaa6a 
Populate the MessageEvent source property with the sender's
worker thread ID for BroadcastChannel messages
originating from worker threads.

Fixes: nodejs#59053
Signed-off-by: Sudhansu Bandha <bandhasudhansu@gmail.com>
@SudhansuBandha SudhansuBandha force-pushed the update-broadcast-channel-source branch from 8a9de03 to 98aaa6a Compare July 7, 2026 02:37
@jdmarshall

jdmarshall commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown

In the MDN documentation, https://developer.mozilla.org/en-US/docs/Web/API/MessageEvent/source

The source read-only property of the MessageEvent interface is a MessageEventSource (which can be a WindowProxy, MessagePort, or ServiceWorker object) representing the message emitter.

The threadId might be sufficient to work with the new API you folks were working on but I'm wondering if there's a technical limitation that makes diverging from the web api an absolute necessity? Ideally this would be the Worker Object, since one could immediately invoke postMessage() for any rendezvous coordination necessitated by the broadcast. With that in mind a MessagePort would also work if that's available, but I'm guessing that is not...

jasnell
jasnell requested changes Jul 7, 2026
View reviewed changes

@jasnell jasnell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change making this a semver-major. It will need doc updates as well. I think we should instead try to find a way of doing this so that it's not a breaking change

@SudhansuBandha

SudhansuBandha commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review, @jasnell. I agree that changing MessageEvent.source makes this a semver-major change. Do you have any particular approach in mind for exposing the sender information while preserving the existing semantics of MessageEvent.source? I'd like to make sure I'm exploring all feasible directions that best align with Node's API design.

@jdmarshall

jdmarshall commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown

@jasnell Source currently resolves to null. Why would this a breaking change when there is nobody in the forest to hear the tree fall?

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

Reviewers

@jasnell jasnell jasnell requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. worker Issues and PRs related to Worker support.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BroadcastChannel doesn't fill in useful MessageEvent fields like source, or ports

4 participants

@SudhansuBandha @jdmarshall @jasnell @nodejs-github-bot