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

Project API Types are not exported at the top (module) level. #652

Open
tewen opened this issue Sep 27, 2023 · 9 comments
Open

Project API Types are not exported at the top (module) level. #652

tewen opened this issue Sep 27, 2023 · 9 comments

Comments

@tewen
Copy link

tewen commented Sep 27, 2023

SDK you're using (please complete the following information):

  • Version 4.36.0

Is your feature request related to a problem? Please describe.
All of the types for the Projects API are imported deeper in the module dependencies. Not only is this harder to manage, but it causes issues for some project build types (specifically Next.JS and Vercel):

image

Describe the solution you'd like
Like the Accounting API, I would like these exports to be available at the top-level of the module.

Describe alternatives you've considered
The main alternative has been to import them in the current way. This works fine for some projects, but I am having trouble with Node libraries that use these types.

@github-actions
Copy link

PETOSS-350

@github-actions
Copy link

Thanks for raising an issue, a ticket has been created to track your request

@madison890000
Copy link

@tewen

I just fix this issue manually updated the code in https://github.com/madison890000/xero-node.

Here is the npm link I published: https://www.npmjs.com/package/fix-xero-node.

You can use it by replacing

"xero-node": "4.35.0"
to

"fix-xero-node": "4.36.1"
.

I have only made 3 changes:

  • first is to move the new parameter "idempotencyKey" to the second-to-last position, just before the headers parameter.
    this means no changes need to be done when updating from 4.35.0 to 4.36.0.
  • second is deleting the Idempotency-Key in header if it is null/undefined.
    this means no need to add idempotencyKey if you don't want to. and it will ignore it as well if it is set to be null/undefined.
  • manually export most of the types in gen model.
    you can check the details in fix: 652 export most of types at the top-level of the module madison890000/xero-node#3

You should be careful if you just want to fix Project API Types issues and your last verson is "xero-node": "4.36.0", because fix-xero-node 4.36.1 has also fix the idempotencyKey issue(only exist in "xero-node": "4.36.0") by moving the new parameter "idempotencyKey" to the second-to-last position. That means you need check the order of params if you are using idempotencyKey in some API functions.

It should be works fine if you are upgrade xero-node from 4.35.0.

@tewen
Copy link
Author

tewen commented Oct 4, 2023

@madison890000 Really appreciate that.

Would you be willing to do a pull request back to upstream? We are extremely reliant on the Xero API, and we would prefer to get all the updates from the main line.

@madison890000
Copy link

@madison890000 Really appreciate that.

Would you be willing to do a pull request back to upstream? We are extremely reliant on the Xero API, and we would prefer to get all the updates from the main line.

I apologize for the inconvenience, but unfortunately,
(https://github.com/XeroAPI/xero-node#participating-in-xeros-developer-community)
it is not possible to fulfill a new request. The code for this SDK is automatically generated from the OpenAPISpec and is not maintained manually by the developer.

And I have no background about how to update the OpenAPISpec to fix this issure.

@tewen
Copy link
Author

tewen commented Oct 7, 2023

@madison890000 So, the way the main-line API does it is by exporting the generated models right at the index level:

https://github.com/XeroAPI/xero-node/blob/master/src/index.ts#L2C47-L2C47

I think it would be as simple as doing this with projects and other models. I just want to understand what the maintainers had in mind by not doing this.

Also, there is a likelihood for name collision. So there may need to be some renaming or a submodule export approach.

Open to different options here, I just want a build system to ensure the project models are packaged as first-class exports.

@madison890000
Copy link

@madison890000 So, the way the main-line API does it is by exporting the generated models right at the index level:

https://github.com/XeroAPI/xero-node/blob/master/src/index.ts#L2C47-L2C47

I think it would be as simple as doing this with projects and other models. I just want to understand what the maintainers had in mind by not doing this.

Also, there is a likelihood for name collision. So there may need to be some renaming or a submodule export approach.

Open to different options here, I just want a build system to ensure the project models are packaged as first-class exports.

Yeah, that's actually how I do it in this PR: madison890000#3. Initially, it confused me why the maintainers were using the old way instead of the way we had in mind.

After exploring OpenAPI-generator, I realized that the maintainers simply defined the OpenAPI docs, which determine how the RESTful API works. The OpenAPI-generator can then automatically generate SDKs for different platforms like Java, NodeJS, .Net, and Python. This suggests that the maintainers focus more on considering and maintaining the overall functionality. They prefer to fix bugs on all platforms rather than just one.

Therefore, it becomes more challenging to have the maintainers fix issues compared to fixing them manually (which is why I forked this repo).

If you have any good ideas or solutions to fix this issue, feel free to try by creating a PR in https://github.com/XeroAPI/Xero-OpenAPI.

@tewen
Copy link
Author

tewen commented Oct 11, 2023

@madison890000 Got it, and thanks for the more thorough explanation.

I may try that route, obviously there is a way the original generator is doing that, so it's probably something we need to change in the configuration. Thanks for your help thus far.

@tewen
Copy link
Author

tewen commented Oct 13, 2023

I am going to keep the issue open to see if there are other ideas, I may also do one at the API generator level.

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

No branches or pull requests

2 participants