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

tsc --watch recompiles unmodified dependencies #3184

Closed
breck7 opened this issue May 15, 2015 · 7 comments
Closed

tsc --watch recompiles unmodified dependencies #3184

breck7 opened this issue May 15, 2015 · 7 comments
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead

Comments

@breck7
Copy link

breck7 commented May 15, 2015

Say you have 2 files:

car.ts:

export class Car {
    public beep(): void {}
}

truck.ts:

import Car = require("./car");
class Truck extends Car.Car {}
new Truck().beep();
// changing this file should not recompile car.ts

Now you run "tsc --watch", wait for the two files to compile to js files, then edit and save truck.ts.

Expected: truck.ts is recompiled, car.ts is not.

Actual: both files are recompiled and truck.js and car.js are written to disk even though output of console states "message TS6032: File change detected. Starting incremental compilation..."

Tested on: "Version 1.5.0-beta" on Windows 10 Build 10074

@CyrusNajmabadi
Copy link
Contributor

It is by design that we recompile all files. Files may be affected by changes in other files. As such, we have to re-emit them to ensure that their contents are appropriately up to date.

We have discussed a small change where we at least see if what we recompiled is the same as what's on disk. If so, we could skip the file write (leaving timestamps unchanged). However, the rest of the compilation pipeline will still have to stay the same in order to prevent invalid code from staying on disk.

@breck7
Copy link
Author

breck7 commented May 15, 2015

Sorry @CyrusNajmabadi, I left out an important detail. The command I run is "tsc --watch --noResolve".

The behavior we are trying to get is only recompiling the edited file to reduce our save-build times from 15 seconds to 1.5.

@breck7
Copy link
Author

breck7 commented May 15, 2015

We are trying to replicate the atom-typescript compile-file-on-save behavior to Visual Studio Code through compile options alone, since plugins are not yet supported.

@CyrusNajmabadi
Copy link
Contributor

@breck7 --noResolve is not really relevant here. Or, at least, it won't do what you think it does.

--noResolve means that we do not consider /// references when trying to find which files to include int he compilation. instead, we only use the files you actually explicitly listed to the compiler.

However, even when we recompile we still use all the files and will recompile them all. We have to, as mentioned before, for correctness.

@CyrusNajmabadi
Copy link
Contributor

We are trying to replicate the atom-typescript compile-file-on-save behavior to Visual Studio Code

Compile-on-save still needs to recompile all files. If you do not do this, then you may end up not updating a necessary file, and the user will end up with broken code. This is what we do in VisualStudio. Here's our code for it:

private async void HandleFileSave()
{
    this.AssertIsInitialThread();

    // Cancel previous task if needed
    cancellationTokenSource.Cancel();
    cancellationTokenSource = new CancellationTokenSource();

    this.userNotificationService.ShowStatusBar(string.Empty);

    var cancellationToken = cancellationTokenSource.Token;

    var document = this.textView.TextBuffer.GetRelatedDocuments().FirstOrDefault();

    ScriptContext scriptContext;
    if (document == null ||
        !this.taskHandler.ScriptContextProvider.TryGetScriptContext(document.Project.Id, out scriptContext) ||
        !this.IsCompileOnSaveEnabledForFile(scriptContext))
    {
        return;
    }

    // First, save all other dirty files.  That way what we emit to disk is valid wrt what
    // TS data is on disk.  Otherwise, we'll emit to disk based on what we have in-memory
    // in VS.
    fileSaveMonitor.SaveDirtyFiles();

    try
    {
        this.userNotificationService.ShowStatusBar(LanguageServiceStrings.OutputGenerationStarted);

        // First, emit the file the user actually saved.

        // Call ConfigureAwait(true) to be clear that we intend to perform further operations
        // on the UI thread.
        var outputFiles = new List<OutputFile>();
        if (!await TryEmitAsync(document, outputFiles, cancellationToken).ConfigureAwait(true) ||
            !await TrySaveAsync(outputFiles, cancellationToken).ConfigureAwait(true))
        {
            return;
        }

        this.AssertIsInitialThread();

        // If the user has specified --out, then we're done at this point, having written
        // out the single output file.
        if (string.IsNullOrWhiteSpace(scriptContext.CompilationSettings.Out))
        {
            // Let the user know the first file saved, and we're moving on to the rest.
            var text = string.Format(LanguageServiceStrings.OutputGenerationFirstFileSucceeded,
                GetNameAndParentDirectory(document.FilePath));
            this.userNotificationService.ShowStatusBar(text);

            if (!await TryEmitAndSaveOtherDocumentsAsync(document, cancellationToken).ConfigureAwait(true))
            {
                return;
            }
        }

        // We're done, let the user know that all output has been generated.
        this.userNotificationService.ShowStatusBar(LanguageServiceStrings.OutputGenerationSucceeded);
    }
    catch (OperationCanceledException e)
    {
        if (e.CancellationToken == cancellationToken)
        {
            return;
        }

        throw;
    }
}

private async Task<bool> TryEmitAndSaveOtherDocumentsAsync(Document document, CancellationToken cancellationToken)
{
    // Otherwise we want to emit all files.  This is necessary so that changes in one 
    // file are properly propagated to affected files.  For example, if enum constant 
    // values change in the file being saved, these value changes need to propagate out to all other files.

    var outputFiles = new List<OutputFile>();
    foreach (var sibling in document.Project.Documents)
    {
        cancellationToken.ThrowIfCancellationRequested();
        if (sibling != document)
        {
            if (!await TryEmitAsync(sibling, outputFiles, cancellationToken).ConfigureAwait(true))
            {
                return false;
            }
        }
    }

    return await TrySaveAsync(outputFiles, cancellationToken).ConfigureAwait(true);
}

private async Task<bool> TryEmitAsync(Document document, List<OutputFile> outputFiles, CancellationToken cancellationToken)
{
    this.AssertIsInitialThread();
    cancellationToken.ThrowIfCancellationRequested();

    var emitOutput = await taskHandler.PerformSemanticFeatureOperationAsync(document.Project, TaskPriority.Normal, cancellationToken,
        (proxy, cache, context) => proxy.GetEmitOutput(cache, context, document, cancellationToken)).ConfigureAwait(true);

    this.AssertIsInitialThread();

    if (emitOutput == null)
    {
        // Can get null if the call outright failed (in which case we'll have reported a
        // Watson exception).
        return false;
    }

    if (emitOutput.EmitSkipped)
    {
        // Emitting was skipped due to errors, report a message to the user and do not
        // proceed any further.
        this.userNotificationService.ShowStatusBar(LanguageServiceStrings.OutputGenerationSkipped);
        return false;
    }

    outputFiles.AddRange(emitOutput.OutputFiles);
    return true;
}

private async Task<bool> TrySaveAsync(List<OutputFile> outputFiles, CancellationToken cancellationToken)
{
    this.AssertIsInitialThread();
    cancellationToken.ThrowIfCancellationRequested();

    var changedOutputFiles = new List<OutputFile>();
    foreach (var outputFile in outputFiles)
    {
        if (await IsChangedAsync(outputFile).ConfigureAwait(true))
        {
            changedOutputFiles.Add(outputFile);
        }
    }

    outputFiles = changedOutputFiles;

    var fileNames = outputFiles.Select(f => f.Name).ToArray();

    uint querySaveResult;
    if (ErrorHandler.Failed(queryEditSave.QuerySaveFiles(0, fileNames.Length, fileNames, null, null, out querySaveResult)) ||
        querySaveResult != (uint)tagVSQuerySaveResult.QSR_SaveOK)
    {
        // user was not ok with us writing to any of these files. Stop further work.
        this.userNotificationService.ShowStatusBar(LanguageServiceStrings.OutputGenerationCanceled);
        return false;
    }

    foreach (var outputFile in outputFiles)
    {
        cancellationToken.ThrowIfCancellationRequested();
        if (!await TrySaveAsync(outputFile, cancellationToken).ConfigureAwait(true))
        {
            return false;
        }
    }

    return true;
}

private async Task<bool> IsChangedAsync(OutputFile outputFile)
{
    // Use a defaultValue of true so that if any IO errors occur, we will think the file
    // is changed and will try to rewrite it.
    var isChanged = await FileHelpers.PerformIOAsync<bool>(async () =>
    {
        var directory = Path.GetDirectoryName(outputFile.Name);
        EnsureDirectory(directory);

        using (var fileStream = File.OpenRead(outputFile.Name))
        using (var reader = new StreamReader(fileStream, detectEncodingFromByteOrderMarks: true))
        {
            var contents = await reader.ReadToEndAsync().ConfigureAwait(true);
            this.AssertIsInitialThread();
            return contents != outputFile.Text;
        }
    }, defaultValue: true).ConfigureAwait(true);

    return isChanged;
}

private async Task<bool> TrySaveAsync(OutputFile outputFile, CancellationToken cancellationToken)
{
    this.AssertIsInitialThread();

    var saved = await FileHelpers.PerformIOAsync(async () =>
    {
        var directory = Path.GetDirectoryName(outputFile.Name);
        EnsureDirectory(directory);

        using (var fileStream = File.Create(outputFile.Name))
        using (var writer = new StreamWriter(fileStream, new UTF8Encoding(encoderShouldEmitUTF8Identifier: outputFile.WriteByteOrderMark)))
        {
            await writer.WriteAsync(outputFile.Text).ConfigureAwait(true);
        }

        this.AssertIsInitialThread();
        return true;
    }, defaultValue: false).ConfigureAwait(true);

    if (!saved)
    {
        // Let the user know that saving the file to disk failed for some reason. 
        // TODO(cyrusn): Should we instead pop up an dialog to let them know what the actual
        // error was?
        var text = string.Format(LanguageServiceStrings.OutputGenerationSaveFailed, GetNameAndParentDirectory(outputFile.Name));
        this.userNotificationService.ShowStatusBar(text);
    }

    return saved;
}

@DanielRosenwasser
Copy link
Member

For detail, #3122 is the issue where we discussed --noResolve with @breck7.

@mhegazy mhegazy added the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label May 18, 2015
@mhegazy
Copy link
Contributor

mhegazy commented May 18, 2015

I have commented on #3122. as @CyrusNajmabadi this is the expected behavior.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead
Projects
None yet
Development

No branches or pull requests

4 participants