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

Not Type Checking TypeScript #5436

Closed
kitsonk opened this issue May 15, 2020 · 22 comments · Fixed by #6456
Closed

Not Type Checking TypeScript #5436

kitsonk opened this issue May 15, 2020 · 22 comments · Fixed by #6456
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)

Comments

@kitsonk
Copy link
Contributor

kitsonk commented May 15, 2020

Type checking TypeScript and transforming it to JavaScript will always be slower than just transforming to JavaScript. We have talked around the issue before in #3321 and #4323 but it makes sense to be clear about the need for this feature. It makes sense though to allow a user of Deno to be able to opt in or out of full type checking of TypeScript. Especially when other tools are in place (like an intelligent IDE) or there is fail fast prototyping going on, Deno forcing type checking only impedes the use of TypeScript, which is a sad thing.

We should have a "flaggable" mode where TypeScript files are not typed checked, but simply transformed/type stripped.

We currently use swc for deno doc and deno fmt. We should consider using swc to do the parsing and transform.

We should also figure out if type checking is on or off by default. There are arguments both ways. I personally was of the opinion that it should be on by default, but recent conversations have me leaning towards off by default, (though maybe on by default with deno test).

Another consideration is how we handle the cache when switching between modes, and ensure invalidation of the cache occurs in a way that meets expectations.

