feat: add support for new version of @heyapi and use fetch client ins… #125

omridevk · 2024-06-12T22:24:17Z

…tead of axios

omridevk · 2024-06-12T22:31:54Z

@7nohe @seriouslag
This is a draft PR that maybe can get some discussion going on how to make this less of a breaking change.
The new hey api clients doesn't provide a service anymore by default.

omridevk · 2024-06-12T22:32:07Z

By the way, really great lib :D

omridevk · 2024-06-12T22:34:32Z

I also noticed that in the main branch the react example the UI doesn't update when you create a new pet, this address this issue as well

omridevk · 2024-06-12T22:37:22Z

might help address this as well:
#94

seriouslag

Thank you for the amazing work!

seriouslag · 2024-06-12T23:09:15Z

examples/react-app/src/App.tsx

  // This is an example of using a hook that has all parameters optional;
  // Here we do not have to pass in an object
-  const { data: _ } = useDefaultServiceFindPets();
+  const { data: _ } = useFindPets();


Dropping the use of tag names makes this a breaking change.

I must consider whether dropping the tag to generate the hook is helpful.
I see a benefit in having the tag help describe the hook, especially for large spec files.
I also see the beauty in not having the tag there.

are you referring the "defaultService"? because wasn't sure what to put there if there's no real concept of a service in the new client.

I was referring to defaultService. For large spec files it’s a nice to have buckets to put similar api calls. dogService, catService, etc. makes DX better as you can discover api calls related to what you are working with using intelliSense/autocomplete

How are the names of the hooks being generated now? I believe they are generated from the method names of the generated client code from @hey-api. Does @hey-api use operationId or is it generated from the displayName, or something else? How are duplicate names handled.

I can imagine 2 generated hooks having the same or similar name using the clients code now that tags are not used in the names.

That was my stream of consciousness. I haven’t dug into the code yet.

@seriouslag it is using the operationId and AFAIK it should be unique in the spec no?

I believe OperationId to be unique but not a required field in the OpenAPI spec.

yeah I checked and it is either using the operationId, or build it using the URL, so we can definitely have a conflicts no?

seriouslag · 2024-06-12T23:09:32Z

examples/react-app/src/App.tsx

