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

Incremental --build, then delete generated js file, then another incremental --build does not recreate js file #30602

Open
mmorearty opened this issue Mar 26, 2019 · 40 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: --incremental The issue relates to incremental compilation Suggestion An idea for TypeScript

Comments

@mmorearty
Copy link
Contributor

TypeScript Version: 3.4.0-dev.20190326

Search Terms:

incremental

Code

tsconfig.json:

{
  "compilerOptions": {
    "incremental": true
  }
}

x.ts:

console.log("hello");

Expected behavior:

  • $ tsc --build
  • x.js has been generated
  • $ rm x.js
  • $ tsc --build

Expected: x.js has been regenerated

Actual behavior:

x.js still does not exist

Related Issues:

Did not find any.

@sheetalkamat
Copy link
Member

sheetalkamat commented Mar 26, 2019

Hmm the root cause of this is that in general our incremental build (BuilderProgram) isn't equipped to deal with missing output file as indication to build but changed file as more like it. Need to think about what we can do here. Till then the workaround is to run --clean and then build or add comment or something into the file whose output needs to be generated.
Note that this didn't originate with just --incremental, this would be same behavior when you followed same pattern but were running tsc --build --watch from first step (even without --incremental in compiler options)

FYI @RyanCavanaugh @DanielRosenwasser

@sheetalkamat sheetalkamat added the Bug A bug in TypeScript label Mar 26, 2019
@irakliy81
Copy link

A separate, but possibly related issue: deleting source .ts files does not remove the related .js files. Right now, to address this, I clean the output directory before every compilation - but this makes it impossible for me to use the --incremental flag.

@sheetalkamat
Copy link
Member

@irakliy81 that's totally out of scope and does not work even without --incremental. Build cannot delete any output file unless you specify clean (which will also delete output files of config specified at that moment)

@sdwvit
Copy link

sdwvit commented Apr 2, 2019

👍
Also experienced this bug:
We had a piece of code which imported a recently deleted file error in one of the dev-branches, and since we use caching of buildinfo in CI, parallel branch builds started to experience this missing file too. However, the error was different than just 'module not found', it was another type error from patient-0 branch.

@irakliy81
Copy link

@sheetalkamat - yes, I wouldn't expect it to work without --incremental flag since in that case there is no info stored about the previous build. But with the --incremental flag, it should be possible to remove output files if the source files have been removed.

A separate question: how do you specify clean? It doesn't seem to be a valid compiler option.

@milesj
Copy link

milesj commented Apr 2, 2019

@irakliy81 --clean

@irakliy81
Copy link

@milesj - thanks! But it doesn't seem to be listed here: https://www.typescriptlang.org/docs/handbook/compiler-options.html and if I try to set it in VS Code I get "unknown compiler option" error. Am I missing something?

@milesj
Copy link

milesj commented Apr 2, 2019

It's part of project references. Not sure if it's available outside of that, nor as a compiler option. https://www.typescriptlang.org/docs/handbook/project-references.html

@sdwvit
Copy link

sdwvit commented Apr 15, 2019

any plans on fixing this? cc @sheetalkamat

it kills the point of caching build info in CI

@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.6.0 milestone Apr 15, 2019
@sheetalkamat sheetalkamat added the Domain: --incremental The issue relates to incremental compilation label Apr 15, 2019
@RyanCavanaugh
Copy link
Member

For clarity: What/who is deleting the output files and why?

@sheetalkamat
Copy link
Member

There are multiple workarounds for this issue.

  1. Run tsc --b --clean
  2. Delete .tsbuildinfo file as well

Its not clear why only .js file is deleted

@haggholm
Copy link

@RyanCavanaugh

For clarity: What/who is deleting the output files and why?

I’ve had this happen when an unrelated clean script deleted output directories including, but not necessarily limited to, Typescript output. I mean, normally I assume that if I delete output and re-run a build tool, the output will be recreated; that’s true of all build tools I’m used to, for web development or otherwise, except, in this case, Typescript.

@RyanCavanaugh
Copy link
Member

Your expectations are fine, we're just trying to figure out if the scenario is common enough that e.g. people who have 1,200 build outputs should be paying the cost on every single operation for TS to check if all those outputs still exist. Can the "unrelated clean script" just delete the .tsbuildinfo as well?

@sdwvit
Copy link

sdwvit commented Apr 17, 2019

Well yes, we should check files existence on our side in CI. Which is fine, but as already mentioned, it's against expectations. Also, if only 1 file is deleted accidentally, deleting whole buildinfo file is a bit overkill

@b4dnewz
Copy link

b4dnewz commented May 8, 2019

I've noticed also tsc -b --clean only cleans the output files without rebuilding them, is this the desired behavior? Since the command is tsc -b --clean I assumed it will build also.

@matthiasg
Copy link

tsbuildinfo also does not get invalidated when tsconfig.json changes. Thus code generated e.g with target: es2017 does not get rebuilt when the target gets changed.

@glen-84
Copy link

glen-84 commented May 13, 2019

@matthiasg See #31118.

@simonbuchan
Copy link

Your expectations are fine, we're just trying to figure out if the scenario is common enough that e.g. people who have 1,200 build outputs should be paying the cost on every single operation for TS to check if all those outputs still exist. Can the "unrelated clean script" just delete the .tsbuildinfo as well?

This is reasonable, but I think the 90% case is worth covering here: something like if outDir doesn't exist at all (or maybe the first output file?) does not exist when starting a new tsc --incremental, assume it's been externally deleted and ignore .tsbuildinfo?

Another workaround / fix for people who need these external tools is to simply place the .tsbuildinfo in the outDir so it gets cleaned too.

@sdwvit
Copy link

sdwvit commented May 17, 2019

@simonbuchan

so it gets cleaned too.

killing the whole purpose here

@molant
Copy link

molant commented May 29, 2019

The --incremental feature is really new and I haven't found a lot of information on how it works internally.

Is it fair to assume that the TS compiler will/could generate the output faster when it has a .tsbuildinfo file even if there isn't any output files?

If the build time is going to be the same as a fully clean one with no .tsbuildinfo, then storing them in git does not make any sense.
On the other side, if the build time should be indeed faster then I think the scenario is compelling enough to investigate how much of cost it will have and if it is worth fixing this. Any project that runs CI or a developer doing a fresh checkout will benefit from this and save precious time.

@sdwvit
Copy link

sdwvit commented Jun 1, 2019

Here is an usecase when incremental build breaks developer experience:

