FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

buffer: add Blob.prototype.stream method and other cleanups by jasnell · Pull Request #39693 · nodejs/node · GitHub

/ node Public

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

Closed
jasnell wants to merge 4 commits into
nodejs:masterfrom
jasnell:blob2
ConversationCommits (4)ChecksFiles changed
Closed

buffer: add Blob.prototype.stream method and other cleanups#39693
jasnell wants to merge 4 commits into
nodejs:masterfrom
jasnell:blob2

Conversation

jasnell commented Aug 7, 2021

Copy link
Copy Markdown
Member

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
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>
test: enable blob.prototype.stream tests
51fcc3e 
Signed-off-by: James M Snell <jasnell@gmail.com>
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 reviewed Aug 7, 2021
View reviewed changes
Comment thread lib/internal/blob.js Outdated
Comment thread lib/internal/url.js Outdated
Comment thread lib/internal/url.js Outdated
Comment thread lib/internal/url.js Outdated
Comment thread lib/internal/url.js Outdated
Comment thread lib/internal/blob.js Outdated
Comment thread lib/internal/blob.js Outdated
Comment thread lib/internal/blob.js Outdated
Comment thread lib/internal/blob.js Outdated
addaleax requested changes Aug 7, 2021
View reviewed changes
Comment thread doc/api/url.md Outdated
Comment thread lib/internal/blob.js Outdated
Comment thread lib/internal/url.js Outdated
Comment thread lib/internal/url.js Outdated
Comment thread src/env.h Outdated
Comment thread src/node_blob.cc Outdated
Comment thread src/node_blob.cc Outdated
Comment thread src/node_blob.cc Outdated
Comment thread src/node_blob.cc Outdated
bmeck reviewed Aug 7, 2021
View reviewed changes
Comment thread doc/api/buffer.md Outdated
jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Aug 7, 2021
jasnell requested review from addaleax, aduh95 and bmeck August 7, 2021 15:27

This comment has been minimized.

Sign in to view

bmeck commented Aug 7, 2021

Copy link
Copy Markdown
Member

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 commented Aug 7, 2021

Copy link
Copy Markdown
Member Author

@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.

This comment has been minimized.

Sign in to view

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

Copy link
Copy Markdown
Collaborator

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

jasnell added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Aug 8, 2021
jasnell force-pushed the blob2 branch 2 times, most recently from 3ad7f65 to 0ae84b1 Compare August 8, 2021 18:26
jasnell mentioned this pull request Aug 8, 2021
Closed
aduh95 reviewed Aug 8, 2021
View reviewed changes
Comment thread lib/internal/blob.js Outdated
Comment thread lib/internal/blob.js Outdated
Comment thread lib/internal/blob.js Outdated
Comment thread lib/internal/url.js Outdated

targos commented Aug 9, 2021

Copy link
Copy Markdown
Member

Fixes: #39537

jimmywarting reviewed Aug 9, 2021
View reviewed changes
Comment thread doc/api/url.md Outdated
jimmywarting reviewed Aug 9, 2021
View reviewed changes
Comment thread lib/internal/blob.js Outdated
jimmywarting reviewed Aug 9, 2021
View reviewed changes
Comment thread lib/internal/blob.js Outdated
jimmywarting reviewed Aug 9, 2021
View reviewed changes
Comment thread lib/internal/blob.js Outdated
url,buffer: implement URL.createObjectURL
0998e6e 
Signed-off-by: James M Snell <jasnell@gmail.com>
jasnell removed the needs-ci PRs that need a full CI run. label Aug 9, 2021

This comment has been minimized.

Sign in to view

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

Copy link
Copy Markdown
Collaborator

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

jimmywarting mentioned this pull request Aug 10, 2021
Closed
5 tasks

This comment has been minimized.

Sign in to view

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

Copy link
Copy Markdown
Collaborator

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

jasnell commented Aug 12, 2021

Copy link
Copy Markdown
Member Author

Landed in 87d6fd7...31d1d0c

jasnell closed this Aug 12, 2021
jasnell added a commit that referenced this pull request Aug 12, 2021
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
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
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
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
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
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 mentioned this pull request Aug 16, 2021
Merged
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

aduh95 aduh95 left review comments
Qard Qard approved these changes
addaleax addaleax approved these changes
benjamingr benjamingr approved these changes
+2 more reviewers
bmeck bmeck approved these changes
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 outstanding review comments, 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

Back | FazBrowse Home | New Git URL