GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Api for creating program in watch mode and using builder to get incremental emit/semantic diagnostics by sheetalkamat · Pull Request #20234 · microsoft/TypeScript

Conversation

@sheetalkamat

@sheetalkamat Sheetal Nandi (sheetalkamat) commented Nov 23, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Example usage of the watch API as well as builder api is in jrieken/gulp-tsb#74
Example usage of the watch API and builder api to get semantic diagnostics only is in s-panferov/awesome-typescript-loader#519

The below api is to allow caller to create a program in watch mode that will watch the changes in files/options etc and update it. As a result, the emit happens only on the affected files incrementally and semantic diagnostics are cached as well resulting in getting new errors only for the affected files. (note the program reports diagnostics for whole program but recalculates semantic diagnostics for affected files)

    type DiagnosticReporter = (diagnostic: Diagnostic) => void;
    /**
     * Create the watched program for config file
     */
    function createWatchOfConfigFile(configFileName: string, optionsToExtend?: CompilerOptions, system?: System, reportDiagnostic?: DiagnosticReporter): WatchOfConfigFile<Program>;
    /**
     * Create the watched program for root files and compiler options
     */
    function createWatchOfFilesAndCompilerOptions(rootFiles: string[], options: CompilerOptions, system?: System, reportDiagnostic?: DiagnosticReporter): WatchOfFilesAndCompilerOptions<Program>;

The system and reportDiagnostics if not provided defaults to using ts.sys and writing diagnostics to using sys.write. This is optional parameter to allow callers of the api to provide own implementation of how to read/write files

The returned result of these watch mode program is interface that has method to getProgram by synchronizing with changes in the host to emit files and/or report diagnostics if needed. Note that in the above api usage of System allows program to watch for changes and it would update itself on detecting changes in files/settings and so on. Note that program is synchronized on such updates through System.setTimeout so that it can batch the updates. The below method is way for user to ask for update on the program.

    interface Watch<T>{
        /** Synchronize with host and get updated program */
        getProgram(): T;
    }
    /**
     * Creates the watch what generates program using the config file
     */
    interface WatchOfConfigFile<T> extends Watch<T> {
    }
    /**
     * Creates the watch that generates program using the root files and compiler options
     */
    interface WatchOfFilesAndCompilerOptions<T> extends Watch<T> {
        /** Updates the root files in the program, only if this is not config file compilation */
        updateRootFileNames(fileNames: string[]): void;
    }

Below api allows user to have more control on what to do after creation/update of the program as well as provide CompilerHost like api:

    /**
     * Creates the watch from the host for root files and compiler options
     */
    function createWatch(host: WatchOfFilesAndCompilerOptionsHost): WatchOfFilesAndCompilerOptions<Program>;
    /**
     * Creates the watch from the host for config file
     */
    function createWatch(host: WatchOfConfigFileHost): WatchOfConfigFile<Program>;

The host implantation will allow the caller to provide callbacks to be called after and before program create through beforeProgramCreate and afterProgramCreate. Example usage of these is the WatchHost created by tsc where on beforeProgramCreate enables statistics for the program and afterProgramCreate it reports those as well as diagnostics and emits files. The callbacks are optional and are useful if host allows synchronizing async after detecting changes (through setTimeout and clearTimeout). The host has most of the methods like CompilerHost, except it doesnt have getSourceFile api as sourceFile management is done inside the api itself. In addition to CompilerHost like api, it has watchFile and watchDirectory methods that have to be provided, these are important so that WatchProgram is intelligent and efficient with reuse.

    interface WatchCompilerHost {
        /** If provided, callback to invoke before each program creation */
        beforeProgramCreate?(compilerOptions: CompilerOptions): void;
        /** If provided, callback to invoke after every new program creation */
        afterProgramCreate?(program: Program): void;
        /** If provided, called with Diagnostic message that informs about change in watch status */
        onWatchStatusChange?(diagnostic: Diagnostic, newLine: string): void;
        useCaseSensitiveFileNames(): boolean;
        getNewLine(): string;
        getCurrentDirectory(): string;
        getDefaultLibFileName(options: CompilerOptions): string;
        getDefaultLibLocation?(): string;
        /**
         * Use to check file presence for source files and
         * if resolveModuleNames is not provided (complier is in charge of module resolution) then module files as well
         */
        fileExists(path: string): boolean;
        /**
         * Use to read file text for source files and
         * if resolveModuleNames is not provided (complier is in charge of module resolution) then module files as well
         */
        readFile(path: string, encoding?: string): string | undefined;
        /** If provided, used for module resolution as well as to handle directory structure */
        directoryExists?(path: string): boolean;
        /** If provided, used in resolutions as well as handling directory structure */
        getDirectories?(path: string): string[];
        /** If provided, used to cache and handle directory structure modifications */
        readDirectory?(path: string, extensions?: ReadonlyArray<string>, exclude?: ReadonlyArray<string>, include?: ReadonlyArray<string>, depth?: number): string[];
        /** Symbol links resolution */
        realpath?(path: string): string;
        /** If provided would be used to write log about compilation */
        trace?(s: string): void;
        /** If provided, used to resolve the module names, otherwise typescript's default module resolution */
        resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames?: string[]): ResolvedModule[];
        /** Used to watch changes in source files, missing files needed to update the program or config file */
        watchFile(path: string, callback: FileWatcherCallback, pollingInterval?: number): FileWatcher;
        /** Used to watch resolved module's failed lookup locations, config file specs, type roots where auto type reference directives are added */
        watchDirectory(path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher;
        /** If provided, will be used to set delayed compilation, so that multiple changes in short span are compiled together */
        setTimeout?(callback: (...args: any[]) => void, ms: number, ...args: any[]): any;
        /** If provided, will be used to reset existing delayed compilation */
        clearTimeout?(timeoutId: any): void;
    }
    /**
     * Host to create watch with root files and options
     */
    interface WatchCompilerHostOfFilesAndCompilerOptions extends WatchCompilerHost {
        /** root files to use to generate program */
        rootFiles: string[];
        /** Compiler options */
        options: CompilerOptions;
    }
    /**
     * Reports config file diagnostics
     */
    interface ConfigFileDiagnosticsReporter {
        /**
         * Reports the diagnostics in reading/writing or parsing of the config file
         */
        onConfigFileDiagnostic: DiagnosticReporter;
        /**
         * Reports unrecoverable error when parsing config file
         */
        onUnRecoverableConfigFileDiagnostic: DiagnosticReporter;
    }
    /**
     * Host to create watch with config file
     */
    interface WatchCompilerHostOfConfigFile extends WatchCompilerHost, ConfigFileDiagnosticsReporter {
        /** Name of the config file to compile */
        configFileName: string;
        /** Options to extend */
        optionsToExtend?: CompilerOptions;
        /**
         * Used to generate source file names from the config file and its include, exclude, files rules
         * and also to cache the directory stucture
         */
        readDirectory(path: string, extensions?: ReadonlyArray<string>, exclude?: ReadonlyArray<string>, include?: ReadonlyArray<string>, depth?: number): string[];
    }

In addition to this there are builder program api's that let you create program that can only emit affected files and cache the semantic diagnostics for files that havent changed or have same diagnostics.

There are two apis to create two types of the Builder, one that manages only semantic diagnostics and other one that manages semantic diagnostics as well as allows emitting only affected files.

    interface BuilderProgramHost {
        /**
         * return true if file names are treated with case sensitivity
         */
        useCaseSensitiveFileNames(): boolean;
        /**
         * If provided this would be used this hash instead of actual file shape text for detecting changes
         */
        createHash?: (data: string) => string;
        /**
         * When emit or emitNextAffectedFile are called without writeFile,
         * this callback if present would be used to write files
         */
        writeFile?: WriteFileCallback;
    }
    /**
     * Create the builder to manage semantic diagnostics and cache them
     */
    function createSemanticDiagnosticsBuilderProgram(newProgram: Program, host: BuilderProgramHost, oldProgram?: SemanticDiagnosticsBuilderProgram): SemanticDiagnosticsBuilderProgram;
    /**
     * Create the builder that can handle the changes in program and iterate through changed files
     * to emit the those files and manage semantic diagnostics cache as well
     */
    function createEmitAndSemanticDiagnosticsBuilderProgram(newProgram: Program, host: BuilderProgramHost, oldProgram?: EmitAndSemanticDiagnosticsBuilderProgram): EmitAndSemanticDiagnosticsBuilderProgram;

The host takes in createHash as optional parameter wherein, it would be invoked to store the hash for the module shape of the files. If not provided, the text itself is stored (which can mean storing larger strings) and hence createHash if possible is recommended. The writeFile if provided, is used as fallback during emit if there is no local writeFile callback. Failing to provide this, the emit would invoke CompilerHost's writeFile callback.

You can create new program by passing old Program just like createProgram api to get incremental program. Both builders also have method getAllDependencies allows one to get all the dependencies for the file. Eg. usage it can be used to report those dependencies to webpack.

The other api's in BuilderProgram are just wire through program so that you dont need to store program and can just store BuilderProgram instead. Note that getSemanticDiagnostics and emit are special depending on the type of the builder and will be discussed in detail further.

    /**
     * Builder to manage the program state changes
     */
    interface BuilderProgram {
        /**
         * Returns current program
         */
        getProgram(): Program;
        /**
         * Get compiler options of the program
         */
        getCompilerOptions(): CompilerOptions;
        /**
         * Get the source file in the program with file name
         */
        getSourceFile(fileName: string): SourceFile | undefined;
        /**
         * Get a list of files in the program
         */
        getSourceFiles(): ReadonlyArray<SourceFile>;
        /**
         * Get the diagnostics for compiler options
         */
        getOptionsDiagnostics(cancellationToken?: CancellationToken): ReadonlyArray<Diagnostic>;
        /**
         * Get the diagnostics that dont belong to any file
         */
        getGlobalDiagnostics(cancellationToken?: CancellationToken): ReadonlyArray<Diagnostic>;
        /**
         * Get the syntax diagnostics, for all source files if source file is not supplied
         */
        getSyntacticDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray<Diagnostic>;
        /**
         * Get all the dependencies of the file
         */
        getAllDependencies(sourceFile: SourceFile): ReadonlyArray<string>;
        /**
         * Gets the semantic diagnostics from the program corresponding to this state of file (if provided) or whole program
         * The semantic diagnostics are cached and managed here
         * Note that it is assumed that when asked about semantic diagnostics through this API,
         * the file has been taken out of affected files so it is safe to use cache or get from program and cache the diagnostics
         * In case of SemanticDiagnosticsBuilderProgram if the source file is not provided,
         * it will iterate through all the affected files, to ensure that cache stays valid and yet provide a way to get all semantic diagnostics
         */
        getSemanticDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray<Diagnostic>;
        /**
         * Emits the JavaScript and declaration files.
         * When targetSource file is specified, emits the files corresponding to that source file,
         * otherwise for the whole program.
         * In case of EmitAndSemanticDiagnosticsBuilderProgram, when targetSourceFile is specified,
         * it is assumed that that file is handled from affected file list. If targetSourceFile is not specified,
         * it will only emit all the affected files instead of whole program
         *
         * The first of writeFile if provided, writeFile of BuilderProgramHost if provided, writeFile of compiler host
         * in that order would be used to write the files
         */
        emit(targetSourceFile?: SourceFile, writeFile?: WriteFileCallback, cancellationToken?: CancellationToken, emitOnlyDtsFiles?: boolean, customTransformers?: CustomTransformers): EmitResult;
    }

The builder created by createSemanticDiagnosticsBuilderProgram has method getSemanticDiagnosticsOfNextAffectedFile to iterate through changed files and get its semantic diagnostics and the affected source file the diagnostics are for. When any changed file results in affecting whole program the diagnostics are returned with the affected as program. When this method is called and there are no more changed/affected files that need to re-evaluate semantic diagnostics, it returns undefined notifying that the iteration is complete. The another method getSemanticDiagnostics is available to get diagnostics for the source file but it is required that iteration through the program change is complete Only in case of SemanticDiagnosticsBuilderProgram if sourceFile is not provided, it will iterate through all affected files and cache the diagnostics. The purpose of this method is mainly to get diagnostics in the non-affected program so that the user of the API doesn't need to hold diagnostics from previous program and it can report all the errors instead of just affected file errors, or in case of EmitAndSemanticDiagnosticsBuilderProgram it can also be used to get all errors after emit.

    /**
     * The builder that caches the semantic diagnostics for the program and handles the changed files and affected files
     */
    interface SemanticDiagnosticsBuilderProgram extends BuilderProgram {
        /**
         * Gets the semantic diagnostics from the program for the next affected file and caches it
         * Returns undefined if the iteration is complete
         */
        getSemanticDiagnosticsOfNextAffectedFile(cancellationToken?: CancellationToken, ignoreSourceFile?: (sourceFile: SourceFile) => boolean): AffectedFileResult<ReadonlyArray<Diagnostic>>;
    }

The builder created using createEmitAndSemanticDiagnosticsBuilderProgram allows you to iterate through changed files to emit the changed/affected files through emitNextAffectedFile. Just like earlier method to iterate through semantic diagnostics of the changed files, this method returns the emit result and affected as program or source file depending on the change resulting in the emit. Once iteration is complete the call to emitNextAffectedFile return undefined.

 /**
     * The builder that can handle the changes in program and iterate through changed file to emit the files
     * The semantic diagnostics are cached per file and managed by clearing for the changed/affected files
     */
    interface EmitAndSemanticDiagnosticsBuilderProgram extends BuilderProgram {
        /**
         * Get the current directory of the program
         */
        getCurrentDirectory(): string;
        /**
         * Emits the next affected file's emit result (EmitResult and sourceFiles emitted) or returns undefined if iteration is complete
         * The first of writeFile if provided, writeFile of BuilderProgramHost if provided, writeFile of compiler host
         * in that order would be used to write the files
         */
        emitNextAffectedFile(writeFile?: WriteFileCallback, cancellationToken?: CancellationToken, emitOnlyDtsFiles?: boolean, customTransformers?: CustomTransformers): AffectedFileResult<EmitResult>;
    }

Apart from this there is also api that uses Watch and BuilderProgram together to create easier Watch<BuilderProgram> through createWatchBuilderProgram. With this api if you provide createSemanticDiagnosticsBuilderProgram or createEmitAndSemanticDiagnosticsBuilderProgram as parameter createBuilderProgram everytime you query getProgram on the watch you would get BuilderProgram of that kind instead of program.

    /**
     * Creates the watch from the host for root files and compiler options
     */
    function createWatchBuilderProgram<T extends BuilderProgram>(host: WatchCompilerHostOfFilesAndCompilerOptions & BuilderProgramHost, createBuilderProgram: (newProgram: Program, host: BuilderProgramHost, oldProgram?: T) => T): WatchOfFilesAndCompilerOptions<T>;
    /**
     * Creates the watch from the host for config file
     */
    function createWatchBuilderProgram<T extends BuilderProgram>(host: WatchCompilerHostOfConfigFile & BuilderProgramHost, createBuilderProgram: (newProgram: Program, host: BuilderProgramHost, oldProgram?: T) => T): WatchOfConfigFile<T>;

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

HerringtonDarkholme, johnnyreilly, piotr-oles, quanterion, errendir, fathyb, MattiasBuelens, zpdDG4gta8XKpMCd, chriskrycho, SlurpTheo, and guptag reacted with thumbs up emoji
All reactions
  • 👍 11 reactions
Sheetal Nandi (sheetalkamat) added 11 commits November 22, 2017 11:48
@sheetalkamat
Use builder state to emit instead
a06f0c3
@sheetalkamat
Expose the watch and builder API in the typescript.d.ts
576fe1e
@sheetalkamat
Lint errors fix
7ebf9d9
@sheetalkamat
Allow watch host to specify module name resolver
3c5a6e1
@sheetalkamat
Add api to get the dependencies of the file
c9a17f3
@sheetalkamat
Make the versions in the source file non zero when the source file is…
6d36a3d
… created
@sheetalkamat
Make the builder state as internal and expose builder instead of buil…
85ce1d0
…der state
@sheetalkamat
Use the results from affected file enumerator apis as Affected File r…
e102fee
…esult
@sheetalkamat
Set program as affected if emitting/diagnostics for whole program
ffa64e8
@sheetalkamat
To handle cancellation token, remove changed/affected files from the …
012f12b
…changeset only after getting the result
@sheetalkamat
Handle emit only declaration file to always produce declaration file …
0b79f4a
…and skip the diagnostics check
This was referenced Nov 23, 2017
Using the new watch api of compiler jrieken/gulp-tsb#74
Closed
Using the new watch api of compiler s-panferov/awesome-typescript-loader#519
Merged
@HerringtonDarkholme

Herrington Darkholme (HerringtonDarkholme) commented Nov 26, 2017

Copy link
Copy Markdown
Contributor

This is amazing! cc John Reilly (@johnnyreilly)

johnnyreilly reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@johnnyreilly

John Reilly (johnnyreilly) commented Nov 26, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

This is awesome! Sheetal Nandi (@sheetalkamat) would you be open to submitting a similar PR against ts-loader? I'd love to get this support into ts-loader as fast as possible (as I really, really like fast builds 😄 )

It'd be most welcome and I'd be happy to provide any assistance you might need along the way. (I can help slay test pack dragons 🐉 etc)

Great work BTW! I'm really pleased the TypeScript team are doing this - thank you so much! 🌻

BTW your link to jrieken/gulp-tsb is broken... Think it should be jrieken/gulp-tsb#74

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@johnnyreilly

John Reilly (johnnyreilly) commented Nov 26, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Piotr Oleś (@piotr-oles) this could be useful for fork-ts-checker-webpack-plugin perhaps? (In fact should make it much simpler I would guess?)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sheetalkamat Sheetal Nandi (sheetalkamat) mentioned this pull request Nov 28, 2017
Using the new watch api of compiler TypeStrong/ts-loader#685
Merged
@sheetalkamat

Sheetal Nandi (sheetalkamat) commented Nov 28, 2017

Copy link
Copy Markdown
Member Author

John Reilly (@johnnyreilly) I did create a branch for ts-loader but wasn't sure if that is improvement over current implementation or not.
Here were my findings:

  • ts-loader uses custom module resolution, which means we wont have information of failed lookup locations to watch, hence we wont know if we are updating the program correctly or not.
  • ts-loader reports diagnostics for affected files + files with error from previous compilation list of which it already has. That means if it uses SemanticDiagnosticsBuilder it would benefit only for caching errors in files with previous compilation that are not part of affected file. I thought that the cache here wouldn't be that helpful because those kind of files would be small set and might not be worth the extra work builder would do to keep them.
  • both ts-loader and awesome-typescript-loader already know which files to emit since webpack is doing that work to watch and emit the files.

I have still submitted PR with the work I had done at TypeStrong/ts-loader#685

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@johnnyreilly

John Reilly (johnnyreilly) commented Nov 28, 2017

Copy link
Copy Markdown

Thanks so much - I'll have a tinker!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@johnnyreilly

John Reilly (johnnyreilly) commented Nov 28, 2017

Copy link
Copy Markdown
  • ts-loader uses custom module resolution, which means we wont have information of failed lookup locations to watch, hence we wont know if we are updating the program correctly or not.

I'm guessing you're referring to the code here? If memory serves, ts-loader had custom module resolution from James Brantly (@jbrantly)'s original work. You'll see that we use the TypeScript resolution in certain circumstances. There's multiple strategies in play for backwards compatibility reasons.

With the next major version of ts-loader I was planning to drop support for TypeScript <= 2.3 to simplify the codebase. I was also planning to look again at whether we could just use the TypeScript module resolution and drop the custom resolution entirely. (Again it would be simpler and more consistent.)

Am I right in understanding that using the PR without that change could be problematic?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sheetalkamat

Sheetal Nandi (sheetalkamat) commented Nov 29, 2017

Copy link
Copy Markdown
Member Author

John Reilly (@johnnyreilly) yes thats the code.. while looking at it in deep there are cases when module resolution through typescript is used and sometimes not. Wasnt very clear on whats going on there.. Was not even sure if you would have information about locations to watch on for resolving module names to consider having that as part of the api.
As you have guessed, the PR would be less reliable especially in case of when the resolved modules arent present during first compilation and appear later, say through npm install..

quanterion and johnnyreilly reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@johnnyreilly

John Reilly (johnnyreilly) commented Nov 29, 2017

Copy link
Copy Markdown

Sheetal Nandi (@sheetalkamat) cool - this just reinforces my plan to drop custom module resolution and move to use TypeScript's entirely. My current plan is to wait until your TypeScript PR has been merged and so this functionality starts appearing in the nightly builds.

Once that's in place I can then look to merge your ts-loader PR and potentially drop the custom module resolution at the same time. The code should become much clearer 👍

Have you any idea when this is likely to get merged? Thanks again for all your work x 💯

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sheetal Nandi (sheetalkamat) added 8 commits December 4, 2017 14:21
@sheetalkamat
Merge branch 'master' into builderApi
374536b
@sheetalkamat
Rename getProgram to getExistingProgram
3dda217
@sheetalkamat
Rename Watch.synchronizeProgram to getProgram and return the updated …
61fc9b9
…program as part of this api
@sheetalkamat
Rename WatchHost.moduleNameResolver to WatchHost.resolveModuleNames t…
471c83b
…o align with compiler host
@sheetalkamat
Make before and after program create callbacks optional
1a91256
@sheetalkamat
Merge branch 'master' into builderApi
f046d82
@sheetalkamat
Instead of using system as object on WatchHost, create WatchCompilerH…
944f8b8
…ost that combines the functionality
@sheetalkamat
More functions moved from system to WatchCompilerHost
43c2610
mhegazy
Mohamed Hegazy (mhegazy) reviewed Jan 18, 2018
View reviewed changes
Comment thread src/compiler/watch.ts Outdated
/**
* Creates the watch compiler host from system for config file in watch mode
*/
export function createWatchCompilerHostOfConfigFile(configFileName: string, optionsToExtend: CompilerOptions | undefined, system: System, reportDiagnostic: DiagnosticReporter | undefined, reportWatchStatus: WatchStatusReporter | undefined): WatchCompilerHostOfConfigFile {

@mhegazy Mohamed Hegazy (mhegazy) Jan 18, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

consider combining these two APIs into createWatchCompilerHost (jsut like we havecreateCompilerHost) with two overloads instead of two separate functions.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@sheetalkamat Sheetal Nandi (sheetalkamat) Jan 18, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Sheetal Nandi (sheetalkamat) added 2 commits January 17, 2018 16:11
@sheetalkamat
Merge branch 'master' into builderApi
ed23ca5
@sheetalkamat
Do not expose createWatchOfConfigFile and createWatchOfFilesAndCompil…
29dee9f
…erOptions
mhegazy
Mohamed Hegazy (mhegazy) reviewed Jan 18, 2018
View reviewed changes
Comment thread tests/baselines/reference/api/typescript.d.ts Outdated
type WatchStatusReporter = (diagnostic: Diagnostic, newLine: string) => void;
interface WatchCompilerHost {
/** If provided, callback to invoke before each program creation */
beforeProgramCreate?(compilerOptions: CompilerOptions): void;

@mhegazy Mohamed Hegazy (mhegazy) Jan 18, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Question: Can we replace beforeProgramCreate and afterProgramCreate with just createProgram and make it required. this returns a Program instance that can either be a regular Program or a BuilderProgram based on what the caller chooses?
this way we do not need to distinguish between createWatchBuilderProgram and createWatchProgram.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@sheetalkamat Sheetal Nandi (sheetalkamat) Jan 18, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

But host is passed to createWatchProgram or createWatchBuilderProgram to actually create the program or BuilderProgram.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@mhegazy Mohamed Hegazy (mhegazy) Jan 18, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I am thinking of something like

export function createWatchProgram<T extends Program = EmitAndSemanticDiagnosticsBuilderProgram>(host: WatchCompilerHostOfFilesAndCompilerOptions, createProgram : (old:T|undefined) =>T = createEmitAndSemanticDiagnosticsBuilderProgram): WatchOfFilesAndCompilerOptions<T>;

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@mhegazy Mohamed Hegazy (mhegazy) Jan 18, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I am not sure i understood your answer. maybe easier to do this in person tommrow.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@sheetalkamat Sheetal Nandi (sheetalkamat) Jan 19, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

builderApi...builderApiWithCreateProgram the change to take in createProgram as input on host.. I think it makes it little bit more messy.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
mhegazy
Mohamed Hegazy (mhegazy) reviewed Jan 18, 2018
View reviewed changes
Comment thread tests/baselines/reference/api/typescript.d.ts Outdated
/**
* Creates the watch from the host for root files and compiler options
*/
function createWatchBuilderProgram<T extends BuilderProgram>(host: WatchCompilerHostOfFilesAndCompilerOptions & BuilderProgramHost, createBuilderProgram: (newProgram: Program, host: BuilderProgramHost, oldProgram?: T) => T): WatchOfFilesAndCompilerOptions<T>;

@mhegazy Mohamed Hegazy (mhegazy) Jan 18, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

seems that this is the default behavior of createWatchProgram anyways, and to override that you need to change the onAfterProgramCreate on the host.. if this is accurate, why do we need this helper?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@sheetalkamat Sheetal Nandi (sheetalkamat) Jan 18, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

createWatchProgram just creates the program.. The user of createWatchProgram gets the Program. createWatchBuilderProgram creates the watchProgram and exposes the builder program instead of watchProgram. Eg. ts-loader doesnt need builder at all.. But gulp-tsb uses createBuilderProgram

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@mhegazy Mohamed Hegazy (mhegazy) Jan 18, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

in createWatchCompilerHost we return an object whose afterProgramCreate property is set to emitFilesAndReportErrorUsingBuilder by default. this will create a builderProgram using createEmitAndSemanticDiagnosticsBuilderProgram. what am i missing?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@sheetalkamat Sheetal Nandi (sheetalkamat) Jan 18, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

But ts-loader or gulp-tsb do not use createWatchCompilerHost to create watch compiler host. Instead they create their own WatchCompilerHost that doesnt have these properties set.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Sheetal Nandi (sheetalkamat) added 2 commits January 17, 2018 16:44
@sheetalkamat
Expose createWatchCompilerHost as overload
f29c0e3
@sheetalkamat
Move getCurrentDirectory to builder program
bd43e45
@sheetalkamat
Sheetal Nandi (sheetalkamat) force-pushed the builderApi branch from 752e9d7 to 1c1226e Compare January 19, 2018 23:29
@sheetalkamat
Add createProgram on WatchCompilerHost
2be231d
@sheetalkamat
Sheetal Nandi (sheetalkamat) force-pushed the builderApi branch from 1c1226e to 2be231d Compare January 19, 2018 23:52
@sheetalkamat

Sheetal Nandi (sheetalkamat) commented Jan 19, 2018

Copy link
Copy Markdown
Member Author

Mohamed Hegazy (@mhegazy) Updated as per our offline discussion

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sheetalkamat
Merge branch 'master' into builderApi
8a51cda
mhegazy
Mohamed Hegazy (mhegazy) approved these changes Jan 20, 2018
View reviewed changes
@sheetalkamat
Sheetal Nandi (sheetalkamat) merged commit cc6d18e into master Jan 20, 2018
@sheetalkamat
Sheetal Nandi (sheetalkamat) deleted the builderApi branch January 20, 2018 00:14
This was referenced Jan 20, 2018
Provide API for faster incremental compilers #9282
Closed
Expose incremental Compile-on-Save builder API #17493
Closed
@weswigham

Wesley Wigham (weswigham) commented Jan 20, 2018

Copy link
Copy Markdown
Member

Mohamed Hegazy (@mhegazy) Is this going to land in the 2.7 release? If so, niiiiice.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@johnnyreilly

John Reilly (johnnyreilly) commented Jan 20, 2018

Copy link
Copy Markdown

Sheetal Nandi (@sheetalkamat) - well done for all your work! So glad this has now landed! 🎆

Question: is any of the refactoring done as part of getting this merged into master likely to have an impact on your PR for ts-loader?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sheetalkamat

Sheetal Nandi (sheetalkamat) commented Jan 20, 2018

Copy link
Copy Markdown
Member Author

John Reilly (@johnnyreilly) the ts-loader PR is uptodate with the changes of this PR

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@johnnyreilly

John Reilly (johnnyreilly) commented Jan 20, 2018

Copy link
Copy Markdown

Thanks Sheetal Nandi (@sheetalkamat) - I'll give it a whirl and report back!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@HerringtonDarkholme Herrington Darkholme (HerringtonDarkholme) mentioned this pull request Jan 21, 2018
Vue file. Intellisense shows outdated list of exported members vuejs/vetur#355
Closed
2 tasks done
@johnnyreilly John Reilly (johnnyreilly) mentioned this pull request Jan 21, 2018
Watch API caching stale errors and not supporting lib compiler option #21325
Closed
@johnnyreilly

John Reilly (johnnyreilly) commented Jan 21, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Hey Sheetal Nandi (@sheetalkamat),

I've merged your PR and pushed out a new version of ts-loader adding the watch support behind a new option called experimentalWatchApi (which is disabled by default). It looks like there's some problems. I've raised this issue here to provide details:

#21325

I've also created a minimal illustration repo to demonstrate the problems:

https://github.com/johnnyreilly/typescript-ts-loader-watch-api-illustration

I'm highly motivated to get these issues resolved - if I can help in any way then please do tell me what I can do.

Thanks again for all your hard work - I really appreciate it.

donaldpipowitch reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@chuckjaz Chuck Jazdzewski (chuckjaz) mentioned this pull request Jan 22, 2018
Update ngc to take advantage of the new watch API in TS 2.8 angular/angular#21703
Closed
@chriskrycho Chris Krycho (chriskrycho) mentioned this pull request Feb 6, 2018
Support incremental compilation via TS API tildeio/broccoli-typescript-compiler#57
Open
@mhegazy Mohamed Hegazy (mhegazy) mentioned this pull request Feb 8, 2018
Suggestion: CompilerHost should be able to "plug into" a LanguageServiceHost #9017
Closed
@mhegazy Mohamed Hegazy (mhegazy) mentioned this pull request Mar 29, 2018
Typescript Watch - Cleaning Target Files on Source Deletion #16057
Open
@dfreeman Dan Freeman (dfreeman) mentioned this pull request May 14, 2018
Stop inspecting output strings to determine when builds complete typed-ember/ember-cli-typescript#209
Merged
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@mhegazy Mohamed Hegazy (mhegazy) mhegazy approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@sheetalkamat @HerringtonDarkholme @johnnyreilly @weswigham @mhegazy
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.