-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Blazor Byte Array Interop Support #33015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
e538e9b
Blazorserver Byte Array Interop Support
TanayParikh 137a5f1
PR Feedback
TanayParikh e56bf6f
Merge branch 'main' into taparik/byteArrayDirectInterop
TanayParikh 66815f3
Merge branch 'main' into taparik/byteArrayDirectInterop
TanayParikh 18ade7c
PR Feedback
TanayParikh 4a006e6
Merge branch 'main' into taparik/byteArrayDirectInterop
TanayParikh 8293e92
PR Feedback
TanayParikh 99c2b2c
Byte Array Interop in WASM (#33104)
TanayParikh fc7d066
Fix CI
TanayParikh b533959
ByteArrayJsonConverterTest.cs
TanayParikh 1780543
Additional Tests
TanayParikh 72dae17
Add github issue
TanayParikh f74f93f
Update DefaultWebAssemblyJSRuntime.cs
TanayParikh d4fc26e
Merge branch 'main' into taparik/byteArrayDirectInterop
TanayParikh 108b9f2
Update InputFile.ts
TanayParikh 6285ba1
Update release files
TanayParikh 4c3edd6
PR Feedback
TanayParikh db57f3a
Update WarningSuppression
TanayParikh 514d320
E2E Tests
TanayParikh 11e2ceb
Merge branch 'main' into taparik/byteArrayDirectInterop
TanayParikh a66eac6
Update roundTripByteArrayAsyncFromJS tests to not use some ES features
TanayParikh f710bd0
Revert ByteArrayJsonConverter Impl
TanayParikh 02b2b75
PR Feedback
TanayParikh b6b2fdf
Merge branch 'main' into taparik/byteArrayDirectInterop
TanayParikh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,9 +101,9 @@ async function ensureArrayBufferReadyForSharedMemoryInterop(elem: InputElement, | |
getFileById(elem, fileId).arrayBuffer = arrayBuffer; | ||
} | ||
|
||
async function readFileData(elem: InputElement, fileId: number, startOffset: number, count: number): Promise<string> { | ||
async function readFileData(elem: InputElement, fileId: number, startOffset: number, count: number): Promise<Uint8Array> { | ||
const arrayBuffer = await getArrayBufferFromFileAsync(elem, fileId); | ||
return btoa(String.fromCharCode.apply(null, new Uint8Array(arrayBuffer, startOffset, count) as unknown as number[])); | ||
return new Uint8Array(arrayBuffer, startOffset, count); | ||
} | ||
Comment on lines
-104
to
107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Required change to our current |
||
|
||
export function getFileById(elem: InputElement, fileId: number): BrowserFile { | ||
|
@@ -134,4 +134,4 @@ function getArrayBufferFromFileAsync(elem: InputElement, fileId: number): Promis | |
} | ||
|
||
return file.readPromise; | ||
} | ||
} |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still doing
ToArray
in place of passing through theReadOnlySequence<byte>
.Concerns with using the
ReadOnlySequence
relate to the fact that it doesn't implicitly cast tobyte[]
so when we're examining method parameters we have a type mistmatch.Additionally, as Javier mentioned in the other PR we need to verify the lifecycle/ownership of the
ReadOnlySequence
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you'd need to change the receiving code (
ComponentHub.SupplyByteArray
) to accept aReadOnlySequence<byte>
instead of abyte[]
, so there wouldn't be a type mismatch.Agreed on verifying the ownership of the buffer before doing that though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, lets keep it as is for the time being and then we can do a quick check and change afterwards.