Allow type-checking when --isolatedModules specified: can PR for this change

[mheiber]

Expected behavior:

There is a way to get type-checking while using --isolatedModules

Actual behavior:

when passing --isolatedModules no type-checking is done. This is by design, but we want a way to get the same behavior as tsc, where --isolatedModules checks that separate compilation si safe, but doesn't actually do separate compilation.

Your gulpfile:

Include your gulpfile, or only the related task (with ts.createProject).

ts.createProject({ isolatedModules: true })

tsconfig.json

{
    "isolatedModules": true
}

Code

Include your TypeScript code, if necessary.

const x: string = 2; // should error

Suggestion Implementation: new GulpTsSettings with key useFileCompiler:

Happy to make a PR for this change. Sketch of implementation:

main.ts

export interface GulpTsSettings {
    useFileCompiler?: boolean
}

export function createProject(fileNameOrSettings?: string | Settings, settings?: Settings, gulpTsSettings: GulpTsSettings = {}): Project {
...
}

...

const project = _project.setupProject(projectDirectory, tsConfigFileName, rawConfig, tsConfigContent, compilerOptions, projectReferences, typescript, finalTransformers, gulpTsSettings.useFileCompiler);

project.ts

function setupProject(...useFileCompiler: boolean | undefined) {
    ...
    if (useFileCompiler && !options.isolatedModules) {
        throw Error("useFileCompiler: true can only be used if config.compilerOptions.isolatedModules is also set to true");
    }
    const compiler: ICompiler = (options.isolatedModules && (useFileCompiler === undefined || useFileCompiler === true)) ? new FileCompiler() : new ProjectCompiler();

[mheiber]

pinging @ivogabe about this: we have code for this mostly done, but want to verify the approach and naming. Really appreciate your time if you could take a look, this change would really help us.

[mheiber]

ping @ivogabe

[ivogabe]

Hi Max, sorry for the delay. I think it's a good idea to change it like you propose.

For the API, I'd like to also integrate the 'typescript' option (https://github.com/ivogabe/gulp-typescript/tree/d0410c2dc37cd4259689e4bc354a8ec68e779823#typescript-version) in the GulpTsSettings object. The createProject should also work when you don't pass it a tsconfig file, so it should have be overloaded:

export function createProject(settings?: Settings, gulpTsSettings?: GulpTsSettings): Project;
export function createProject(fileName: string, settings?: Settings, gulpTsSettings?: GulpTsSettings): Project;

For users which do not use the createProject interface, we should modify the main export (

gulp-typescript/lib/main.ts

Line 8 in d0410c2

function compile(proj: _project.Project, theReporter?: _reporter.Reporter): compile.CompileStream;

). This function also takes a reporter, which we will need to move one position to the right. I think that the GulpTsSettings are used more often than the reporters. We cannot add Reporter to the GulpTsSettings interface, as that would not work with the createProject interface; the reporter is there namely passed in the gulp task, not at the creation of the project. The signature of the main export would then thus look like this:

function compile(settings?: compile.Settings, gulpTsSettings?: GulpTsSettings, theReporter?: _reporter.Reporter): compile.CompileStream;

You can remove the overload taking a project, as that API is deprecated and we can remove that in the next major release.

Great that you want to create a PR for this! Let me know if you need any help.

[mheiber]

Hi @ivogabe thanks very much for getting back to me and taking the time to put together this guidance. I return from holiday next week and can put something together then. If you need something sooner, please ping me here and I'll see what I can do.
Thanks!

[mheiber]

Looks like I'll have bandwidth to work on this this week.

[mheiber]

Working on this intermittently today.

The overloads got complicated, so I'm thinking of separating the 'figuring out which overload we are in' logic from the implementation of createProject.

export function createProject(): Project;
export function createProject(tsConfigFileName: string, settings?: Settings, gulpTsSettings?: GulpTsSettings): Project;
export function createProject(settings: Settings, gulpTsSettings?: GulpTsSettings): Project;
export function createProject(fileNameOrSettings?: string | Settings, settingsOrGulpTsSettings?: Settings | GulpTsSettings, gulpTsSettingsParam?: GulpTsSettings): Project {
	// overload: createProject()
	if (!fileNameOrSettings) {
		return _createProject({
			fileName: undefined,
			settings: {},
			gulpTsSettings: {}
                });
	}
	// overload: createProject(tsConfigFileName, settings, gulpTsSettings)
	if (typeof fileNameOrSettings === 'string') {
		return _createProject({
			fileName: fileNameOrSettings,
			settings: settingsOrGulpTsSettings || {},
			gulpTsSettings: gulpTsSettingsParam || {} }
		);
	}
	// overload: createProject(settings, gulpTsSettings)
	return _createProject({
		fileName: undefined,
		settings: fileNameOrSettings || {},
		gulpTsSettings: settingsOrGulpTsSettings as GulpTsSettings || {} }
	);
}

[ivogabe]

The first overload can be removed, as it is already handled by the last overload. You only need to mark the settings argument of that overload as optional:

export function createProject(tsConfigFileName: string, settings?: Settings, gulpTsSettings?: GulpTsSettings): Project;
export function createProject(settings?: Settings, gulpTsSettings?: GulpTsSettings): Project;

export function createProject(fileNameOrSettings?: string | Settings, settingsOrGulpTsSettings?: Settings | GulpTsSettings, gulpTsSettingsParam?: GulpTsSettings): Project {
	// overload: createProject(tsConfigFileName, settings, gulpTsSettings)
	if (typeof fileNameOrSettings === 'string') {
		return _createProject({
			fileName: fileNameOrSettings,
			settings: settingsOrGulpTsSettings || {},
			gulpTsSettings: gulpTsSettingsParam || {} }
		);
	}
	// overload: createProject(settings, gulpTsSettings)
	return _createProject({
		fileName: undefined,
		settings: fileNameOrSettings || {},
		gulpTsSettings: settingsOrGulpTsSettings as GulpTsSettings || {} }
	);
}

[mheiber]

@ivogabe I've posted a WIP PR to get your feedback.

The reason it is 'WIP' is that tests are hanging on my branch and I'm not sure why. Do you have any advice on how to troubleshoot? There aren't any error messages.

[mheiber]

I know it's been a long time.
I've been really swamped but haven't forgotten about this. If there isn't much demand, then would it be OK for me to pick this back up the week of August 5th?