vaadin / hilla

Build better business applications, faster. No more juggling REST endpoints or deciphering GraphQL queries. Hilla seamlessly connects Spring Boot and React to accelerate application development.
https://hilla.dev
Apache License 2.0
867 stars 58 forks source link

feat: add support for JsonNode subclasses #2537

Closed cromoteca closed 1 week ago

cromoteca commented 3 weeks ago

Support for JsonNode was added recently. This PR adds the same treatment for its subclasses.

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.00%. Comparing base (c02ab2e) to head (da9a187).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2537 +/- ## ======================================= Coverage 95.00% 95.00% ======================================= Files 66 66 Lines 4546 4546 Branches 661 661 ======================================= Hits 4319 4319 Misses 182 182 Partials 45 45 ``` | [Flag](https://app.codecov.io/gh/vaadin/hilla/pull/2537/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vaadin) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/vaadin/hilla/pull/2537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vaadin) | `95.00% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vaadin#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

platosha commented 3 weeks ago

Just wondering: should we rather represent JsonObject as Record<never, never> and JsonArray as unknown[]?

cromoteca commented 3 weeks ago

I was thinking the same thing, but mostly for simple types like numbers and strings which are transmitted as they are. But I don't expect classes like IntNode to be used as service types, as you can just use int or Integer. Concerning more complex types like the ones you mentioned, having unknown[] instead of unknown doesn't add big value to me as you still need to cast items. Representing ObjectNode as Record<never, never> shouldn't it be Record<string, unknown>? But then you still have to cast all property values.

cromoteca commented 3 weeks ago

I decided to only support the 3 non-raw-value classes, which also removes the need to use ClassGraph for discovery.

sonarcloud[bot] commented 1 week ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud