Closed (fixed)
Project:
API Client
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
26 Aug 2023 at 21:58 UTC
Updated:
1 Nov 2023 at 16:04 UTC
Jump to comment: Most recent
Problem/Motivation
We should support adding parameters to JSON:API requests in order to adjust the query.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork api_client-3383578
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
cosmicdreams commentedMy hope for this project is that whatever api we use to decorate a JSON:API request to include parameters doesn't hide what the eventual url is.
Seeing that full jsonapi url is a helpful way of having confidence that the request building code is doing the right thing.
Comment #3
pratik_kamble@brianperry Can you confirm the outcome for this issue is to allow adding query parameter(filter) while retrieving content? If true then we should implement it like below
- get method will have additional parameter filter get(type: string, filter?: [string, any][])
- use received filter to prepare the query parameter.
Comment #4
pratik_kambleComment #6
pratik_kambleComment #7
brianperry@pratik_kamble and @CobyPear sorry for the lack of detail here. Yes, the intent was to support all possible JSON:API query params.
Existing projects have slightly different APIs for this.
Drupal State has a params option that allows you to pass either an instance of DrupalJsonApiParams, or a full query string. https://project.pages.drupalcode.org/drupal_state/en/including-related-d... (sorry for the unstyled/busted docs...)
The Next for Drupal client can accept either an object defining the query params (https://next-drupal.org/docs/fetching-resources), or the equivalent result of DrupalJsonApiParam's getQueryObject method.
It isn't completely obvious what the ideal approach is here and I'm open to thoughts. But for the POC I'm leaning in the direction of a params option that is just a query string that is applied to the fetch request. This would allow a developer to construct the query manually, or use DrupalJsonApiParams and convert the result to a query string for use with our params option using getQueryString()
Comment #8
pratik_kambleComment #9
pratik_kambleComment #10
pratik_kamble@brianperry @CobyPear, just wondering how cache should work with api parameter. Should we add query param(by doing base64 encoding) in the cache key?
Comment #11
coby.sher commented> how cache should work with api parameter. Should we add query param(by doing base64 encoding) in the cache key?
Yeah, a hash of the query string added to the rest of the key makes sense to me.
Comment #12
pratik_kamble@coby.sher I have tried to add the base64 encoded query params to cache key. Though it work. I just feel cache key will be too long based on the query params so reverted it.
BTW PR is ready for review.
Comment #13
pratik_kambleComment #14
coby.sher commentedComment #15
pratik_kambleComment #16
brianperryReviewed the latest updates. Functionally this looks good to me. However, one thing I noticed was that the use of Crypto (most likely the polyfill) inflates our bundle up to 1.45 MB (from about 2 KB.) I tried removing the polyfill, but as expected the build failed. I also tried a more specific import, but it didn't do much to reduce the bundle size.
Instead, I refactored to use @aws-crypto/sha256-js instead of Crypto. I'd prefer not to add a dependency here, but I think bringing the bundle size from 1.45 MB back down to 2.5 KB is worth the tradeoff.
Comment #17
pratik_kambleComment #19
brianperry