There is work in progress to move dependency analysis out of the compiler isolate into Rust (see #5029) which is really a pre-req before we can really land anything related to this.

@stefk
Copy link
Contributor

stefk commented May 15, 2020

I understand that skipping type checking is desirable or necessary in some cases, but doesn't a --no-type-check option solve the issue in every situation?

One of the goals of Deno is to promote a large standard library written in TypeScript and I would personally find it weird that by default, programs could make an incorrect use of it.

Having different opt-in/opt-out policies for different deno commands would be even more confusing IMO.

@kitsonk
Copy link
Contributor Author

kitsonk commented May 15, 2020

I am suggesting effectively this, just a question of it being --no-type-check or --type-check. Both have their advantages and disadvantages.

@DerDrodt
Copy link

What are the arguments for --type-check?

I think it would be confusing to claim to have first class TS support and not type check by default.

@slikts
Copy link

slikts commented May 15, 2020

Testing is one use case where type checking at "authoring time" instead of later can get in the way; prototyping is another. Not having a way to opt out of it is a significant limitation.

@kitsonk
Copy link
Contributor Author

kitsonk commented May 15, 2020

What are the arguments for --type-check?

Again, I was originally against this idea. But many developers use IDEs to develop JavaScript or TypeScript. Those editors enforce types as you edit the code. Therefore Deno type checking is effectively redundant. Type checking will always have overhead to starting up Deno. Currently it is about 10x just using JavaScript, even if it is just 1 TypeScript file. If 90% of the time you invoke Deno, you already have a safety net of an editor, then 90% of the time you are "wasting time". Over the long term it could be argued that erodes adoption of TypeScript. Part of the reason many of us got into JavaScript in the first place was it was easy to make changes rapidly, versus the old way of having to wait for a compiler build.

@stefk
Copy link
Contributor

stefk commented May 15, 2020

Those editors enforce types as you edit the code.

That doesn't mean deno should be unsafe by default.

Aliasing deno --no-type-check to something shorter is convenient enough for prototyping and IDE integration and doesn't contradict the branding of this project.

@DerDrodt
Copy link

But many developers use IDEs to develop JavaScript or TypeScript. Those editors enforce types as you edit the code. Therefore Deno type checking is effectively redundant.

Sure, but that doesn't catch all errors necessarily. If I edit a type in a large project it might lead to errors in a different file that is not open and I might not see this error. This might lead to errors and negates the point of TS.

I am aware of the overhead and that is a problem. Let's see if #5432 helps. If it does, that might be enough

@DerDrodt
Copy link

Also, if a user is sure that the IDE checking is enough, they could still use the --no-type-check option. If this is documented, that should be enough.

Doing it the other way would lead to users oblivious that no checking occurs.

@Ciantic
Copy link

Ciantic commented May 15, 2020

Edit I think here Why does the compile and type checking need to be synchronously? Can't it emit the JS immediately and run it. Then do type checking delayedly in separate thread?

I think the "no-type-check" option should be an option. Maybe even recommended option e.g.

deno --production script.ts

If it were named --production it could include other crazy hacks to speed things up, that you don't want in development.

If you run without the production flag it could include development time improvements: type checking in separate thread, emit and run as fast as possible so you can see changes immediately, hot module reloading etc.

Note Why --production and not --development? Think it like this, how often do you run the development mode vs production mode from cli? Answer is that default should be development mode because we developers who use the cli do that all the time.

In development mode the type checking is critical but in production mode you probably want just speed, and production mode is ran less often: thus --production switch is an option.

@DerDrodt
Copy link

I like the idea of an option for speed hacks, but it has to be clear to the user what is dropped for speed. If they don't know that type-checking is dropped, they might use it always and be frustrated when errors aren't caught.

And I know it was just an example, but --production sounds like it might take longer, because more optimization is done.

@David-Else
Copy link

Basically I am just saying what the majority before have said in this thread. I think a --no-type-check option is a great idea, but having type checking off by default is a terrible idea.

What is the problem for the user to just add --no-type-check if they want? They can even alias it, there is no loss to them whatsoever. Deno is meant to be 'all about' TypeScript!

@nayeemrmn
Copy link
Collaborator

Another consideration is how we handle the cache when switching between modes, and ensure invalidation of the cache occurs in a way that meets expectations.

This issue already exists with different tsconfigs I believe. Needs to be solved either way.

@Nimelrian
Copy link

Testing is one use case where type checking at "authoring time" instead of later can get in the way;

TS 3.9 introduces @ts-expect-error directives which are designed to solve exactly this problem: https://devblogs.microsoft.com/typescript/announcing-typescript-3-9-beta/#ts-expect-error-comments

@Bnaya
Copy link

Bnaya commented May 15, 2020

Worth noting:
That's kinda the approach of typescript support in babel.
Leave the type check to tsc (Or other tools), just strip away types and turn the code into ES code
They even take it another step forward, you need to write your code with isolatedModules: true, which guarantee that you can process every file separately and the types have no effect on the emitted code.
You even don't need tsconfig to transform typescript that way
I believe that also https://romejs.dev/ supports typescript in a similar manner

@bartlomieju
Copy link
Member

bartlomieju commented May 15, 2020

We currently use swc for deno doc and deno fmt. We should consider using swc to do the parsing and transform.

Blocked by swc-project/swc#769

Another consideration is how we handle the cache when switching between modes, and ensure invalidation of the cache occurs in a way that meets expectations.

@kitsonk would there be any difference in compiled source code depending if type checking is on/off? Once "no type check" option is working it'd err to always use the same method for transpiling TS to JS (ie. SWC). TSC would only be used to perform the type check, but without emit step.

@kitsonk
Copy link
Contributor Author

kitsonk commented May 15, 2020

would there be any difference in compiled source code depending if type checking is on/off?

There should be no type directed emits, true... but I am just thinking about transitory dependencies, where if we aren't careful, we could either not detect errors. Right now, type checking is the barrier to if something populates that cache, it won't be with this option. I think we just need to think about it.

@DerDrodt
Copy link

swc-project/swc#769 is now resolved.

@bartlomieju bartlomieju added cli related to cli/ dir feat new feature (which has been agreed to/accepted) labels May 18, 2020
@bartlomieju
Copy link
Member

With #5029 landed this issue is now feasible. Unfortunately we'd still need to use TS compiler worker instead of SWC, but I guess it should be a net gain anyway. @kitsonk how do we tackle this problem, could we just use ts.transpileModule() instead of ts.createProgram() when --no-type-check flag is passed?

@kitsonk
Copy link
Contributor Author

kitsonk commented May 18, 2020

@bartlomieju yup... the runtime compiler API for transpileOnly should be close... important thing is that there are tconfig.json options that effect the emit (specifically things like experimentalDecorators which people use)

@nayeemrmn
Copy link
Collaborator

The CLI design of this should keep #5460 in mind:

--skip-type-check=<all|none>
->
--skip-type-check=<all|remote|none|<blacklist>...>

Regardless of how fast compilation gets, #5460 will be ultimately necessary to support breaking changes in TS, as well as the existing issue of varying strictness.

@kitsonk
Copy link
Contributor Author

kitsonk commented Jun 24, 2020

I've started work on this and have it working in a branch, except ran into a major challenge... With TypeScript constructs like:

export { AnInterface } from "mod.ts";

Because the transpiler isn't looking at any sort of type information, it doesn't know that AnInterface should be erased. It seems like other transpilers that just erase types have this problem as well, and one of the main motivations for the introduction of import type and export type. So the challenge is we can deliver this, but the problem is some existing Deno TypeScript code will break, as the above needs to become the following to be transpiled properly:

export type { AnInterface } from "mod.ts";

@vwkd
Copy link
Contributor

vwkd commented Oct 16, 2020

I'm sad that the opt-out --no-check was chosen instead of opt-in --check (or something similar). This will be a default option for most people now, since types are already checked by the IDE.

Also as already mentioned by other people, perfectly fine JavaScript with wrong types won't run anymore by default. People here say this is being "safe", but this has nothing to do with safety. Safety is guaranteed by the sandbox and permission flags. Not running perfectly fine code by default because of what is essentially "incorrect comments" is just wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)
Projects
None yet
Development

Successfully merging a pull request may close this issue.