Set WebSocket.binaryType default value to 'blob'

[jeremymeng]

Summary:

Per WHATWG spec,

Each WebSocket object has an associated binary type, which is a BinaryType. Initially it must be "blob".

Fixes https://github.com/facebook/react-native/issues/37524

Changelog:

[General][Changed] - Change the default value of WebSocket.binaryType from undefined to blob to align with WHATWG spec.

Test Plan:

• Add a test to verify the default value of WebSocket.binaryType equals 'blob'.

[facebook-github-bot]

Hi @jeremymeng!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,733,444	-7,371
android	hermes	armeabi-v7a	8,044,647	-7,062
android	hermes	x86	9,223,552	-7,438
android	hermes	x86_64	9,075,555	-7,280
android	jsc	arm64-v8a	9,298,169	-5,006
android	jsc	armeabi-v7a	8,486,940	-4,729
android	jsc	x86	9,359,318	-5,125
android	jsc	x86_64	9,615,438	-4,944

Base commit: be8af22740997b4268b90746526e4e953e9ca5bf Branch: main

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[javache]

The code currently relies on a call to setBinaryType to register the right listeners with blob module. This will now break code that does

if (ws.binaryType != 'blob') {
   ws.binaryType = 'blob';
}

[jeremymeng]

@javache that's right. Thanks for pointing it out. Maybe I could change to call the setter in the constructor. Still currently BlobModule is not required. With the change it will become required. Any suggestion?

[Pranav-yadav]

@jeremymeng could you please link the issue to this PR? By mentioniting below in the PR description:

Fixes #37524

[javache]

You could make the binaryType getter lazy-init the binaryType_ field, and add the listener only when the binaryType field is accessed. Feels hacky though, are there any better ways to know the binary data is required?

[jeremymeng]

You could make the binaryType getter lazy-init the binaryType_ field, and add the listener only when the binaryType field is accessed. Feels hacky though, are there any better ways to know the binary data is required?

my motivation is to avoid having to create a WebSocket wrapper just to set binaryType to 'blob' since undefined doesn't work for our SDK. So lazy init wouldn't help us.

Always requiring BlobModule doesn't sound reasonable either. This probably needs more discussion.

[jeremymeng]

forgot the link to wrapper https://github.com/Azure/azure-sdk-for-js/blob/0bf3e3056a39f2477c9f90be270f8f132b133fae/samples/frameworks/react-native-expo/ts/messaging/src/wsWrapper.ts#L7

[jeremymeng]

How about calling the setter in constructor to set 'blob' if BlobManager is available? How could I make the mock to work?

[jeremymeng]

@javache thanks for the through review! This is beyond my limited knowledge of react-native. Maintainers please feel free to take over or close this PR.

[github-actions[bot]]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[github-actions[bot]]

This PR was closed because it has been stalled for 7 days with no activity.