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: use "import type" in the Typescript SDK #623

Merged
merged 1 commit into from
Apr 22, 2021
Merged

Conversation

jkaster
Copy link
Contributor

@jkaster jkaster commented Apr 22, 2021

Updated the code generator and the TS SDK to use import type where appropriate

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@joeldodge79
Copy link
Contributor

Looks like this has been around since TS 3.8 - but I do have a question around what versions of typescript we're committed to supporting in this SDK?

Also, I'd consider switching to a fix: or feat: prefix - just so it shows up in the changelog so that maybe an old TS 3.7 project that breaks might more easily find a clue as to why. But if you don't want it showing up in the changelog, then seems like refactor: is a more human-reading-git-log meaningful prefix.

@joeldodge79
Copy link
Contributor

CI is wonky - it's reporting the "skipped" runs (from the cla-bot applying "cla: yes" tag) instead of the real runs: you can see both on the checks tab (CI did pass). Need to figure that out

@jkaster
Copy link
Contributor Author

jkaster commented Apr 22, 2021

Good question about supported TS versions. I'd say since we're just now approaching GA, TS 4.x is our minimum

@jkaster jkaster changed the title chore: use "import type" in the Typescript SDK fix: use "import type" in the Typescript SDK Apr 22, 2021
@jkaster
Copy link
Contributor Author

jkaster commented Apr 22, 2021

We can consider it a "fix" although nothing was broken. It just wasn't optimal.

@joeldodge79 joeldodge79 self-requested a review April 22, 2021 23:13
@jkaster jkaster merged commit c7b5bf8 into main Apr 22, 2021
@jkaster jkaster deleted the jk/ts_import_type branch April 22, 2021 23:14
@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

Test Results

    7 files    73 suites   3m 57s ⏱️
153 tests 153 ✔️ 0 💤 0 ❌
535 runs  535 ✔️ 0 💤 0 ❌

Results for commit c7b5bf8.

@AprilArcus
Copy link
Contributor

AprilArcus commented Apr 22, 2021

@joeldodge79 I agree; the minimum version we support in the GA should be the current version at that time. That's TS 4.2 as of today, with TS 4.3 scheduled for May 25.

At minimum, TS 4.1 has some extremely powerful new features that I would not want to be without in the long term.

@jkaster
Copy link
Contributor Author

jkaster commented Apr 23, 2021

We currently build codegen with TS 4.2.2, so I'm personally fine with saying 4.2 and above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants