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

fix: Export values and all type information #914

Merged
merged 2 commits into from
Feb 14, 2023

Conversation

aaronhuggins-carewell
Copy link
Contributor

Fixes

For CommonJS exports, namespaces and functions can be merged. This allows the compiler to merge the function with value properties instead of manually assigning each one, while also exposing types at the top level of the module export.

Fixes #913

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

For CommonJS exports, namespaces and functions can be merged. This
allows the compiler to merge the function with value properties instead
of manually assigning each one, while also exposing types at the top
level of the module export.
@aaronhuggins-carewell
Copy link
Contributor Author

I recognize that there are no tests, but actually testing type exports can be difficult and there's no obvious standard. Tell me how you'd like tests for type exports written and I'll happily add them.

Otherwise, from my perspective, assuming all tests pass in CI as they do locally, existing tests should validate the change.

Copy link
Contributor

@Hunga1 Hunga1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @aaronhuggins-carewell. Thanks for making this contribution! Looks good.

@Hunga1 Hunga1 merged commit 67e3a5f into twilio:main Feb 14, 2023
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.

How to get Twilio class type?
2 participants