Skip to content
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

Blob constructor error message when passing a string is misleading #38856

Closed
Ethan-Arrowood opened this issue May 30, 2021 · 7 comments · Fixed by #42338
Closed

Blob constructor error message when passing a string is misleading #38856

Ethan-Arrowood opened this issue May 30, 2021 · 7 comments · Fixed by #42338
Labels
buffer Issues and PRs related to the buffer subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core.

Comments

@Ethan-Arrowood
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Working with the new Blob api. Cannot instantiate using a string though I don't see why not.

new Blob('nodejs')

// -> The "sources" argument must be an instance of Iterable. Received type string ('nodejs')

Describe the solution you'd like

Maybe I'm misunderstanding, but I think strings to implement [Symbol.iterator]; does that not make it an instance of Iterable?

Describe alternatives you've considered

Plenty of work arounds - think this would make for a good addition.

Happy to contribute this feature too

@Ethan-Arrowood
Copy link
Contributor Author

Oh looks like this is intentionally not supported

typeof sources === 'string') {

@Ethan-Arrowood
Copy link
Contributor Author

Can we add a reason why to the docs?

@bl-ue
Copy link
Contributor

bl-ue commented May 30, 2021

#36811

@aduh95
Copy link
Contributor

aduh95 commented May 30, 2021

Here's my interpretation of why a string is not accepted: The specs dictates that the param must be a sequence<BlobPart>. It's not clear to me if the string is interpreted as a USVString (which doesn't seem to be a sequence in that specification), or a DOMString (which is defined as a sequence<unsigned short> and unsigned short is not a BlobPart), but either way new Blob('string') is meant to fail.

Now I agree the error message is misleading, for reference here's the error messages thrown by some browsers:

  • Chromium: TypeError: Failed to construct 'Blob': The provided value cannot be converted to a sequence.
  • Firefox: TypeError: Blob constructor: Argument 1 can't be converted to a sequence.
  • Safari: TypeError: Value is not a sequence

@aduh95 aduh95 added buffer Issues and PRs related to the buffer subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels May 30, 2021
@jasnell
Copy link
Member

jasnell commented May 31, 2021

This works just fine, however: new Blob(['hello'])

@jasnell
Copy link
Member

jasnell commented May 31, 2021

@Ethan-Arrowood ... just as a heads up since you're using Blob... sometime in the next two months as time allows, I will be working on Blob supporting async data sources. It won't impact the API usage at all, but figured it's worthwhile to give a heads up.

@Ethan-Arrowood
Copy link
Contributor Author

Yes I saw that, very excited for it. I'm using this in undici-fetch and it'll be a nice perf boost if blob can support the async iterator sources directly 😄

@aduh95 aduh95 changed the title Support string as source for Blob Blob constructor error message when passing a string is misleading Jun 1, 2021
nodejs-github-bot pushed a commit that referenced this issue Apr 4, 2022
resolve: #38856

PR-URL: #42338
Fixes: #38856
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit to juanarbol/node that referenced this issue Apr 5, 2022
resolve: nodejs#38856

PR-URL: nodejs#42338
Fixes: nodejs#38856
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this issue Apr 6, 2022
resolve: #38856

PR-URL: #42338
Fixes: #38856
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this issue Apr 6, 2022
resolve: #38856

PR-URL: #42338
Fixes: #38856
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
xtx1130 pushed a commit to xtx1130/node that referenced this issue Apr 25, 2022
resolve: nodejs#38856

PR-URL: nodejs#42338
Fixes: nodejs#38856
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this issue May 31, 2022
resolve: #38856

PR-URL: #42338
Fixes: #38856
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this issue Jun 27, 2022
resolve: #38856

PR-URL: #42338
Fixes: #38856
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jul 11, 2022
resolve: #38856

PR-URL: #42338
Fixes: #38856
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jul 31, 2022
resolve: #38856

PR-URL: #42338
Fixes: #38856
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
resolve: nodejs/node#38856

PR-URL: nodejs/node#42338
Fixes: nodejs/node#38856
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants