Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: usage of axios [PHX-2738] #370

Merged
merged 2 commits into from Jul 19, 2023
Merged

fix: usage of axios [PHX-2738] #370

merged 2 commits into from Jul 19, 2023

Conversation

marcolink
Copy link
Member

@marcolink marcolink commented Jul 17, 2023

Every major version bump needs to be followed by a fix 馃檲

Changes

headers type

Internally we always initialise an AxiosHeaders object by default

Custom error type

The newly introduced error type had an unknown prop. We changed it to any for simplicity.

Remove paramsSerializer again

We provided a custom paramsSerializer prop based on this discussion. At the time released (with latest axios@1.4.0), this is not required anymore, thanks to this fix.

Testing

These changes are manually tested against contentful.js and contentful-management.js. As soon as this is released, we can create PRs for both projects.

Migration Learnings

I also started a little doc for all the findings.

@marcolink marcolink requested a review from a team as a code owner July 17, 2023 14:07
@marcolink marcolink changed the title fix: types (headers and error) [NONE] fix: usage of axios [NONE] Jul 18, 2023
@marcolink marcolink changed the title fix: usage of axios [NONE] fix: usage of axios [PHX-2738] Jul 18, 2023
@marcolink marcolink merged commit b014b84 into master Jul 19, 2023
2 checks passed
@marcolink marcolink deleted the fix/types branch July 19, 2023 09:13
@contentful-automation
Copy link

馃帀 This PR is included in version 8.1.0 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

ruderngespra added a commit that referenced this pull request Nov 29, 2023
We reintroduce the `paramsSerializer` (again) after removing it
[here](#370).

This is to unblock [the axios bump in
contentful-merge](contentful/contentful-merge#462)
or the skd-core bump [as
such](contentful/contentful-merge#552). We saw
that tests were failing because the query set together
[here](https://github.com/contentful/contentful-merge/blob/main/src/engine/create-changeset/tasks/create-fetch-partial-entities-task.ts#L43)
can not properly be handled by the API anymore and instead runs into

`error: InvalidQuery: The query you sent was invalid. Probably a filter
or ordering specification is not applicable to the type of a field.`

Tested locally on `contentful.js` and `contentful-merge`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants