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

chore: Rewrite hand-crafted JS files into TS files #840

Merged
merged 14 commits into from
Dec 15, 2022
Merged

Conversation

Hunga1
Copy link
Contributor

@Hunga1 Hunga1 commented Dec 9, 2022

Converts hand-crafted JS files into TS files and removes the accompanying *.d.ts declaration files.

Notable Changes:

  • Updated exports/imports in hand-crafted library files and test files to ES module syntax
  • Removed TS namespaces (no longer necessary with ES module imports)
  • Added CA certificate bundle to https.Agent agent options used by this.axios.defaults.httpsAgent
  • Disabled RequestClient.spec.js "should cache the CA after loading it for the first time" test. Current implementation doesn't allow the custom a https.Agent to be created at request execution with the current CA bundle for the Axios instance to use (ref: DI-2477)

Internal ref: DI-1418

@Hunga1
Copy link
Contributor Author

Hunga1 commented Dec 9, 2022

Some of the tests will still fail for the following reasons:

  • Auto-generated TwiML modules updated to use the TS version of the TwiML module is introduced in pending chore: Convert TwiML to typescript #838 (merged)
  • Required TwilioResponsePayload.uri property conflicts with optional uri property in other interfaces that auto-generated types extend (ref: DI-2475)
  • Auto-generated FlowRevisionInstance._solution property does not fulfill FlowRevisionContextSolution interface (ref: DI-2475)
  • Auto-generated file imports and references to the Page exports should no longer rely on a "Page" namespace (chore: Updated Page module imports and references twilio-oai-generator#362) (merged)

@Hunga1
Copy link
Contributor Author

Hunga1 commented Dec 13, 2022

Changes should be merged alongside the removal of the Page namespace references in the generated code (namespace no longer exists). Link to PR: #841 (merged)

Copy link
Contributor

@mattcole19 mattcole19 left a comment

Choose a reason for hiding this comment

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

LGTM!

lib/base/Domain.ts Outdated Show resolved Hide resolved
lib/base/Page.ts Outdated Show resolved Hide resolved
lib/base/Page.ts Outdated Show resolved Hide resolved
lib/base/Page.ts Outdated Show resolved Hide resolved
lib/base/Page.ts Outdated Show resolved Hide resolved
lib/base/Page.ts Outdated Show resolved Hide resolved
Hunga1 and others added 3 commits December 14, 2022 16:50
Improved TS type annotations for optional and required parameters and undefined return values.
@Hunga1 Hunga1 merged commit 8345f7e into 4.0.0-rc Dec 15, 2022
@Hunga1 Hunga1 deleted the replace_js_with_ts branch December 15, 2022 00:42
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.

3 participants