Skip to content

buffer: add Blob.prototype.stream method and other cleanups #39693

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

buffer: add Blob.prototype.stream method and other cleanups #39693

wants to merge 4 commits into from

Conversation

@jasnell
Copy link
Member

@jasnell jasnell commented Aug 7, 2021

There are a couple of changes here:

  1. It cleans up the Blob implementation to better match the spec.
  2. It adds the Blob.prototype.stream() method to get a ReadableStream from a Blob
  3. It enables the relevant web platform tests for stream()
  4. It adds an experimental implementation of URL.createObjectURL(), URL.revokeObjectURL(), and buffer.resolveObjectURL(). The URL.createObjectURL() and URL.revokeObjectURL() are standard Web Platform API.

/cc @bmeck

jasnell added 2 commits August 6, 2021 17:31
@jasnell
buffer: add Blob.prototype.stream method and other cleanups
7eb128a 
Adds the `stream()` method to get a `ReadableStream` for the `Blob`.
Also makes some other improvements to get the implementation closer
to the API standard definition.

Signed-off-by: James M Snell <jasnell@gmail.com>
@jasnell
test: enable blob.prototype.stream tests
51fcc3e 
Signed-off-by: James M Snell <jasnell@gmail.com>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 7, 2021
aduh95
aduh95 reviewed Aug 7, 2021
View reviewed changes
addaleax
addaleax requested changes Aug 7, 2021
View reviewed changes
bmeck
bmeck reviewed Aug 7, 2021
View reviewed changes
@jasnell jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Aug 7, 2021
@jasnell jasnell requested review from addaleax, aduh95 and bmeck August 7, 2021 15:27
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@bmeck
Copy link
Member

bmeck commented Aug 7, 2021

I know this is incremental/experimental, but can we add ESM integration by adding integration with:

This also could be done as a follow on PR.

@jasnell
Copy link
Member Author

jasnell commented Aug 7, 2021

@bmeck:

This also could be done as a follow on PR.

Yeah, definitely want to see that work done but let's definitely separate it out.

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 8, 2021 •
edited by jasnell
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/39513/ 💚

@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 8, 2021
@jasnell jasnell force-pushed the blob2 branch 2 times, most recently from 3ad7f65 to 0ae84b1 Compare August 8, 2021 18:26
@jasnell jasnell mentioned this pull request Aug 8, 2021
Closed
aduh95
aduh95 reviewed Aug 8, 2021
View reviewed changes
@targos
Copy link
Member

targos commented Aug 9, 2021

Fixes: #39537

jimmywarting
jimmywarting reviewed Aug 9, 2021
View reviewed changes
jimmywarting
jimmywarting reviewed Aug 9, 2021
View reviewed changes
jimmywarting
jimmywarting reviewed Aug 9, 2021
View reviewed changes
jimmywarting
jimmywarting reviewed Aug 9, 2021
View reviewed changes
@jasnell
url,buffer: implement URL.createObjectURL
0998e6e 
Signed-off-by: James M Snell <jasnell@gmail.com>
@jasnell jasnell removed the needs-ci PRs that need a full CI run. label Aug 9, 2021
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 9, 2021 •
edited by jasnell
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/39524/ 💚

@jimmywarting jimmywarting mentioned this pull request Aug 10, 2021
Closed
5 tasks
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 11, 2021 •
edited by jasnell
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/39543/ 💚

@jasnell
Copy link
Member Author

jasnell commented Aug 12, 2021

Landed in 87d6fd7...31d1d0c

@jasnell jasnell closed this Aug 12, 2021
jasnell added a commit that referenced this pull request Aug 12, 2021
@jasnell
buffer: add Blob.prototype.stream method and other cleanups
0bb2605 
Adds the `stream()` method to get a `ReadableStream` for the `Blob`.
Also makes some other improvements to get the implementation closer
to the API standard definition.

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #39693
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
jasnell added a commit that referenced this pull request Aug 12, 2021
@jasnell
test: enable blob.prototype.stream tests
793c08b 
Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #39693
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
jasnell added a commit that referenced this pull request Aug 12, 2021
@jasnell
url,buffer: implement URL.createObjectURL
31d1d0c 
Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #39693
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
danielleadams pushed a commit that referenced this pull request Aug 16, 2021
@jasnell @danielleadams
buffer: add Blob.prototype.stream method and other cleanups
091a579 
Adds the `stream()` method to get a `ReadableStream` for the `Blob`.
Also makes some other improvements to get the implementation closer
to the API standard definition.

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #39693
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
danielleadams pushed a commit that referenced this pull request Aug 16, 2021
@jasnell @danielleadams
test: enable blob.prototype.stream tests
53cf53c 
Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #39693
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
danielleadams pushed a commit that referenced this pull request Aug 16, 2021
@jasnell @danielleadams
url,buffer: implement URL.createObjectURL
37dda19 
Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #39693
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
@danielleadams danielleadams mentioned this pull request Aug 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95 aduh95 aduh95 left review comments

@Qard Qard Qard approved these changes

@addaleax addaleax addaleax approved these changes

@benjamingr benjamingr benjamingr approved these changes

+2 more reviewers

@bmeck bmeck bmeck approved these changes

@jimmywarting jimmywarting jimmywarting left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@jasnell @nodejs-github-bot @bmeck @targos @Qard @addaleax @jimmywarting @benjamingr @aduh95