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

DEVEXP-382_Change package exports for Fax API #55

Merged
merged 6 commits into from
Apr 12, 2024

Conversation

asein-sinch
Copy link
Collaborator

This PR showcases a new way of exporting an API packages (The rationale behind the need to do that is explained in the Jira ticket)
Summary of the changes:

  • extract the requestData interfaces from the APIs files to put them in a dedicated folder /models/requests
  • them for the webhook event parsed
  • Breaking change:
    • export the models as Fax
    • update everywhere in the code where a model was imported
      Before:
import { AddEmailToNumbersRequestData } from '@sinch/sdk-core';
const requestData: AddEmailToNumbersRequestData = { ... }

Now:

import { Fax } from '@sinch/sdk-core';
const requestData: Fax.AddEmailToNumbersRequestData = { ... }

If you agree with this approach, the same needs to be done for the other APIs. This PR is just a POC of a "small" API.

@asein-sinch asein-sinch requested a review from a team April 9, 2024 08:13
@JPPortier
Copy link

I did not dug to full implementation but according to PR's description, what you're trying to fix could be compared to package tree for java to avoid name collisions.
So general question before details: have you look at migrating to the use of namespace?
That seems to be exactly what it's for: https://www.typescriptlang.org/docs/handbook/namespaces.html

@asein-sinch
Copy link
Collaborator Author

So general question before details: have you look at migrating to the use of namespace?

Thanks for your remark and for considering different approaches for organizing the TypeScript code. I appreciate your willingness to explore the use of namespaces and I'm open to discussing this further.
I would like to share some considerations that have let me to prefer not using them: it's considered better practice to use the ES2015 module syntax (using import and export statements) as it aligns with industry best practices and offers several advantages in terms of compatibility, encapsulation, and optimization. It's also future-proofing the codebase and staying aligned with modern JavaScript development practices: it ensures the code remains compatible with evolving tooling and easier to onboard new team members.

@JPPortier
Copy link

JPPortier commented Apr 11, 2024

So general question before details: have you look at migrating to the use of namespace?

Thanks for your remark and for considering different approaches for organizing the TypeScript code. I appreciate your
....

👍

asein-sinch and others added 3 commits April 12, 2024 09:46
DEVEXP-382: SMS API - Change package exports (#60)
DEVEXP-382: Verification API - Change package exports (#61)
DEVEXP-382: Voice API - Change package exports (#62)
DEVEXP-382: Conversation API - Change package exports (#63)
@asein-sinch asein-sinch merged commit 48fa039 into main Apr 12, 2024
2 checks passed
@asein-sinch asein-sinch deleted the DEVEXP-382_Change-package-exports branch April 12, 2024 08:04
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.

2 participants