Skip to content

/ node Public

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

Jump to bottom

Body Mixin text() is supposed to return USVString #39804

Open
ronag opened this issue Aug 18, 2021 · 6 comments · May be fixed by #40351
Open

Body Mixin text() is supposed to return USVString #39804

ronag opened this issue Aug 18, 2021 · 6 comments · May be fixed by #40351
Labels
good first issue stream web streams

Comments

@ronag
Copy link
Member

@ronag ronag commented Aug 18, 2021

https://fetch.spec.whatwg.org/#body-mixin

@jasnell I think the Blob and consumer helper text() methods need to run through toUSVString which is used in the url module?
@ronag ronag changed the title Body Mixin Text is supposed to return USVString Body Mixin text() is supposed to return USVString Aug 18, 2021
@jasnell
Copy link
Member

@jasnell jasnell commented Aug 18, 2021

To be fully compliant, yes we likely need to.

@LobRockyl
Copy link

@LobRockyl LobRockyl commented Aug 23, 2021

Should I take this issue up?

@jimmywarting
Copy link

@jimmywarting jimmywarting commented Aug 27, 2021 •
edited

just notice util.toUSVString came out in 16.8, good for ppl writing polyfills... got a side question: dose TextDecoder.decode() return a USVString? cuz that is what i have been using in some .text() methods.... This will also remove BOM

I also wonder what the diffrent is between USVString and a "normal" string is and doing String(val) or ${val} to convert them
Maybe stream-consumers text(iterable) should resolve with a USVString also?

@WenheLI
Copy link

@WenheLI WenheLI commented Sep 1, 2021

Anyone currently working on this issue? If not, can I try to solve it?

@git-srinivas
Copy link

@git-srinivas git-srinivas commented Oct 3, 2021 •
edited

Hi @ronag I am working on this issue. I could see the Blob contains the text() method. what is consumer helper here? can you please point the file or provide some more information about it?

@Mesteery
Copy link
Member

@Mesteery Mesteery commented Oct 4, 2021

https://github.com/nodejs/node/blob/master/lib/stream/consumers.js

git-srinivas added a commit to git-srinivas/node that referenced this issue Oct 6, 2021
@git-srinivas
lib: body mixin text is to return usvstring
0a2fcb8 
Readable web stream returns  strings.
Where it supposed to return usvstring. This change will fix the issue

Fixes: nodejs#39804
Refs: https://fetch.spec.whatwg.org/#body-mixin
git-srinivas added a commit to git-srinivas/node that referenced this issue Oct 6, 2021
@git-srinivas
lib: added test case for consumers.js and blob.js
98dd270 
Added test cases for the code changes.

Fixes: nodejs#39804
Refs: https://fetch.spec.whatwg.org/#body-mixin
git-srinivas added a commit to git-srinivas/node that referenced this issue Oct 6, 2021
@git-srinivas
lib: lint errors fixed in consumers.js
0995716 
Fixes: nodejs#39804
Refs: https://fetch.spec.whatwg.org/#body-mixin
git-srinivas added a commit to git-srinivas/node that referenced this issue Oct 7, 2021
@git-srinivas
lib: lint errors fixed in consumers.js
18827d1 
Fixes: nodejs#39804
Refs: https://fetch.spec.whatwg.org/#body-mixin
@git-srinivas git-srinivas linked a pull request that will close this issue Oct 7, 2021
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue stream web streams
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

lib: body mixin text to return usvstring
7 participants
@jasnell @jimmywarting @ronag @git-srinivas @WenheLI @LobRockyl @Mesteery