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

Uses existing instance if config file is same as already built solution #1177

Merged
merged 7 commits into from Sep 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,8 @@
# Changelog

## v8.0.4
* [Uses existing instance if config file is same as already built solution](https://github.com/TypeStrong/ts-loader/pull/1177) - thanks @sheetalkamat

## v8.0.3
* [Fix the wrong instance caching when using `appendTsSuffixTo` and `appendTsxSuffixTo` together](https://github.com/TypeStrong/ts-loader/pull/1170) - thanks @meowtec

Expand Down
2 changes: 1 addition & 1 deletion package.json
@@ -1,6 +1,6 @@
{
"name": "ts-loader",
"version": "8.0.3",
"version": "8.0.4",
"description": "TypeScript loader for webpack",
"main": "index.js",
"types": "dist",
Expand Down
24 changes: 16 additions & 8 deletions src/after-compile.ts
Expand Up @@ -6,6 +6,7 @@ import * as constants from './constants';
import { getEmitFromWatchHost, getEmitOutput } from './instances';
import {
FilePathKey,
LoaderOptions,
TSFiles,
TSInstance,
WebpackModule,
Expand All @@ -17,6 +18,7 @@ import {
formatErrors,
isReferencedFile,
populateReverseDependencyGraph,
tsLoaderSource,
} from './utils';

export function makeAfterCompile(
Expand All @@ -41,7 +43,7 @@ export function makeAfterCompile(
callback();
return;
}
removeCompilationTSLoaderErrors(compilation);
removeCompilationTSLoaderErrors(compilation, instance.loaderOptions);

provideCompilerOptionDiagnosticErrorsToWebpack(
getCompilerOptionDiagnostics,
Expand Down Expand Up @@ -235,7 +237,7 @@ function provideErrorsToWebpack(
const associatedModules = modules.get(instance.filePathKeyMapper(fileName));
if (associatedModules !== undefined) {
associatedModules.forEach(module => {
removeModuleTSLoaderError(module);
removeModuleTSLoaderError(module, loaderOptions);

// append errors
const formattedErrors = formatErrors(
Expand Down Expand Up @@ -299,7 +301,7 @@ function provideSolutionErrorsToWebpack(
const associatedModules = modules.get(filePath);
if (associatedModules !== undefined) {
associatedModules.forEach(module => {
removeModuleTSLoaderError(module);
removeModuleTSLoaderError(module, loaderOptions);

// append errors
const formattedErrors = formatErrors(
Expand Down Expand Up @@ -445,14 +447,18 @@ function provideAssetsFromSolutionBuilderHost(
* the loader, we need to detect and remove any pre-existing errors.
*/
function removeCompilationTSLoaderErrors(
compilation: webpack.compilation.Compilation
compilation: webpack.compilation.Compilation,
loaderOptions: LoaderOptions
) {
compilation.errors = compilation.errors.filter(
error => error.loaderSource !== 'ts-loader'
error => error.loaderSource !== tsLoaderSource(loaderOptions)
);
}

function removeModuleTSLoaderError(module: WebpackModule) {
function removeModuleTSLoaderError(
module: WebpackModule,
loaderOptions: LoaderOptions
) {
/**
* Since webpack 5, the `errors` property is deprecated,
* so we can check if some methods for reporting errors exist.
Expand All @@ -464,11 +470,13 @@ function removeModuleTSLoaderError(module: WebpackModule) {

Array.from(warnings || []).forEach(warning => module.addWarning(warning));
Array.from(errors || [])
.filter((error: any) => error.loaderSource !== 'ts-loader')
.filter(
(error: any) => error.loaderSource !== tsLoaderSource(loaderOptions)
)
.forEach(error => module.addError(error));
} else {
module.errors = module.errors.filter(
error => error.loaderSource !== 'ts-loader'
error => error.loaderSource !== tsLoaderSource(loaderOptions)
);
}
}
68 changes: 51 additions & 17 deletions src/instances.ts
Expand Up @@ -13,7 +13,6 @@ import {
LoaderOptions,
TSFiles,
TSInstance,
TSInstances,
WebpackError,
} from './interfaces';
import * as logger from './logger';
Expand All @@ -34,7 +33,8 @@ import {
} from './utils';
import { makeWatchRun } from './watch-run';

const instances = {} as TSInstances;
const instances = new Map<string, TSInstance>();
const instancesBySolutionBuilderConfigs = new Map<FilePathKey, TSInstance>();

/**
* The loader is executed once for each file seen by webpack. However, we need to keep
Expand All @@ -47,20 +47,26 @@ export function getTypeScriptInstance(
loaderOptions: LoaderOptions,
loader: webpack.loader.LoaderContext
): { instance?: TSInstance; error?: WebpackError } {
if (instances.hasOwnProperty(loaderOptions.instance)) {
const instance = instances[loaderOptions.instance];
if (!instance.initialSetupPending) {
ensureProgram(instance);
const existing = instances.get(loaderOptions.instance);
if (existing) {
if (!existing.initialSetupPending) {
ensureProgram(existing);
}
return { instance: instances[loaderOptions.instance] };
return { instance: existing };
}

const colors = new chalk.constructor({ enabled: loaderOptions.colors });
const log = logger.makeLogger(loaderOptions, colors);
const compiler = getCompiler(loaderOptions, log);

if (compiler.errorMessage !== undefined) {
return { error: makeError(colors.red(compiler.errorMessage), undefined) };
return {
error: makeError(
loaderOptions,
colors.red(compiler.errorMessage),
undefined
),
};
}

return successfulTypeScriptInstance(
Expand Down Expand Up @@ -121,13 +127,25 @@ function successfulTypeScriptInstance(
const { message, file } = configFileAndPath.configFileError;
return {
error: makeError(
loaderOptions,
colors.red('error while reading tsconfig.json:' + EOL + message),
file
),
};
}

const { configFilePath, configFile } = configFileAndPath;
const filePathKeyMapper = createFilePathKeyMapper(compiler, loaderOptions);
if (configFilePath && loaderOptions.projectReferences) {
const configFileKey = filePathKeyMapper(configFilePath);
const existing = getExistingSolutionBuilderHost(configFileKey);
if (existing) {
// Reuse the instance if config file for project references is shared.
instances.set(loaderOptions.instance, existing);
return { instance: existing };
}
}

const basePath = loaderOptions.context || path.dirname(configFilePath || '');
const configParseResult = getConfigParseResult(
compiler,
Expand All @@ -151,6 +169,7 @@ function successfulTypeScriptInstance(

return {
error: makeError(
loaderOptions,
colors.red('error while parsing tsconfig.json'),
configFilePath
),
Expand All @@ -174,12 +193,11 @@ function successfulTypeScriptInstance(
filePath
)
: (filePath: string) => filePath;
const filePathKeyMapper = createFilePathKeyMapper(compiler, loaderOptions);

if (loaderOptions.transpileOnly) {
// quick return for transpiling
// we do need to check for any issues with TS options though
const transpileInstance: TSInstance = (instances[loaderOptions.instance] = {
const transpileInstance: TSInstance = {
compiler,
compilerOptions,
appendTsTsxSuffixesIfRequired,
Expand All @@ -198,8 +216,8 @@ function successfulTypeScriptInstance(
configParseResult,
log,
filePathKeyMapper,
});

};
instances.set(loaderOptions.instance, transpileInstance);
return { instance: transpileInstance };
}

Expand All @@ -223,6 +241,7 @@ function successfulTypeScriptInstance(
} catch (exc) {
return {
error: makeError(
loaderOptions,
colors.red(
`A file specified in tsconfig.json could not be found: ${normalizedFilePath!}`
),
Expand All @@ -231,7 +250,7 @@ function successfulTypeScriptInstance(
};
}

const instance: TSInstance = (instances[loaderOptions.instance] = {
const instance: TSInstance = {
compiler,
compilerOptions,
appendTsTsxSuffixesIfRequired,
Expand All @@ -249,11 +268,22 @@ function successfulTypeScriptInstance(
configParseResult,
log,
filePathKeyMapper,
});

};
instances.set(loaderOptions.instance, instance);
return { instance };
}

function getExistingSolutionBuilderHost(key: FilePathKey) {
const existing = instancesBySolutionBuilderConfigs.get(key);
if (existing) return existing;
for (const instance of instancesBySolutionBuilderConfigs.values()) {
if (instance.solutionBuilderHost!.configFileInfo.has(key)) {
return instance;
}
}
return undefined;
}

export function initializeInstance(
loader: webpack.loader.LoaderContext,
instance: TSInstance
Expand Down Expand Up @@ -421,13 +451,17 @@ export function buildSolutionReferences(
loader,
instance
);
instance.solutionBuilder = instance.compiler.createSolutionBuilderWithWatch(
const solutionBuilder = instance.compiler.createSolutionBuilderWithWatch(
instance.solutionBuilderHost,
instance.configParseResult.projectReferences!.map(ref => ref.path),
{ verbose: true }
);
instance.solutionBuilder!.build();
solutionBuilder.build();
ensureAllReferences(instance);
instancesBySolutionBuilderConfigs.set(
instance.filePathKeyMapper(instance.configFilePath!),
instance
);
} else {
instance.solutionBuilderHost.buildReferences();
}
Expand Down
6 changes: 0 additions & 6 deletions src/interfaces.ts
Expand Up @@ -204,9 +204,6 @@ export interface TSInstance {

reportTranspileErrors?: boolean;
solutionBuilderHost?: SolutionBuilderWithWatchHost;
solutionBuilder?: typescript.SolutionBuilder<
typescript.EmitAndSemanticDiagnosticsBuilderProgram
>;
configFilePath: string | undefined;

filePathKeyMapper: (fileName: string) => FilePathKey;
Expand All @@ -220,9 +217,6 @@ export interface LoaderOptionsCache {
[name: string]: WeakMap<LoaderOptions, LoaderOptions>;
}

export interface TSInstances {
[name: string]: TSInstance;
}
export type DependencyGraph = Map<FilePathKey, ResolvedModule[]>;
export type ReverseDependencyGraph = Map<FilePathKey, Map<FilePathKey, true>>;

Expand Down
8 changes: 7 additions & 1 deletion src/utils.ts
Expand Up @@ -100,6 +100,7 @@ export function formatErrors(
: loaderOptions.errorFormatter(errorInfo, colors);

const error = makeError(
loaderOptions,
message,
merge.file === undefined ? errorInfo.file : merge.file,
position === undefined
Expand All @@ -124,6 +125,7 @@ export function fsReadFile(
}

export function makeError(
loaderOptions: LoaderOptions,
message: string,
file: string | undefined,
location?: { line: number; character: number }
Expand All @@ -132,10 +134,14 @@ export function makeError(
message,
location,
file,
loaderSource: 'ts-loader',
loaderSource: tsLoaderSource(loaderOptions),
};
}

export function tsLoaderSource(loaderOptions: LoaderOptions) {
return `ts-loader-${loaderOptions.instance}`;
}

export function appendSuffixIfMatch(
patterns: (RegExp | string)[],
filePath: string,
Expand Down
@@ -0,0 +1,5 @@
import { lib } from '../lib';
import { utils } from "../utils";

console.log(lib.one, lib.two, lib.three);
utils();
@@ -0,0 +1,12 @@
{
"compilerOptions": {
"types": []
},
"files": [
"./app.ts"
],
"references": [
{ "path": "../lib" },
{ "path": "../utils" },
]
}
@@ -0,0 +1,22 @@
var path = require('path')

module.exports = {
mode: 'development',
entry: './app.ts',
output: {
filename: 'bundle.js'
},
resolve: {
extensions: ['.ts', '.js']
},
module: {
rules: [
{ test: /(app|lib|common|indirect).+\.ts$/, loader: 'ts-loader', options: { projectReferences: true } },
{ test: /utils.+\.ts$/, loader: 'ts-loader', options: { instance: 'different', projectReferences: true } }
]
}
}

// for test harness purposes only, you would not need this in a normal project
module.exports.resolveLoader = { alias: { 'ts-loader': require('path').join(__dirname, "../../../../index.js") } }

@@ -0,0 +1,3 @@
export function common() {
return 30;
}
@@ -0,0 +1,6 @@
{
"compilerOptions": {
"composite": true,
"types": []
}
}