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

Use SWC to strip types for "--no-check" flag #6895

Merged
merged 15 commits into from
Jul 28, 2020

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented Jul 27, 2020

v1.2.1

▶ ./third_party/prebuilt/mac/hyperfine -w 3 -i 'deno cache --reload --no-check std/examples/chat/server_test.ts'
Benchmark #1: deno cache --reload --no-check std/examples/chat/server_test.ts

  Time (mean ± σ):     979.5 ms ±  10.7 ms    [User: 1.640 s, System: 0.071 s]

  Range (min … max):   967.1 ms … 1003.7 ms

this PR

▶ ./third_party/prebuilt/mac/hyperfine -w 3 -i 'target/release/deno cache --reload --no-check std/examples/chat/server_test.ts'
Benchmark #1: target/release/deno cache --reload --no-check std/examples/chat/server_test.ts

  Time (mean ± σ):      90.3 ms ±   3.5 ms    [User: 69.6 ms, System: 16.7 ms]

  Range (min … max):    86.5 ms … 100.1 ms

Binary size increased from 55942928 to 58668472

Closes #6864

Blocked by swc-project/swc#903

@bartlomieju bartlomieju changed the title [WIP] strip types using SWC Use SWC to strip type for "--no-check" flag Jul 27, 2020
@bartlomieju bartlomieju changed the title Use SWC to strip type for "--no-check" flag Use SWC to strip types for "--no-check" flag Jul 27, 2020
@bartlomieju
Copy link
Member Author

bartlomieju commented Jul 27, 2020

@kitsonk I have a question for you - if we'll land this PR is anything stopping us from using TS compiler with noEmit option and doing the actual transpilation using SWC? If I'm not mistaken this move could shave off some time that is spent in TS compiler.

EDIT: Statistics for deno cache --reload std/examples/chat/server_test.ts

DEBUG - Compilation Statistics:
Files: 62
Nodes: 48638
Identifiers: 17259
Symbols: 13988
Types: 5777
Instantiations: 4550
Parse time: 167
Bind time: 178
Check time: 683
Emit time: 363
Total TS time: 1391
Compile time: 1418.7361609999998

It's 250-300ms, or 15-20% of potential saves in compile time

@kitsonk
Copy link
Contributor

kitsonk commented Jul 27, 2020

Couple thoughts... I know we are eager to do this, but I think the refactor of the compiler is important (#6894). This should be even easier to implement when that is redone.

Second, we can't use it for the type checking without breaking a lot of code in the wild, especially code that doesn't use export type and import type plus const enum and potentially other code. (You want to run it with "isolatedModules": true so that the type checking is as close to just type stripping as possible). Honestly we wouldn't be able to do that until 2.0 IMO.

@bartlomieju
Copy link
Member Author

Couple thoughts... I know we are eager to do this, but I think the refactor of the compiler is important (#6894). This should be even easier to implement when that is redone.

I agree that compiler refactor is important, I don't suggest trying to hook SWC up to type checking process in this PR. Are you ok with landing change just for --no-check?

Second, we can't use it for the type checking without breaking a lot of code in the wild, especially code that doesn't use export type and import type plus const enum and potentially other code. (You want to run it with "isolatedModules": true so that the type checking is as close to just type stripping as possible). Honestly we wouldn't be able to do that until 2.0 IMO.

@kitsonk thanks for clear explanation, that sounds good to me.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 27, 2020

I agree that compiler refactor is important, I don't suggest trying to hook SWC up to type checking process in this PR. Are you ok with landing change just for --no-check?

Yeah, I hadn't actually looked at the changes... straight forwards and should be easy to integrate. 👍

Thinking more, we might want to help people make the transition. I know we have avoided it, but if we surfaced --isolated as an option for run and check, then people would be able to easily run their code in type check and get back errors that are more meaningful to what they need to fix in order to support just type stripping. It would make the introduction of the breaking change easier.

cli/swc_util.rs Show resolved Hide resolved
cli/tsc.rs Outdated Show resolved Hide resolved
@CLAassistant
Copy link

CLAassistant commented Jul 27, 2020

CLA assistant check
All committers have signed the CLA.

cli/tsc.rs Outdated Show resolved Hide resolved
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM - very nice

@bartlomieju bartlomieju merged commit c691713 into denoland:master Jul 28, 2020
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.

Strip TypeScript types using SWC
5 participants