Skip to content

Conversation

@achingbrain
Copy link
Contributor

Stores local copies of the id, label and protocol fields for RTCDataChannel polyfill objects.

This is because libdatachannel throws if these fields are accessed on a DataChannel after it's been closed, but browsers allow access so brings the polyfill in line with other implementations.

Stores local copies of the id, label and protocol fields for
RTCDataChannel polyfill objects.

This is because libdatachannel throws if these fields are accessed
on a DataChannel after it's been closed, but browsers allow access
so brings the polyfill in line with other implementations.
@ThaUnknown
Copy link
Contributor

it's one of the things i fixed in ##324 ;-;

@mertushka
Copy link
Contributor

@murat-dogan Can you merge this please?

@murat-dogan
Copy link
Owner

Hello @achingbrain ,

I am not sure about the tests.
I mean we can not write all tests for polyfills, and actually, we don't need it.
We want to pass WPT tests as much as we can.

I think just a basic polyfill test will be enough.

@murat-dogan murat-dogan merged commit 91a423e into murat-dogan:master May 9, 2025
1 check passed
@mertushka
Copy link
Contributor

@achingbrain @murat-dogan After applying the readyState fix #359 and run the build workflows, this test "it can access datachannel informational fields after closing" is failing with a timeout, but only on macOS (x64). Interestingly, the same test passes without issues on the Linux builds. It's a bit puzzling.

You can see the Build - Mac x64 failure here:
https://github.com/mertushka/node-datachannel/actions/runs/15748038280/job/44388296991#step:6:26

@mertushka
Copy link
Contributor

After re-running the workflow without any changes, the test passed on macOS (x64). I guess it was something on the GitHub Runner's side? Pretty weird.

https://github.com/mertushka/node-datachannel/actions/workflows/build-mac-x64.yml

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.

4 participants