Add proper type exports

[JonathanWolfe]

Add .d.ts exports and point the package.json's"types" to it so that internal type errors don't bubble up into consumers type checks.

Interal type errors can be seen by turning noUncheckedIndexedAccess on giving an output like:

$ /.../app/node_modules/.bin/tsc
node_modules/wordpress-api-client/src/util.ts:27:3 - error TS2322: Type 'string | undefined' is not assignable to type 'string'.
  Type 'undefined' is not assignable to type 'string'.

27   return (json as { message: string[] }).message[0]
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 1 error in node_modules/wordpress-api-client/src/util.ts:27

The compiler option skipLibCheck disables checking .d.ts for node_modules to fix this exact issue, but since you pointed types to the actual .ts files it ignored that setting.

[dkress59]

Hey @JonathanWolfe, first of all, thanks for reaching out.

I'm not sure if I'm okay with your PR. Obviously (if you look at the ToDo in the source code), the line in question, or rather the utility function it belongs to, needs to be refactored. Your PR obfuscates the issue rather than fixing it.

Alas, I will not merge this PR – but I will keep it open until the issue has been resolved (by you, me, or anyone else). I have added an overdue ticket to the project's KanBan board, though!

Feel free to refactor the utility function, yourself. This repo has been a little inactive, recently, because I have taken on replacing the wp-types package with my own because of the known issues – so it's unlikely that I'll be fixing the utility function myself anytime soon.

Keep it up!

[dkress59]

hey @JonathanWolfe, how are you doing – do you still use this package? I'm planning on updating it and fixing the console errors in the near future.

[JonathanWolfe]

Yep, we still use it. We do have 2 patches (1 for error handling, 1 for pagination) we apply but haven't fixed this specific issue.

I'll post the patches here after lunch and my meetings and lunch.

[dkress59]

oh, that's cool! no rush from my side, it's 9pm over here and I won't need the client, myself, for another few weeks – but I'll make sure to review and apply your patches asap, of course.

[JonathanWolfe]

wordpress-api-client+0.4.6.patch.txt
Here's the .patch file for the other changes. It's from patch-package so it only edits the dist but the same changes to the ts files should be the same

[dkress59]

Heya, thanks for the patch!
So you would like to always return the totalPages together with each .get or .getAll request?
I do see the use case, but I would prefer to have each method only return one type of data – after all, that's what the .getTotal method is meant for, and if it's about performance you can always refer to Promise.all(), right?

I am going to patch in the changes to .createEndpointTotal, just one question before I do that:
The param you added to the method is called defaultQuery, according to e.g. .createEndpointGet, but then it gets passed into http.get as the headers – meaning it won't be appended to the URI as URLSearchParams like in .createEndpointGet, but rather it will be used as the request headers.
So my question is, what is the use case? Do you need to pass in request headers or is it meant to be URLSearchParams, or do you think both options should be implemented?

[JonathanWolfe]

Returning pagination on all routes definitely seems like the right thing to do, especially since it's dependent on the per_page of the query being used. The current getTotal method isn't useful without allowing for query parameters to be sent in with it.

If HEAD requests did return the wp pagination headers on all hosts (wpengine doesn't respond for instance) then getTotal could have a use case, but I don't have enough research on other hosted WP sites to know if it's that's true enough to stay.

[dkress59]

okay, so we need to implement URLSearchParams (such as &per_page=1) into the .getTotal method, I see – you can expect v0.4.7 in the next couple of hours.
and now I also understand why you implemented the totalPages in the .get and .getAll methods: GET vs HEAD (vs OPTIONS) – I'm still not 100% sold on the solution, but the problem will be taken care of!

[dkress59]

@all-contributors please add @JonathanWolfe for code, ideas

should somebody else be added, @JonathanWolfe ?

[allcontributors]

@dkress59

I've put up a pull request to add @JonathanWolfe! 🎉

[JonathanWolfe]

No, I'm the only one who's modified the library at my work.

[dkress59]

I've released v0.4.7 with the defaultQuery applied to {postType}.total() thanks for pointing out the issue!
over the weekend I will try to improve readability on the source code and fix those console errors, which this PR originally was about.
When I'm done with that I'll give another thought to including pagination on the .get methods!

[dkress59]

v0.4.8 is out now, including declaration files. I also fixed the bug which was causing the error (I think), and added unit tests to the function.
thanks for your help, let me know if this solves your issues!