Skip to content

Update FetchAPI constructors#103

Merged
nojaf merged 2 commits into
rescript-lang:mainfrom
stephanoskomnenos:main
May 22, 2025
Merged

Update FetchAPI constructors#103
nojaf merged 2 commits into
rescript-lang:mainfrom
stephanoskomnenos:main

Conversation

@stephanoskomnenos

Copy link
Copy Markdown
Contributor

No description provided.

@nojaf nojaf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you, looks good!
One small nit, I would like to avoid the KV abbreviation.

Comment thread src/FetchAPI/HeadersInit.res Outdated
/**
[Read more on MDN](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#setting_headers)
*/
external fromKVArray: array<(string, string)> => headersInit = "%identity"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
external fromKVArray: array<(string, string)> => headersInit = "%identity"
external fromKeyValueArray: array<(string, string)> => headersInit = "%identity"

Comment thread src/FetchAPI/URLSearchParams.res Outdated
*/
@new
external make2: (~init: any=?) => urlSearchParams = "URLSearchParams"
external fromKVArray: array<(string, string)> => urlSearchParams = "URLSearchParams"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
external fromKVArray: array<(string, string)> => urlSearchParams = "URLSearchParams"
external fromKeyValueArray: array<(string, string)> => urlSearchParams = "URLSearchParams"

Comment thread tests/FetchAPI/Headers__test.res Outdated

let h2 = Headers.fromDict(dict{"X-Vegetable": "Carrot"})

let h3 = Headers.fromKVArray([("X-Fruit", "Apple"), ("X-Vegetable", "Carrot")])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
let h3 = Headers.fromKVArray([("X-Fruit", "Apple"), ("X-Vegetable", "Carrot")])
let h3 = Headers.fromKeyValueArray([("X-Fruit", "Apple"), ("X-Vegetable", "Carrot")])

params1->URLSearchParams.keys->Iterator.toArray->Array.forEach(Console.log)

let params2 = URLSearchParams.make3(~init="?foo=1&bar=b")
let params2 = URLSearchParams.fromKVArray([("foo", "1"), ("bar", "b")])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
let params2 = URLSearchParams.fromKVArray([("foo", "1"), ("bar", "b")])
let params2 = URLSearchParams.fromKeyValueArray([("foo", "1"), ("bar", "b")])

@stephanoskomnenos

Copy link
Copy Markdown
Contributor Author

Thanks for your suggestion!

@stephanoskomnenos

Copy link
Copy Markdown
Contributor Author

How is Response.fromString("") comparing to Response.make(~body=BodyInit.fromString(""))?

@nojaf

nojaf commented May 22, 2025

Copy link
Copy Markdown
Member

How is Response.fromString("") comparing to Response.make(~body=BodyInit.fromString(""))?

Hmm, you are suggesting to maybe reuse BodyInit?
Response.fromString("") seems more ergonomic.

@nojaf nojaf merged commit ef9aeac into rescript-lang:main May 22, 2025
2 checks passed
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.

2 participants