This package does not adhere fully to JSON:API ๐ฅฒ
Opened this issue ยท 16 comments
Here's a write up of the discrepancies with the spec, and my proposed solutions.
Violations
Pagination
JSON:API does not allow the use of non-family query parameters for implementation semantics, and on top of that we SHOULD use the page
query parameter family.
So we have to change to page[number]
and page[limit]
or similar.
Sorting
Here we don't do much of what we MUST do:
- MUST use the
sort
query parameter - MAY support multiple sorts by the comma
,
, applying the sorts in order - MUST be ascending, unless the field is prefixed by the minus
-
, which then MUST be descending
Filtering
JSON:API is strategy agnostic, but it does specify the use of the filter
query parameter family, and that entails that the filter query should be structured like this:
?filter[column]=<this piece is already compliant>
Since JSON:API does not allow the use of non-family query parameters for implementation semantics, we have to change the search
query parameter to perhaps filter[@search]
(@ is the only allowed symbol at the start of a member name that can avoid collision with field names).
Sparse fieldsets
Currently we use select
, but this should be changed to fields[TYPE]
query parameter family, eg /articles?fields[articles]=title,body
Include relationships
I didn't see from the docs whether we support any syntax for this? But we could support the "Inclusion of Related Resources" as specified:
/articles?include=comments,author
Proposal
- We can create backwards-compatible additions to the syntax, and post a deprecation warning when a query contains old syntax.
- We let users opt-in to JSON:API through a new decorator
JsonApiPaginate
that constructs a standardPaginateQuery
. - We add a new overload signature to the existing
Paginate
decorator:@Paginate(format: 'legacy' | 'json:api')
with default value'legacy'
and can swap the default value after the deprecation period.
The package doesn't have a deprecation policy, and deprecating the existing syntax isn't necesarily an end goal here, just a personal preference to promote and transition to the standard.
Pagination
- Change
page
topage[number]
- Change
limit
topage[size]
Sorting
- Change
sortBy
tosort
- Change current syntax (
field:ASC/DESC
) to a comma separated ordered list of the sort fields with minus as desc operator (field1,-field2
)
Filtering
- Change
filter.field
tofilter[field]
Searching
- Change
search
to@search[query]
- Change
searchBy
to@search[fields]
- Allow
@search
as a shorthand for@search[query]
(without the possibility to then specify fields)
Sparse fieldsets
- Change
select=field1,field2
tofields[TYPE]=field1,field2
- Expand paginate logic to allow field selection for relations as well (if any fields are specified for the sub-
TYPE
)
We can try to guess the types of relations by using their key in the object and using pluralize
to derive the singular form. e.g., post.comments
would have TYPE
s post
and comment
. For more advanced cases we can accept a documentTypes: Record<string, string>
map in the PaginateConfig
that map any appearing relationship keys to their type:
{
documentTypes: {
'post.deeper.weirdKeyForComment': 'comment'
}
}
so that fields[comment]
would affect post.deeper.weirdKeyForComment
Include relationships
- When a
includes
query parameter is given, we should only load the intersect of the configuredrelations
and the givenincludes
. Use dot notation for nested relations eg?includes=post.comments
@Helveg looks like a good idea
Do you use some tool that is required to comply with this format?
No, but I like that JSON:API provides a full "HTTP-URL-compliant" spec to query REST API's including nested relationships and field selects! And in general I enjoy adhering to standards because it comes with many benefits :)
(Also, we already claim in the README to be JSON:API compliant, so this may even be considered a bugfix! ๐ )
I will get started on a PR. Which backwards compatibility / deprecation / opt-in mechanism should I add? My favorites:
Module configuration
Several idioms/flavors:
imports: [
PaginateModule.forRoot({syntax: 'json:api'})
]
imports: [
JsonApiPaginateModule
]
providers: [
JsonApiPagination
]
- + Configure in one central location
- + Backwards compatible
- + Opt-in
New decorator or decorator param
@Get()
controllerMethod(@JsonApiPaginate() query: PaginateQuery) {}
@Get()
controllerMethod(@Paginate('json:api') query: PaginateQuery) {}
- + Backwards compatible
- + Opt-in
- - Need to adjust all occurences of
Paginate
Dual parsing
We just allow both syntaxes by default ๐
- + Backwards compatible.
- + No changes required to user code.
- - Not opt-in, might mask some dev errors, change edge case behaviours, or overlap with existing application semantics of the user.
we also have swagger now, it need to be adjusted as well, once you finish I can help with it
I don't really have a strong opinion on it; as I am currently not having this package in any production use-case. But I see your point, it claims compliance - so it better be compliant lol.
Jsonapi spec seems to be quite active and reasonably popular, so I think I would be okay with taking the leap into full compliance with a breaking change release.
Don't think the headache is worthwhile to maintain backwards compatibility. In case of any security patches we can still patch the current major.
Let's go for a new major with full jsonapi compliance ๐
Sounds good! Is the PR/release workflow documented somewhere? I'll get started on this ๐
Cool, this repo uses semantic release. so every commit (if prefixed) triggers a release.
for larger major releases, we could add a beta
or next
branch. but likely overkill for such a small package.
invited both of you to the repo :)
Just one thing, I misinterpreted the query parameter section, and as long as a query parameter contains at least one non a-z character it's a valid implementation query parameter:
A โquery parameter familyโ is the set of all query parameters whose name starts with a โbase nameโ, followed by zero or more instances of empty square brackets (i.e. []) or square-bracketed legal member names. The family is referred to by its base name.
For example, the filter query parameter family includes parameters named: filter, filter[x], filter[], filter[x][], filter[][], filter[x][y], etc. However, filter[_] is not a valid parameter name in the family, because _ is not a valid member name.
Note the zero square brackets, I missed that, so no sub-member is required. And:
Implementation-Specific Query Parameters
Implementations MAY support custom query parameters. However, the names of these query parameters MUST come from a family whose base name is a legal member name and also contains at least one non a-z character (i.e., outside U+0061 to U+007A).
It is RECOMMENDED that a capital letter (e.g. camelCasing) be used to satisfy the above requirement.
This means that filter[@search]
might be needlessly complicated, and we can use a camelCased name instead, like searchText
, or simply @search
, but I can't really find a satisfying, intuitive replacement. So if you prefer anything over filter[@search]
let me know.
Some ChatGPT generated camelCased possibilities: "searchQuery", "searchColumns", "searchAll", "queryAll", "fullSearch"
For now I'm going with @search=hello
, and I'm including an expanded form with @search[query]=hello&@search[fields]=description,name
to succeed the searchBy
; I can also do searchQuery
and searchBy
? I chose @search
for similarity to the existing implementation, and the extended form @search[query]
and @search[fields]
pleased my eyes the most. I chose it over searchQuery
/searchBy
because then we'll be using strictly single words and square brackets everywhere, and @-members are designated implementation semantics.
@Helveg
just a note, it will make sense to release those fixes one by one, having it all in one will be hard to review properly
Ok, I'll PR them separately but targetting a new feature branch, so that they land on master
as 1 breaking change!
Any update or eta on this work and the complaint status? :)
Yes, I'm working on it on the json-api
branch, and have an open PR #883 that addresses some of the points here already.
I currently dont have that much open source volunteer time, and am more focused on triaging existing issues. My schedule should open up more in a month or 2