Skip to content

fix: implement fetch per spec#928

Merged
ronag merged 56 commits intomainfrom
node-fetch
Aug 12, 2021
Merged

fix: implement fetch per spec#928
ronag merged 56 commits intomainfrom
node-fetch

Conversation

@ronag
Copy link
Copy Markdown
Member

@ronag ronag commented Aug 5, 2021

No description provided.

@ronag ronag requested a review from mcollina August 5, 2021 06:28
@ronag

This comment has been minimized.

@ronag

This comment has been minimized.

Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

The node-fetch copyright should be retained somewhere.

@ronag
Copy link
Copy Markdown
Member Author

ronag commented Aug 5, 2021

The node-fetch copyright should be retained somewhere.

#928 (comment)

Any suggestion of how to best do that?

@ronag
Copy link
Copy Markdown
Member Author

ronag commented Aug 5, 2021

How about 8c198e1?

@mcollina
Copy link
Copy Markdown
Member

mcollina commented Aug 5, 2021

ok for me.

@ronag ronag force-pushed the node-fetch branch 2 times, most recently from 469f55a to 7ec6fbc Compare August 5, 2021 08:44
Comment thread index.js Outdated
Comment thread test/node-fetch/response.js
Comment thread test/node-fetch/headers.js Outdated
Comment thread test/node-fetch/main.js Outdated
Comment thread test/node-fetch/main.js Outdated
@ronag ronag changed the title test: port node-fetch test: port node-fetch tests Aug 5, 2021
@ronag ronag force-pushed the node-fetch branch 4 times, most recently from de12940 to 6cdaa6e Compare August 5, 2021 11:06
Comment thread lib/api/api-fetch/headers.js Outdated
@ronag
Copy link
Copy Markdown
Member Author

ronag commented Aug 5, 2021

Applied the file restructure/refactor to main and rebased to make it more clear what changes this PR does.

@ronag ronag force-pushed the node-fetch branch 2 times, most recently from 9179fd8 to ab64f2e Compare August 5, 2021 11:10
Comment thread scripts/test-node-fetch.js Outdated
@GrosSacASac
Copy link
Copy Markdown

Does it mean fetch will be built in to nodejs ?

@jasnell
Copy link
Copy Markdown
Member

jasnell commented Aug 12, 2021

Not yet. nodejs/undici is still a separate module independent of node.js core. There is an active discussion happening about whether it will be vendored in but that decision has not been made yet.

@szmarczak
Copy link
Copy Markdown
Member

Just finished reviewing this. This is really good piece of art :)

@max-hk
Copy link
Copy Markdown

max-hk commented Aug 12, 2021

Not yet. nodejs/undici is still a separate module independent of node.js core. There is an active discussion happening about whether it will be vendored in but that decision has not been made yet.

Where do the discussion take place?

@trivikr
Copy link
Copy Markdown
Member

trivikr commented Aug 12, 2021

Not yet. nodejs/undici is still a separate module independent of node.js core. There is an active discussion happening about whether it will be vendored in but that decision has not been made yet.

Where do the discussion take place?

It's at nodejs/node#38533

Comment thread lib/fetch/body.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.