Should returning top-level arrays in API responses be a no-no?

[TylerMatteo]

@TangoYankee I wanted capture a possible anti-pattern we've been implementing and get your thoughts on it. I've seen some best practices for designing API responses recommend always return a top-level object and never an array as the response body. We have several endpoints that return "collections" such as /zoning-districts/{uuid}/classes that return top-level arrays. I'll try explain some reasons why this isn't a great precedent, as I understand it:

• It's rare that you can actually get away with returning a top-level array in the first place. We do it in these endpoints because we want all of the data that exists for the given resource and are confident that it can all reasonably fit in one response. However, it the vast majority of cases, we would need "meta" info on the response for doing things like pagination. We haven't implemented a /api/tax-lots endpoint, but if we did, I'd imagine the response would look something like

{
  taxLots: [...tax lot objects],
  page: 1
  // other stuff as needed...
}

Given that context, one could make the argument that even endpoints that don't need pagination should follow this pattern for the sake of consistency

• If we do always return a top level object, instead of an array, it raises the question of what to call the property that holds the array (taxLots in my example above). I think there are two approaches here. The first (and the one I would prefer) is that you use a pluralized, camelcased property name describing what it is a list of, hence taxLots. One thing I like about this approach is that it gives you a nice property to destructure out of the response object in the code gen'd client code, instead of the developer having to rename the data property, which can lead to inconsistency and clunky variable names. Instead of this code:

const { data: landUses } = useGetLandUses(); // note `landUses` could be anything

If the API returns an object with a landUses property, you can do

// In these examples, note that `data` is a property of the object Tanstack Query's `useQuery` hook returns (https://tanstack.com/query/v4/docs/react/reference/useQuery)
const { data: { landUses } } = useGetLandUses() 

The second approach is that you adopt a convention to always name the property the same thing, regardless of the resource the endpoint is referencing. I mostly see data used for this. I find this approach to be clunky, especially when you get around to making calls on the frontend because many frontend HTTP client libraries return objects that wrap the entire HTTP response in an object that has the body under an object called data. So, if our /land-uses endpoint returned an object like this:

{
  data: [...array of land use object]
}

and you called that endpoint with Axios, for example, you end up with code like this:

const response = await axios.get("/api/land-uses");
response.data.data.map(landUse => { console.log(landUse)} // yucky 

• I only came across the concern today but apparently there are security implications around returning arrays, though if that SO post is to believed, those concerns may be outdated

Conclusion

Thoughts on adopting this pattern now? It would be pretty trivial to make the changes, not that we would have to undertake it right away necessarily. If we did, my recommendation would be for the first approach described above.

[TylerMatteo]

I should add - I think adopting this pattern only affects endpoints that return collections of resources, not singletons. I don't see a problem with /tax-lots/<bbl> just returning the tax lot object itself.

[TangoYankee]

Nesting the array inside a key named for the object its returning. seems like a good pattern from a meta-data perspective. The security implications seem to have faded. But, being able to extend the endpoints with additional meta-data is a strong reason on its own.

[TangoYankee]

Getting into the specifics:

• /boroughs
  • update to return

{ boroughs: Array<borough> }

• /land-uses
  • update to return

{ landUses: Array<landUse> }

• tax-lots/<bbl>
  • No update
• tax-lots/<bbl>/geojson
  • No update
• tax-lots/<bbl>/zoning-districts
  • Update to return

{
  zoningDistricts: Array<zoningDistrict>
}

• tax-lots/<bbl>/zoning-districts/classes
  • Update to return

{
  zoningDistrictClasses: Array<zoningDistrictClass>
}

• zoning-districts/<uuid>
  • No update
• zoning-districts/<uuid>/classes
  • Update to return

{
  zoningDistrictClasses: Array<zoningDistrictClass>
}

• zoning-district-classes
  • Update to return

{
  zoningDistrictClasses: Array<zoningDistrictClass>
}

• zoning-district-classes/category-colors
  • No update
• zoning-district-classes/<id>
  • No update

** Edited to remove path parameter from returned object

[TylerMatteo]

My only critique to this is to remove the cases where you're returning the path parameter because I think it will create an awkward precedent and is returning more than is necessary.

[TangoYankee]

Tickets to implement: #59 and #60