Conversation
Contributor
innerdvations
left a comment
There was a problem hiding this comment.
Haven't tested it yet, but code LGTM! Nice work!
Contributor
|
For some reason this test fails for me locally but passes on the CI: ● StrapiSDK › Custom Interceptors › HTTP › fetch should add an application/json Content-Type header to each request
expect(received).toBe(expected) // Object.is equality
Expected: "application/json"
Received: null
190 | expect(headers).toBeDefined();
191 | expect(headers).toBeInstanceOf(Headers);
> 192 | expect((headers as Headers).get('Content-Type')).toBe('application/json');
| ^
193 | });
194 |
195 | it.each([
at Object.<anonymous> (tests/unit/sdk.test.ts:192:58)Update: it fails on Node 20.9.0 but works on 20.12.0, maybe we're relying on a feature that wasn't added until after 20.9? |
Member
Author
|
It seems that a fix has been released after 20.9.0, I've constrained the SDK usage to the latest v20 release as it's in maintenance anyway. Everything should now be working |
Member
Author
|
Note: I've also updated the version for some of our outdated packages while I was there |
innerdvations
approved these changes
Jan 22, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does it do?
Major Changes
HttpClientto theStrapiSDKto remove dependency issues (for example,HttpClientshouldn't know about theAuthManager). Now the core logic is handled by the SDK itself.Minor Changes
get,put,postanddeletemethods for the HTTP modulePathFormatterclass to facilitate validation and transformation of common paths (baseURL, requests path, etc...)Why is it needed?
Groundwork for next features (user API), clean up imperfections in the architecture, and fix how modules communicate/rely on each other.
How to test it?
DEBUG=* node index.js) to check if all the requests are done correctlyYou should end up with something similar to this:
Bundle Size Evolution
The bundle grew quite a lot, but this is to be expected in the first few refactor and big features we're gonna introduce at the beginning as we add the main elements powering future capabilities.
Also, not sure why the browser build doesn't grow 🤔
dist/bundle.browser.min.jsdist/bundle.cjs.jsdist/bundle.esm.js