Closed (fixed)
Project:
API Client
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
25 Jul 2023 at 20:04 UTC
Updated:
19 Sep 2023 at 14:34 UTC
Jump to comment: Most recent
It should be possible to provide an alternate fetch method to an instance of the API Client.
The options argument of the ApiClient class will support an additional optional 'fetch' property which will be a method that matches the signature of the global JS fetch method.
If this fetch option is provided, it will be used for all fetch requests.
* Implement the necessary changes
* Define necessary types
* Document the current ApiClient class inline.
* Add test coverage.
The options object will now include a fetch property.
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:
Comments
Comment #2
coby.sher commentedI do think this is a valid feature, but should it be part of the Vertical Slice PoC?
fetchis still listed as Stability 1 in node v20 but is likely to leave experimental status at some point in the future.fetch and fetch-compatible libraries return a Response object while (still somewhat popular) libraries like Axios use the XMLHttpRequest constructor which does not return the same Response object.
In the case where we want to make this library compatible with Axios or other libraries that rely on XMLHttpRequest as a custom fetcher, we need to make sure our types work for both. Axios is not useable in DrupalState for this reason so it would be great to get out in front of this.
Axios also handles JSON serialization and some other things like intercepting requests and responses. Can we allow the custom fetcher to handle those things if it has the capability?
Alternatively, we could be opinionated and not support Axios and similar libraries.
Comment #3
brianperry> I do think this is a valid feature, but should it be part of the Vertical Slice PoC?
As scope becomes clearer, we might come to the conclusion that this is impractical but what I'm aiming for here is an simplified version of the entire API for a single operation (getting a collection of resources.) I think that will force us to identify questions like the ones you outlined here early, and also make it easier for more people to contribute leading up the 1.0 release. So based on that I'd say we should assume this is part of the POC for now.
As for the fetch vs axios comments, I think that actually helps draw a pretty clear line for this POC. The POC should only support things compatible with fetch. We can than consider support Axios like things for 1.0.
Comment #4
brianperryThis is mostly addressed by our initial project scaffolding, but I think likely needs more complete tests and possibly improved documentation.
Comment #5
coby.sher commentedI am going to start working on this :)
Comment #7
coby.sher commentedComment #9
brianperryMerged. Thanks @coby.sher! Great to have `msw` introduced into the codebase now - mocking API calls is something we'll need to do quite a bit.