@@ -42,8 +42,8 @@ function App() {
    <div className="App">
      <h1>Pet List</h1>
      <ul>
-        {Array.isArray(data) &&
-          data?.map((pet, index) => (
+        {Array.isArray(data?.data) &&


data?.data is ugly right?
Why do we have another object with data inside of it?
This also introduces a breaking change.

yup, was planning to dig into this and see what was changed.

@seriouslag fixed. not sure how I like the solution but you can you see it in this commit:
a341432

omridevk · 2024-06-13T12:28:52Z

@seriouslag
Also added formatters and linters support, as those would never run on the queries folder but only on the requests folder

seriouslag · 2024-06-13T22:31:31Z

src/format.mts

+  },
+};
+
+export const processOutput = async ({


Great work adding linting support.

I don't support putting linting and formatting in this PR.

@7nohe and I discussed dropping support in favor of letting users run the linters/formatters themselves.

If we were to keep the linters/formatters we should use this implementation to lint all the generated files instead of using the built-in implementation in @hey-api so that we don't have to worry if they break compatibility or change something

np, i'll remove

omridevk · 2024-06-14T16:16:57Z

Let's talk about the big elephant in the PR.
How do we make this a non breaking change?
I don't mind hopping on discord to discuss this.
Appreciate your feedback

seriouslag · 2024-06-14T19:21:45Z

Let's talk about the big elephant in the PR. How do we make this a non breaking change? I don't mind hopping on discord to discuss this. Appreciate your feedback

A breaking change is not no-go but this is a huge change as every single consumer has will have to change every hook. I feel we should not do that and offer a codemod or generate the old and new hooks and deprecate the old hooks. Or we could not break every hook and add back the service layer using the tag names. We could generate the services and this would be a non-breaking change.

omridevk · 2024-06-15T09:04:42Z

Let's talk about the big elephant in the PR. How do we make this a non breaking change? I don't mind hopping on discord to discuss this. Appreciate your feedback

A breaking change is not no-go but this is a huge change as every single consumer has will have to change every hook. I feel we should not do that and offer a codemod or generate the old and new hooks and deprecate the old hooks. Or we could not break every hook and add back the service layer using the tag names. We could generate the services and this would be a non-breaking change.

yes there are several ways to go around this, I was thinking we can discuss it in discord or something so we can align on a way to tackle this with a plan, I don't mind the extra work.
An FYI, we might fork this and modify it to our needs as we don't need support for all other clients, only the new fetch, I have published a beta version with these changes to check if it is working in our env and everything seems to be working.

Regardless, i'd much rather contribute to this great lib and not only using a fork, you guys did really nice work here and i'd really want to give back :D

seriouslag · 2024-06-18T18:11:33Z

Let's talk about the big elephant in the PR. How do we make this a non breaking change? I don't mind hopping on discord to discuss this. Appreciate your feedback

A breaking change is not no-go but this is a huge change as every single consumer has will have to change every hook. I feel we should not do that and offer a codemod or generate the old and new hooks and deprecate the old hooks. Or we could not break every hook and add back the service layer using the tag names. We could generate the services and this would be a non-breaking change.

yes there are several ways to go around this, I was thinking we can discuss it in discord or something so we can align on a way to tackle this with a plan, I don't mind the extra work. An FYI, we might fork this and modify it to our needs as we don't need support for all other clients, only the new fetch, I have published a beta version with these changes to check if it is working in our env and everything seems to be working.

Regardless, i'd much rather contribute to this great lib and not only using a fork, you guys did really nice work here and i'd really want to give back :D

Thank you for all the work you have put into this! I have been away on vacation and will be for another week. Let’s touch base on discord when I get back.

feat: add support for new version of @heyapi and use fetch client ins…

8fd8a16

…tead of axios

omridevk closed this Jun 12, 2024

omridevk reopened this Jun 12, 2024

omridevk marked this pull request as draft June 12, 2024 22:24

chore: remove comment

4d4e8d7

seriouslag requested review from 7nohe and seriouslag June 12, 2024 22:58

seriouslag added the enhancement New feature or request label Jun 12, 2024

seriouslag reviewed Jun 12, 2024

View reviewed changes

omridevk added 6 commits June 13, 2024 11:53

chore: remove comment

961428d

fix: make sure to remove not relevant props from params

d34e761

fix: get data from the client

a341432

fix: get data from the client

0a1ff9d

chore: release v1.4.2-beta.1

f716732

fix: add formatters and linters on the queries output as well

9eedb82

omridevk added 3 commits June 13, 2024 18:01

chore: release v1.4.2-beta.2

55ab44d

chore: release v1.4.2-beta.3

f8df50f

chore: release v1.4.2-beta.4

873c5f2

seriouslag reviewed Jun 13, 2024

View reviewed changes

chore: release v1.4.2-beta.5

cb263a0

chore: release v2.0.0

012f10f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for new version of @heyapi and use fetch client ins… #125

feat: add support for new version of @heyapi and use fetch client ins… #125

omridevk commented Jun 12, 2024

omridevk commented Jun 12, 2024

omridevk commented Jun 12, 2024

omridevk commented Jun 12, 2024 •

edited

Loading

omridevk commented Jun 12, 2024

seriouslag left a comment •

edited

Loading

seriouslag Jun 12, 2024

omridevk Jun 12, 2024

seriouslag Jun 13, 2024 •

edited

Loading

omridevk Jun 13, 2024

seriouslag Jun 13, 2024

omridevk Jun 15, 2024

seriouslag Jun 12, 2024 •

edited

Loading

omridevk Jun 12, 2024 •

edited

Loading

omridevk Jun 13, 2024

omridevk commented Jun 13, 2024

seriouslag Jun 13, 2024 •

edited

Loading

omridevk Jun 14, 2024

omridevk commented Jun 14, 2024

seriouslag commented Jun 14, 2024

omridevk commented Jun 15, 2024 •

edited

Loading

seriouslag commented Jun 18, 2024

feat: add support for new version of @heyapi and use fetch client ins… #125

Are you sure you want to change the base?

feat: add support for new version of @heyapi and use fetch client ins… #125

Conversation

omridevk commented Jun 12, 2024

omridevk commented Jun 12, 2024

omridevk commented Jun 12, 2024

omridevk commented Jun 12, 2024 • edited Loading

omridevk commented Jun 12, 2024

seriouslag left a comment • edited Loading

Choose a reason for hiding this comment

seriouslag Jun 12, 2024

Choose a reason for hiding this comment

omridevk Jun 12, 2024

Choose a reason for hiding this comment

seriouslag Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

omridevk Jun 13, 2024

Choose a reason for hiding this comment

seriouslag Jun 13, 2024

Choose a reason for hiding this comment

omridevk Jun 15, 2024

Choose a reason for hiding this comment

seriouslag Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

omridevk Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

omridevk Jun 13, 2024

Choose a reason for hiding this comment

omridevk commented Jun 13, 2024

seriouslag Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

omridevk Jun 14, 2024

Choose a reason for hiding this comment

omridevk commented Jun 14, 2024

seriouslag commented Jun 14, 2024

omridevk commented Jun 15, 2024 • edited Loading

seriouslag commented Jun 18, 2024

omridevk commented Jun 12, 2024 •

edited

Loading

seriouslag left a comment •

edited

Loading

seriouslag Jun 13, 2024 •

edited

Loading

seriouslag Jun 12, 2024 •

edited

Loading

omridevk Jun 12, 2024 •

edited

Loading

seriouslag Jun 13, 2024 •

edited

Loading

omridevk commented Jun 15, 2024 •

edited

Loading