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.

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

feat(ext/fetch): Request.bytes() and Response.bytes() #23823

Merged
merged 14 commits into from
May 23, 2024
Merged

feat(ext/fetch): Request.bytes() and Response.bytes() #23823

merged 14 commits into from
May 23, 2024

Conversation

iuioiua
Copy link
Collaborator

@iuioiua iuioiua commented May 15, 2024

Closes #23790

iuioiua
iuioiua commented May 15, 2024
View reviewed changes
cli/tsc/dts/lib.dom.d.ts Show resolved Hide resolved
cli/tsc/dts/lib.webworker.d.ts Show resolved Hide resolved
ext/fetch/22_body.js Show resolved Hide resolved
@iuioiua
Copy link
Collaborator Author

iuioiua commented May 15, 2024

I updated the wpt git submodule but idk why there's a merge conflict. How do I fix I it?

@bartlomieju bartlomieju added this to the 1.44 milestone May 15, 2024
@iuioiua iuioiua requested a review from bartlomieju May 15, 2024 12:42
@iuioiua iuioiua marked this pull request as draft May 16, 2024 22:39
@iuioiua iuioiua marked this pull request as ready for review May 16, 2024 22:41
bartlomieju
bartlomieju reviewed May 22, 2024
View reviewed changes
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iuioiua please rebase

tests/testdata/fmt/with_config/subdir/a.ts Show resolved Hide resolved
tests/unit/fetch_test.ts Show resolved Hide resolved
@iuioiua iuioiua enabled auto-merge (squash) May 22, 2024 22:29
bartlomieju
bartlomieju reviewed May 22, 2024
View reviewed changes
tests/wpt/runner/expectation.json
Comment on lines +7156 to +7161
"response-blob-realm.any.html": [
"realm of the Uint8Array from Response bytes()"
],
"response-blob-realm.any.worker.html": [
"realm of the Uint8Array from Response bytes()"
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look funky. What are these tests doing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/web-platform-tests/wpt/blob/master/fetch/api/response/response-blob-realm.any.js

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine since we don't support iframe.

bartlomieju
bartlomieju requested changes May 22, 2024
View reviewed changes
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Temporarily blocking due to unexpected change in WPT

bartlomieju
bartlomieju approved these changes May 22, 2024
View reviewed changes
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM #23823 (comment)

@bartlomieju bartlomieju mentioned this pull request May 22, 2024
Merged
@iuioiua iuioiua merged commit 8a636d0 into denoland:main May 23, 2024
17 checks passed
@iuioiua iuioiua deleted the body-bytes branch May 23, 2024 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsherret dsherret dsherret left review comments

@bartlomieju bartlomieju bartlomieju approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.44
Development

Successfully merging this pull request may close these issues.

Implement Request/Response .bytes() method
3 participants
@iuioiua @dsherret @bartlomieju