We have a file that reads all A/B experiment configuration files (*.ts) from folder using glob.
These files are compiled using tsc (experiments/*.ts in ts-config).

When developer deletes experiment file.ts they expect compiled file to be removed too, otherwise A/B test stays active. Only manually checking and deleting build info helps.

Hope it’s clear.

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript and removed Bug A bug in TypeScript labels Jul 18, 2019
@regevbr
Copy link

regevbr commented Mar 16, 2020

Any news on a fix?

@cyrilchapon
Copy link

cyrilchapon commented Apr 10, 2020

I so much agree with @simonbuchan

and it's an unnecessary small cut to have to remember to delete the build cache too compared to other tools

I just lost 1 hour with that.

Adopted new "project" strategy (for a src + test simple project); built; cleaned the whole dist; wondered half an hour why subsequent tsc -b didn't produce any output at all; spent more half an hour tried to digg and remove incremental tag (which I hate in general, stateless in mind); got "incremental option may not be disabled for composite projects"; got stuck.

@mkalam-alami
Copy link

mkalam-alami commented Oct 18, 2020

I would have assumed that --incremental stores and checks the signatures of both the definitions and the generated sources.

In the current state there's basically a secret contract saying that nobody but tsc can touch the build output. It's not what I would assume from something simply advertised as an incremental build. And apart from subtle phrasing in the 3.4 release notes I don't think it's clear from the docs either that only types definitions are checked.

I too would be for making --incremental check the generated sources by default ; if this comes to an unreasonable cost to certain projects, having an additional, clearly labelled setting (eg. --incrementalTypeChecking) to restore current behavior would probably be enough to make everyone happy.

@wojpawlik
Copy link

I enabled composite to use references, and I lost an hour tracking down this heisenbug.

@trusktr
Copy link
Contributor

trusktr commented Jan 18, 2021

This is a bit confusing.

  • If we don't run with --build, then the tsc program completes very fast, with no clues as to why, as if everything is fine.
    • Running just tsc should give a helpful message. Or, even better, it should run the project in --build mode.
    • Perhaps we need an option in tsconfig.json that can make the --build option not required.
  • If we run tsc --clean, it fails, without a helpful message. There's no obvious way to know that it should be tsc --build --clean

It's just not intuitive. I think many people spend many hours (if not days) fiddling around with this stuff. It is too time consuming.

@Toxaris
Copy link

Toxaris commented May 26, 2021

Should --incremental assume that some files may have been deleted from disk?

There's a real perf cost to doing this and it's unclear how you would often get in this state in a way where you couldn't automate yourself back into a good state (e.g., if a script is deleting some random subset of JS files, that script should also delete .tsbuildinfo and force a rebuild).

Experience report:

We are running into this in the context of converting one project in a monorepo from JS to TS. So we used to have an index.js in one of our libraries. I converted it to index.ts and added a build script in the package.json. All contributors install all dependencies and run all build scripts anyway as part of the monorepo tooling, so I assumed that they would not see any difference. But when contributors switch back and forth between a branch that has my changes and a branch that does not have my changes, git ends up deleting the index.js but not the tsconfig.tsbuildinfo. I think because the other branch has index.js tracked by git but not tsconfig.tsbuildinfo. So the end result of switching branches back and forth is that the build is broken and can only be repaired by cleaning.

I saw a couple of colleagues complaining in Slack and was affected myself, so I guess that more colleagues were affected but didn't complain. I estimate we lost about 10 person hours because of this, plus some confidence in typescript. (Personally I'm still confident in typescript, but some colleagues had a first bad interaction now).

@lorenzodallavecchia
Copy link

We are having problems similar to @Toxaris
Sometimes, tsc stop recompiling some files, assuming that they are unchanged. The only way to recover is to force a change in the affected file or delete the .tsbuildinfo file.
In our case too, it seems to happen when switching branches or rebasing.

I have also tried looking into the .tsbuildinfo contents, but I don't know how to interpret them.
Is there a strategy we can use to pin down the problem?

@MatthiasKunnen
Copy link

Should a subsequent build delete files that were generated by a prior build but no longer have a source?

Yes, yes please. In our use case we compile migration files to a directory and all files in this directory are evaluated when the migrations run. We have had issues where checking out to feature branch A, testing the feature, reverting the migrations and checking out to feature B, preserves the migrations of feature A. This harms developer experience because our developers need to keep track of any compiled files that might linger from other branches.

IMO an incremental build should take the output folder from the current state to the desired state while using .tsbuildinfo to speed up the process and only build when necessary. Running an incremental build should produce the same result as removing the output folder and running a normal build

@WestonThayer
Copy link

For "should a subsequent built delete files that were generated by a prior build but no longer have a source?", #16057 is closely related (with several links to libs that patch in this behavior).

@ronyeh
Copy link

ronyeh commented Jan 13, 2022

Running an incremental build should produce the same result as removing the output folder and running a normal build

I strongly agree with this statement!

At the moment, incremental builds provide maximum performance but with the possibility of "incorrect" or unexpected output (i.e., it does not build a file we expected it to build).

I'd rather the incremental build always be 100% correct. It should still be faster than a full clean and build, since it can skip the compilation of some modules.

Maybe we need a different flag that guarantees correctness, but with a slight performance penalty?

"compilerOptions": {
    ....
    "incrementalSafe": true
}

@wojpawlik
Copy link

Safety should be opt-out, not opt-in.

@ronyeh
Copy link

ronyeh commented Jan 13, 2022

Safety should be opt-out, not opt-in.

I agree, but since this issue has been open 2 years, I was hoping to suggest something that might be accepted, so those of us who care can use the safer option.

Ideally, the "incremental" : true should guarantee correct output, and we can have more advanced options to adjust the build speed / safety if desired, like:

"compilerOptions": {
    ....
    "incremental": {
        "checkMissingOutputFiles": true,  // output *.js file disappears => rebuild it on the next pass. default: true.
        "checkMissingSourceFiles": true,  // source *.ts file disappears => delete the associated output file on the next pass. default: false.
        "checkModifiedOutputFiles": true, // output *.js file modified => rebuild it on the next pass. default: true.
        "checkModifiedSourceFiles": true, // source *.ts file modified => rebuild the output file on the next pass. default: true. This flag can obviously be omitted, as it's implied by "incremental compilation". If we set it to false, incremental wouldn't work! :-)
    }
}

The default values were chosen because they are safest:

  • checkMissingOutputFiles => true
  • checkMissingSourceFiles => false
  • checkModifiedOutputFiles => true

@dylanlan
Copy link

I just got hit by an issue from a renamed source file, where the old output JS file ended up getting executed instead, and had a runtime error.

I think my case was specifically the Should a subsequent build delete files that were generated by a prior build but no longer have a source? issue. But it sounds like the general issue at hand is from trying to keep the source and output files in sync (kind of similar to cache invalidation?)

I'm a fan of having configuration options for correctness / speed tradeoffs if possible.
In my opinion, it should guarantee correctness by default, but configuration options to toggle would be awesome.

It sounds like there's multiple ways for the compiled output directory to get out of sync with the source, where incremental builds won't re-sync. This makes incremental builds less valuable for me, by not being able to trust whether the output directory is in the expected state or requires some manual inspection / fixing.

My understanding of some current workaround options are:
1. Manually delete the outDir & .tsbuildinfo when I run into an error (my current approach, but might be non-obvious if an error is related or not)
2. Write my own custom logic to identify if the outDir has gotten out of sync (seems fairly complicated to me)
3. Always delete the outDir & .tsbuildinfo before incremental compilation (removes most of the benefit of incremental)

I'm curious if there's any other good workarounds!

Regarding precedents for deletion, I'm familiar with some related config flags in a couple other tools.
But they're specifically for source to target directory sync, rather than incremental compilation:

@vtgn
Copy link

vtgn commented Sep 5, 2022

Hi,
I just found the same problem on my NestJS project and lost more than one hour to find the cause of this.
Here is the process with a very simple typescript project to reproduce it easily :

  1. Create a "test" folder, go inside on a shell, and type the command : npm i typescript --save-dev (very simple "package.json" and "package-lock.json" files are created, defining this only dev dependency)".
  2. Type the command: npx tsc --init (a "tsconfig.json" is created).
  3. Modify the "tsconfig.json" contents with this:
    { "compilerOptions": { "module": "commonjs", "baseUrl": "./src", "rootDir": "./src", "outDir": "./dist", "target": "es2017", "incremental": true } }
  4. Create a "src" subfolder in your "test" folder, owning a simple "test.ts" file defining a simple class:
    export class MyClass {}
  5. Build the project by typing in the "test" folder the command: npx tsc -p tsconfig.json
  6. The "test/dist" folder is created owning only one file "myfile.js" defining the MyClass class. But a "tsconfig.tsbuildinfo" file has also been created NOT into the "dist" folder like the documentation explains, but in the "test" folder at the same level than the "tsconfig.json" file.
  7. Delete the "dist" folder and build the project again with the same command than the step 5 above.
  8. The "dist" folder is NOT generated this time, and the tsc command returns no error (code returned is 0).

IMPORTANT: NestJS set this "incremental" property to "true" by default in every new NestJS project. This is how I got the problem. For several days I got no problem: the "tsconfig.tsbuildinfo" file was generated inside the "dist" folder. But at a certain moment, I should have set a bad config because I did several configuration tests, and the "tsconfig.tsbuildinfo" has been generated in the main folder of the project as in the above example, causing the problem.

The only bypass I see:

  1. Delete the "tsconfig.tsbuildinfo" file if it is generated inside the main project folder, instead of inside the generated "dist" folder.
  2. Or simply unset the "incremental" option, or set it to false.

Typescript version: 4.8.2
OS: Windows 10
Shell: Command Prompt (DOS)

Regards.

@jameslnewell
Copy link

Should --incremental assume that some files may have been deleted from disk?

My use case for having the --incremental flag check the existence of "outputted" files on disk is that I have multiple tools building into the same folder and I would like to delete the build folder between builds.

e.g.

{
  "scripts": {
    "clean": "rm -rf dist",
    "build": "npm run clean && npm run build:js && npm run build:types && npm run build:files",
    "build:js": "swc src --outdir dist",
    "build:types": "tsc --declaration --emitDeclarationOnly",
    "build:files": "cpx src dist --ignore *.ts"
  }
}

The reason I would like to clean between builds is because files might have been deleted since the last build and not removing them increases the size of the build artifact, is confusing to debug and can result in unexpected consequences during execution - e.g. move foo.ts to foo/index.ts and nodejs will still require the old generated artifact.

I was hoping that --incremental would save the compiler from a notable amount of work, even if it did mean writing the output files again.

Similar to other posters it took me a while to understand what was happening and finally end up here 😄

@dsogari
Copy link

dsogari commented Mar 9, 2024

One scenario in which output files may be removed is when publishing a package that uses "declarationMap": true. In that case, the generated .d.ts.map files should not be included in the published package, since the original source will not (in general) be available.

And since package.json does not have a syntax to exlucde files from the tarball when using the "files" field, I think TypeScript should consider either implementing this feature or provide an option to generate declaration map files in a specific folder (e.g., "declarationMapDir").

@bcameron1231
Copy link

bcameron1231 commented Apr 12, 2024

I'm seeing similar behavior when running a build doesn't output a new build (I use incremental as well). Deleting the tsbuildinfo does fix this. However I found that the issue I had was using "emitDecoratorMetadata" in the tsconfig for our decorators. This for some reason prevents 'build' from re-generating the build output correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: --incremental The issue relates to incremental compilation Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests