microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.33k stars 12.54k forks source link

Project References and noEmitOnError #32651

Open nullxone opened 5 years ago

nullxone commented 5 years ago

Search Terms

project references noEmitOnError build flag

fast development feedback

babel/preset-typescript babel/plugin-transform-typescript

Suggestion

Project with error in it should not be marked as up to date.

Build flag should respect noEmitOnError preference as set by user in .tsconfig

I understand that current works-as-expected is that the build flag sets this to true. This is a feature request with suggestions on how to change that design.

Per Typescript Handbook - Project References - Caveats

if one of your out-of-date dependencies had a new error, you’d only see it once because a subsequent build would skip building the now up-to-date project

Suggestion: Project with error in it should not be marked as up to date

Use Cases

Fast development feedback. In my case, I'm running both tsc -wb and jest --watch side by side. I want my test cases to run even if my types aren't perfect.

Right now, I am forced to resolve ALL type errors before I can get any feedback. This makes it difficult to develop iteratively.

Moreover, what is not seen in this issue tracker here: A number of people are now running babel and typescript side by side. One of the reasons could very well be just this. There is no need to run babel for pure transpilation if typescript can emit with errors.

Examples

dev feedback; live test cases; live browser refresh; other use cases

Other people: https://github.com/microsoft/TypeScript/issues/29851 https://github.com/microsoft/TypeScript/issues/25600#issuecomment-450644121 https://github.com/microsoft/TypeScript/issues/25600#issuecomment-473389927 https://github.com/microsoft/TypeScript/issues/25600#issuecomment-482865289

Workarounds

Meanwhile, we can do this:

Checklist

My suggestion meets these guidelines:

RyanCavanaugh commented 5 years ago

Suggestion: Project with error in it should not be marked as up to date

What precisely does this mean? There is no "marking" of up-to-dateness; tsc -b simply looks at timestamps to see if outputs are newer than inputs.

A potential answer is to write the output file out, then set its timestamp to some time in the past, but this seems extremely bad in terms of confusing other tools.

nullxone commented 5 years ago

.tsbuildinfo file currently contains some per file cache ( { version, signature } ) and the dependency graph

a new key ( { hasError } ) should be added to the per file cache this means the file is invalid and must always be recompiled

when an error is found in a file:

a project is up to date when [ existing timestamp based condition ] AND [ there is no file with { hasError: true } ]

a file should be recompiled when [ existing signature based logic ] OR [ { hasError: true } for it or dependencies ] Edit: probably doesn't need recompile, but only errors output again, cache those too?


please help me understand:


i am unable to figure out the above from non-source-code-internet. i can try to make this more specific after understanding the above, will try the source next.

i suspect somewhere here is also the reason why tsc --watch is blazing fast ⚡️ but tsc -bw is quite slow

thank you for taking the time to consider this

zerkalica commented 4 years ago

Another case: Use project references in legacy project with broken types. tsc -p . still can build project, but tsc -b not.

berickson1 commented 4 years ago

Having noEmitOnError is also quite helpful for package upgrades & big refactors. Sometimes it's necessary to check behavioural semantics on a new package/changeset while working through type errors. (This is especially important if @types has a bug or is missing a changed type)

A couple questions to the typescript team (@RyanCavanaugh? Sorry for the mention) 1) Is this proposal something that's inline with the product roadmap? 2) Would including error information in .tsbuildinfo be sufficient to make this work, or are there other considerations 3) Beyond prioritization/resourcing, are there any known technical hurdles that would block adding this support?

Lack of this support is hurting developer productivity on our project. Ultimately we need to find a solution & moving off project references is a viable option if there's no chance that this would be supportable in the future. I expect that other large typescript projects would feel similar pain.

sheetalkamat commented 4 years ago

tsc -b is intended to build whole solution/ all projects you specify.. it also does not watch or keep track of anything except time stamps specified by input and output.. this results in fast check to decide if given project needs to be rebuilt..
The theory behind not building project if referenced project has errors, is that if there are errors in dependencies likely, there will be errors in the project itself so you are just increasing number of errors to report (emitting without reporting errors seems like it would be incorrect to do either) and resulting output is potentially not consumable either since it has errors that tsc detected. It means two things happen if we emit irrespective of errors: You are building many more projects which could be huge perf factor for big solutions. And it would probably be very difficult for user to diagnose the issues.

While there are some errors that could only affect in local projects, we have not yet drilled into what it means from build stand point for such errors to be present so we treat all errors same. Other issue with this approach is that not every project built by tsc -b needs to write tsbuildinfo, because only referenced projects have composite mandatory. The outer project might not have incremental set and then it poses another issue with how to detect easily if output stamps are upto date because there is not tsbuildinfo to read to see if this was emitted with or without errors and if project needs to be reconstructed to report errors

So even if this feature is enabled we would only enable them for project set with incremental flag. But its still not clear what the right approach is.. Is it ignoring specific errors, should those be configured by user or could it be any errors. Definitely should be opt in feature as well to avoid having every tsc -b user pay penalty for these extra builds..

tsc -p builds only the project and not its dependencies, It also doesnt check timestamps so it has its limitations as in any change to dependency would not be picked until dependency project is rebuilt.

We need more feedback on why this makes sense and what kind of scenarios people are looking when they want their referenced projects to be built if there are errors to come up with proposal on what this request is.

berickson1 commented 4 years ago

A few more details my usecase here, which I suspect are similar to the OP.

Here's a few scenarios that have come up in my group: 1) A developer is working on some new UI in a component, the code is 4 layers deep in project references from the entry point. They want to remove some old UI first, so they delete a bunch of code in the component render. They build some new placeholder UI, then save, hoping to view it in a browser. This fails to compile because of unused imports enforced by the compiler. They now have to to and comment out/remove a bunch of un-used imports to be able to view their code, even though it technically all runs. The same scenario can occur when refactoring a function [We could disable rules for no unused locals/no unused parameters, but we want to catch and warn on this early]. 2) A developer is upgrading a package that's used in a bunch of places across the app. The new version introduces a few typing errors (unknown impact). The developer wants to make sure that basic functionality works before investing in typing changes. Unfortunately, to make this compile, they're going to have to comment out, as any or fix a bunch of places in code to make the entire app compile 3) A developer is working on a large cross-package refactor. As they start to make changes, they introduce some type errors - however these don't affect the core use-case. They're now unable to test any of their core use-case changes until they've either commented out the "broken" code or addressed the specific type-errors.

Most of these cases are "in-between" states but multiple developers have had their common workflows slowed down by this limitation. We still want these errors to be surfaced/visible (and block CI/production builds). Still the cost of doing multiple re-builds is likely cheaper than an individual dealing with all the potential errors. On the happy path, there won't be any re-builds as everything would compiler error free.

I agree entirely about the opt-in behaviour, I think that the noEmitOnError flag could be utilized here potentially - although the default would have to change for some projects, which could be confusing. (I'd argue it's confusing now since it has no effect)

There's also the question of what .d.ts should be emitted on error, but I imagine that behaviour is already ratified under noEmitOnError handling.

trusktr commented 4 years ago

Sometimes we just want TS to get out of the way. F.e. we have a demo to show tomorrow. The JS code works fine, but recent refactoring (for example) still has type errors. We would like to run the demo regardless, to show other people for example, and get back to the errors later.

Using // @ts-ignore is not idea for this temporary situation, because human error can lead to forgetting to remove them, and that could lead to runtime errors later once the demo has gone to production.

ethangodt commented 3 years ago

This defaults to false, making it easier to work with TypeScript in a watch-like environment where you may want to see results of changes to your code in another environment before making sure all errors are resolved.

Why does this not hold true for multi-project systems? In a single-project system modules/files themselves serve as close analogs to projects in the multi-project system. As a convenience, errors in some modules don't take down the whole show (and even module types are still deduced despite errors). It seems that should be considered here.

patroza commented 3 years ago

I can only agree to what has been said before, I think the gist of it really is well put as "TypeScript sometimes needs to get out of the way". Project references are great, being able to emit even on error together with Project References is even greater.

leidegre commented 2 years ago

While iterating on work, I don't care that my files have errors in time. I'm moving stuff about and testing things. If TypeScript doesn't emit on the slightest error, we're back at having to go through 100s of compiler errors before we get any meaningful feedback from a refactor. This is something I've come to enjoy when working with JavaScript. It's not that I don't care about the correctness of my program, I just don't care right now...

I've been trying out project references but there are several caveats that make it easy to get it wrong. This is one of those things.

ds300 commented 2 years ago

I'd love a --build mode where files are emitted for the whole project regardless of type errors, but type checking stops at the package where the errors were met.

mifi commented 11 months ago

One workaround is to use a more tolerant transpiler like esbuild to output JS code, and use tsc only for the actual type checking.

Jamesernator commented 10 months ago

it also does not watch or keep track of anything except time stamps specified by input and output.. this results in fast check to decide if given project needs to be rebuilt..

Why is it not possible just to include something like lastBuildHadErrors: { thisProject: boolean, subprojectsWithErrors: string[] } in the tsbuildinfo? The presence of which would imply that a rebuild should be done regardless.

Yes this implies a full build again, but if you had errors this is what you would usually want anyway.

codethief commented 4 months ago

I'd love a --build mode where files are emitted for the whole project regardless of type errors, but type checking stops at the package where the errors were met.

Tbh, I'd still like type checking to continue as far as possible. If a coworker¹ just caused master to be red with a tiny type error (yes, we all know direct pushes to master are bad but we all know they happen), then type checks for all projects downstream of the erroneous code will not be type-checked anymore, i.e. feedback both in CI pipelines and on the command line will be non-existent / blocked by that one error. In other words, it will be impossible to ignore upstream errors.

¹) Of course by "coworker" I mean myself.

alecmev commented 4 months ago

Fixed in 5.6, just in case 😉

timdemeyer commented 3 months ago

Adding the following line in your .env-file worked for me (CRA-project): TSC_COMPILE_ON_ERROR=true