src: rename IsAnyByteSource to IsAnyBufferSource #49346
Merged
Conversation
This file contains 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
The current name is somewhat confusing. There is an internal ByteSource class, which is entirely unrelated to what IsAnyByteSource() does, even though both exist in the crypto subsystem of the C++ code. ByteSource objects can also be constructed from strings, for example, for which IsAnyByteSource() returns false. Web IDL calls the types for which this function returns true BufferSource. This type is commonly used across Web Crypto, for example. Thus, rename the function to match the Web IDL naming. Because the function also appears to accept BufferSource objects backed by SharedArrayBuffer instances, the exact Web IDL name would be AllowSharedBufferSource, but that seems unnecessarily long, so I decided to stick with "any".
|
Review requested:
|
nodejs-github-bot
added
c++
joyeecheung
approved these changes
Aug 27, 2023
anonrig
approved these changes
Aug 28, 2023
panva
added
author ready
panva
approved these changes
Aug 28, 2023
github-actions
bot
removed
the
request-ci
This comment was marked as outdated.
This comment was marked as outdated.
RaisinTen
approved these changes
Aug 28, 2023
This comment was marked as outdated.
This comment was marked as outdated.
|
CI infra is broken, see nodejs/build#3475. |
21 tasks
tniessen
added
the
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in 43704dd |
This was referenced Aug 30, 2023
UlisesGascon
pushed a commit
that referenced
this pull request
Sep 10, 2023
The current name is somewhat confusing. There is an internal ByteSource class, which is entirely unrelated to what IsAnyByteSource() does, even though both exist in the crypto subsystem of the C++ code. ByteSource objects can also be constructed from strings, for example, for which IsAnyByteSource() returns false. Web IDL calls the types for which this function returns true BufferSource. This type is commonly used across Web Crypto, for example. Thus, rename the function to match the Web IDL naming. Because the function also appears to accept BufferSource objects backed by SharedArrayBuffer instances, the exact Web IDL name would be AllowSharedBufferSource, but that seems unnecessarily long, so I decided to stick with "any". PR-URL: #49346 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
Merged
alexfernandez
pushed a commit
to alexfernandez/node
that referenced
this pull request
Nov 1, 2023
The current name is somewhat confusing. There is an internal ByteSource class, which is entirely unrelated to what IsAnyByteSource() does, even though both exist in the crypto subsystem of the C++ code. ByteSource objects can also be constructed from strings, for example, for which IsAnyByteSource() returns false. Web IDL calls the types for which this function returns true BufferSource. This type is commonly used across Web Crypto, for example. Thus, rename the function to match the Web IDL naming. Because the function also appears to accept BufferSource objects backed by SharedArrayBuffer instances, the exact Web IDL name would be AllowSharedBufferSource, but that seems unnecessarily long, so I decided to stick with "any". PR-URL: nodejs#49346 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current name is somewhat confusing. There is an internal
ByteSourceclass, which is entirely unrelated to whatIsAnyByteSource()does, even though both exist in the crypto subsystem of the C++ code.ByteSourceobjects can also be constructed from strings, for example, for whichIsAnyByteSource()returns false.Web IDL calls the types for which this function returns true
BufferSource. This type is commonly used across Web Crypto, for example. Thus, rename the function to match the Web IDL naming.Because the function also appears to accept
BufferSourceobjects backed bySharedArrayBufferinstances, the exact Web IDL name would beAllowSharedBufferSource, but that seems unnecessarily long, so I decided to stick with "any".