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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use fetch #541

Merged
merged 18 commits into from
Nov 9, 2023
Merged

Use fetch #541

merged 18 commits into from
Nov 9, 2023

Conversation

k-kozika
Copy link

Solve #531

@CLAassistant
Copy link

CLAassistant commented Oct 21, 2023

CLA assistant check
All committers have signed the CLA.

@k-kozika
Copy link
Author

I think there is a problem due to nock/nock#2397?

@k-kozika
Copy link
Author

I think we can change the test library to msw or wait for nock to complete its task

@tokuhirom
Copy link
Member

Nock is a mock library that was originally chosen by the first author of line-bot-sdk-nodejs. We are using it just because it was initially used, and we don't have a strong preference for it. Switching to MSW would not be a problem.

tokuhirom added a commit that referenced this pull request Nov 2, 2023
nock doesn't support `window.fetch`. As a result,
#541 failes.
So, it's time to use MSW!
@tokuhirom
Copy link
Member

#573 may fix the tests... and so on,

@k-kozika could you sign the CLA?

@k-kozika
Copy link
Author

k-kozika commented Nov 4, 2023

@k-kozika could you sign the CLA?

Ah, I didn't notice so I did it.

@k-kozika
Copy link
Author

k-kozika commented Nov 6, 2023

There seems to be a problem with file uploads and some assertions (9 more tests to pass!)

@tokuhirom
Copy link
Member

@k-kozika can you fix it?

@k-kozika
Copy link
Author

k-kozika commented Nov 7, 2023

@k-kozika can you fix it?

I can't do it, can you do it?

@tokuhirom
Copy link
Member

I had a hard time modifying the test cases, so I created a PR that directly implements fetch code in the OpenAPI-based code generation. #582

@Yang-33 Yang-33 changed the base branch from master to drop-axios November 9, 2023 08:19
@Yang-33
Copy link
Contributor

Yang-33 commented Nov 9, 2023

@k-kozika @tokuhirom
Thank you for the change! Let's try to check this change and #582 work in drop-axios branch at first.

@Yang-33 Yang-33 merged commit 427ae4e into line:drop-axios Nov 9, 2023
1 of 3 checks passed
tokuhirom added a commit that referenced this pull request Mar 1, 2024
As a result of neglecting #582, it became impossible to merge, so I
recreated the Pull-request.
Compared to the last time, we developed it by narrowing down the changes
to make it less likely to cause conflicts.

## Development steps

1. make axios as a optionalDependency
2. move http.ts as http-axios.ts
3. create new http-fetch.ts

as a result, legacy API will depends on the `http-axios.ts`. and so
openapi based clients are depend on `http-fetch.ts`.
@Yang-33
Copy link
Contributor

Yang-33 commented Mar 3, 2024

Note this change won't be merged in master branch. A series of changes (#723) resolved finally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants