Skip to content

Conversation

@gaetschwartz
Copy link
Contributor

@gaetschwartz gaetschwartz commented Apr 20, 2023

Changes

  • Add default response status and status text in the Response constructors
  • Add Response.jsify method for initializing Response with a JS object as body
  • Separate Response.json method from Response.jsify for initializing Response with a JSON-encodable object as body
  • Add documentation for main the constructors of Response

Motivation

jsify is subject to minification and has somewhat bad performance. When using dart compile with minification enabled, the response json gets minified (after a certain depth AFAIK). It should not be the case. The new Response.json behaves simlarly from the previous implementation except that it doesn't minify as it uses json.encode. Renamed the previous implementation to Response.jsify as it could still be useful for some cases.

- Add default response status and status text in the `Response` constructors
- Add `Response.jsify` method for initializing `Response` with a JS object as body
- Separate `Response.json` method from `Response.jsify` for initializing `Response` with a JSON-encodable object as body
- Add documentation for main the constructors of `Response`
@Salakar Salakar requested a review from Ehesp May 2, 2023 10:37
@Ehesp
Copy link
Member

Ehesp commented May 2, 2023

@gaetschwartz thanks for this - what's the use case for the jsify method?

@gaetschwartz
Copy link
Contributor Author

Well, as jsify can translate Dart objects to Js objects directly, people could still have some use. As it was the old behavior I figured I would leave it.

@gaetschwartz gaetschwartz requested a review from Salakar May 14, 2023 12:34
@rrousselGit
Copy link
Collaborator

I'm not really familiar with the project, but it looks alright at a quick glance.

I'd still consider this a breaking change (since the "json" constructor changes).
And it'd be great to have some tests. But the project doesn't seem to have some atm, so that's alright I guess

@Salakar Salakar removed their request for review September 16, 2024 09:33
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.

4 participants