CALL-3878: Migrate to TS and add CJS & ESM Builds

HubSpot / calling-extensions-sdk

A JavaScript SDK for integrating calling apps into HubSpot.

https://developers.hubspot.com/docs/api/crm/extensions/calling-sdk

MIT License

45 stars 29 forks source link

CALL-3878: Migrate to TS and add CJS & ESM Builds #174

Open hemang-thakkar opened 7 months ago

hemang-thakkar commented 7 months ago

Description

Jira Ticket: CALL-3878

Merge Checklist

Q	A
Adds Documentation?
Any Dependency Changes?
Patch: Bug Fix?
Minor: New Feature?
Major: Breaking Change?	✅

BRAVE Checklist

[ ] I have read the BRAVE checklist and confirmed if the following is necessary.

Q	A
Backwards Compatible?
Rollout/Rollback Plan?
Automated test coverage?
Verified that changes work?
Expect Dependencies to Fail?

github-actions[bot] commented 7 months ago

PR Preview Action v1.4.4 :---: :rocket: Deployed preview to https://HubSpot.github.io/calling-extensions-sdk/pr-preview/pr-174/ on branch gh-pages at 2024-02-09 19:54 UTC

alonso-cadenas commented 7 months ago

These changes mean we are coupling our inbound calling release with the TS migration. Here are a few concerns:

Is there more risk for something going wrong?
We're not going to GA with inbound calling realistically until Q3 2024. Are we okay with the TS migration being blocked until then?

esme commented 7 months ago

These changes mean we are coupling our inbound calling release with the TS migration.

Yeah these are valid concerns. We could do the following to mitigate risk:

Split this PR into smaller PRs with files that have varying levels of impact on the release, i.e. lower risk changes such as the React Demo app
Maintain backwards compatibility by duplicating files in JS and TS
Publish an alpha version to test it with other providers in inbound calling beta

hemang-thakkar commented 7 months ago

These changes mean we are coupling our inbound calling release with the TS migration.

Yeah these are valid concerns. We could do the following to mitigate risk:

Split this PR into smaller PRs with files that have varying levels of impact on the release, i.e. lower risk changes such as the React Demo app

Maintain backwards compatibility by duplicating files in JS and TS

Publish an alpha version to test it with other providers in inbound calling beta

I agree we should split this up into multiple PRs and incrementally migrate. I doubt we would need to duplicate files as we are exporting from index.js. So, customers always import from the package which points to index file. Internal structure shouldnt cause problem. I was not aware of realistic date for inbound GA. But, from this PR, we can split up our story into tasks and do incremental migration release.

hemang-thakkar commented 7 months ago

These changes mean we are coupling our inbound calling release with the TS migration. Here are a few concerns:

Is there more risk for something going wrong?

We're not going to GA with inbound calling realistically until Q3 2024. Are we okay with the TS migration being blocked until then?

Ideally breaking change warrants for major version release. But, our timeline might not be helpful here.