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

Assist: ts-loader#945 #1

Closed
Closed
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
6 changes: 3 additions & 3 deletions examples/fork-ts-checker-webpack-plugin/yarn.lock
Expand Up @@ -2293,9 +2293,9 @@ js-tokens@^3.0.2:
integrity sha1-mGbfOVECEw449/mWvOtlRDIJwls=

js-yaml@^3.7.0:
version "3.12.1"
resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-3.12.1.tgz#295c8632a18a23e054cf5c9d3cecafe678167600"
integrity sha512-um46hB9wNOKlwkHgiuyEVAybXBjwFUV0Z/RaHJblRd9DXltue9FTYvzCr9ErQrK9Adz5MU4gHWVaNUfdmrC8qA==
version "3.13.1"
resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-3.13.1.tgz#aff151b30bfdfa8e49e05da22e7415e9dfa37847"
integrity sha512-YfbcO7jXDdyj0DGxYVSlSeQNHbD7XPWvrVWeVUujrQEoZzWJIRrCPoyk6kL6IAjAG2IolMK4T0hNUe0HOUs5Jw==
dependencies:
argparse "^1.0.7"
esprima "^4.0.0"
Expand Down
53 changes: 50 additions & 3 deletions src/config.ts
Expand Up @@ -6,7 +6,7 @@ import * as webpack from 'webpack';

import { ConfigFile, LoaderOptions, WebpackError } from './interfaces';
import * as logger from './logger';
import { formatErrors } from './utils';
import { formatErrors, getPossiblyRenamedFilePath } from './utils';

export function getConfigFile(
compiler: typeof typescript,
Expand Down Expand Up @@ -122,11 +122,12 @@ export function getConfigParseResult(
compiler: typeof typescript,
configFile: ConfigFile,
basePath: string,
configFilePath: string | undefined
configFilePath: string | undefined,
loaderOptions: LoaderOptions
) {
const configParseResult = compiler.parseJsonConfigFileContent(
configFile.config,
compiler.sys,
getSuffixAppendingParseConfigHost(compiler.sys, loaderOptions),
basePath
);

Expand All @@ -139,3 +140,49 @@ export function getConfigParseResult(

return configParseResult;
}

function getSuffixAppendingParseConfigHost(
Copy link
Author

Choose a reason for hiding this comment

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

This is the heart of my changes. It solves the appendSuffix problem by providing TypeScript an interface to the file system that actually acts on the file names that ts-loader tracks, i.e. renamed with the .ts or .tsx suffix appropriately. The result is that configParseResult.fileNames will include things like foo.vue.tsx when ts-loader is told to assume foo.vue is a TSX file.

So I think this is actually a case where we can reliably be certain that the files that exist will not be in the tsconfig.json and in fact cannot be.

Au contraire mon frère, @johnnyreilly 😄

Copy link
Owner

Choose a reason for hiding this comment

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

This is the heart of my changes. It solves the appendSuffix problem by providing TypeScript an interface to the file system that actually acts on the file names that ts-loader tracks

That's clever! But could this be a bit unintuitive for the users? Some patterns would match actual files but not transformed files. For instance, patterns like src/**/*.vue, or just main.vue inside "files". Right?

Of course the user can specify patterns on the transformed files. But then the appendSuffix is not that transparent anymore. Maybe that's not wrong, but still pretty confusing. 🤔

sys: typescript.System,
loaderOptions: LoaderOptions
): typescript.ParseConfigHost {
if (
!loaderOptions.appendTsSuffixTo.length &&
!loaderOptions.appendTsxSuffixTo.length
) {
return sys;
}

return {
useCaseSensitiveFileNames: sys.useCaseSensitiveFileNames,
fileExists: mapArg(sys.fileExists, transformFileName),
readFile: mapArg(sys.readFile, transformFileName),
readDirectory: (rootDir, extensions, ...rest) => {
const allFiles = sys.readDirectory(rootDir);
const customExtensions = allFiles.reduce((exts: string[], fileName) => {
const renamed = transformFileName(fileName);
if (
renamed !== fileName &&
extensions.indexOf(path.extname(renamed).toLowerCase()) > -1
) {
const ext = path.extname(fileName).toLowerCase();
if (ext) {
exts.push(ext);
}
}
return exts;
}, []);

return sys
.readDirectory(rootDir, extensions.concat(customExtensions), ...rest)
.map(transformFileName);
}
};

function transformFileName(rawFileName: string) {
return getPossiblyRenamedFilePath(rawFileName, loaderOptions);
}
}

function mapArg<T, U>(toWrap: (x: T) => U, wrapWith: (x: T) => T): (x: T) => U {
return x => toWrap(wrapWith(x));
}
68 changes: 30 additions & 38 deletions src/index.ts
Expand Up @@ -18,11 +18,12 @@ import {
TSInstance
} from './interfaces';
import {
appendSuffixesIfMatch,
arrify,
formatErrors,
getAndCacheOutputJSFileName,
getAndCacheProjectReference,
getPossiblyRenamedFilePath,
isRootFileOrExempt,
validateSourceMapOncePerProject
} from './utils';

Expand Down Expand Up @@ -62,35 +63,15 @@ function successLoader(
) {
const rawFilePath = path.normalize(loaderContext.resourcePath);

const filePath =
options.appendTsSuffixTo.length > 0 || options.appendTsxSuffixTo.length > 0
? appendSuffixesIfMatch(
{
'.ts': options.appendTsSuffixTo,
'.tsx': options.appendTsxSuffixTo
},
rawFilePath
)
: rawFilePath;
const filePath = getPossiblyRenamedFilePath(rawFilePath, options);

const { version: fileVersion, changedFilesList } = updateFileInCache(
filePath,
contents,
instance
instance,
options
);

if (changedFilesList && !instance.rootFileNames.has(filePath)) {
reloadRootFileNamesFromConfig(loaderContext, instance);
if (
!instance.rootFileNames.has(filePath) &&
!options.onlyCompileBundledFiles
) {
throw new Error(
`The file ${filePath} is not part of the project specified in tsconfig.json. Make sure it is covered by the fields 'include', 'exclude' and 'files'.`
);
}
}

Copy link
Author

Choose a reason for hiding this comment

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

Moved this to come after project reference handling

const referencedProject = getAndCacheProjectReference(filePath, instance);
if (referencedProject !== undefined) {
const [relativeProjectConfigPath, relativeFilePath] = [
Expand Down Expand Up @@ -155,6 +136,18 @@ function successLoader(
callback
);
} else {
if (changedFilesList && !isRootFileOrExempt(filePath, instance)) {
reloadRootFileNamesFromConfig(loaderContext, instance, options);
if (
!instance.rootFileNames.has(filePath) &&
!options.onlyCompileBundledFiles
) {
throw new Error(
`The file ${filePath} is not part of the project specified in tsconfig.json. Make sure it is covered by the fields 'include', 'exclude' and 'files'.`
);
}
}

const { outputText, sourceMapText } = options.transpileOnly
? getTranspilationEmit(filePath, contents, instance, loaderContext)
: getEmit(rawFilePath, filePath, instance, loaderContext);
Expand All @@ -178,28 +171,23 @@ function successLoader(
*/
function reloadRootFileNamesFromConfig(
loaderContext: webpack.loader.LoaderContext,
instance: TSInstance
instance: TSInstance,
options: LoaderOptions
Copy link
Author

Choose a reason for hiding this comment

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

instance.loaderOptions !== options, ouch 😖 Pretty sure we do this wrong in lots more places, but this one was needed to get tests passing

) {
const {
compiler,
basePath,
configFile,
configFilePath,
colors,
loaderOptions
} = instance;
const { compiler, basePath, configFile, configFilePath, colors } = instance;

const configParseResult = getConfigParseResult(
compiler,
configFile,
basePath,
configFilePath
configFilePath,
options
);

if (configParseResult.errors.length > 0 && !loaderOptions.happyPackMode) {
if (configParseResult.errors.length > 0 && !options.happyPackMode) {
const errors = formatErrors(
configParseResult.errors,
loaderOptions,
options,
colors,
compiler,
{ file: configFilePath },
Expand All @@ -211,7 +199,7 @@ function reloadRootFileNamesFromConfig(
throw new Error(colors.red('error while parsing tsconfig.json'));
}

updateInstanceRootFileNames(loaderOptions, instance, configParseResult);
updateInstanceRootFileNames(options, instance, configParseResult);

return;
}
Expand Down Expand Up @@ -400,7 +388,8 @@ function makeLoaderOptions(instanceName: string, loaderOptions: LoaderOptions) {
function updateFileInCache(
filePath: string,
contents: string,
instance: TSInstance
instance: TSInstance,
options: LoaderOptions
) {
let fileWatcherEventKind: typescript.FileWatcherEventKind | undefined;
let changedFilesList = false;
Expand Down Expand Up @@ -432,6 +421,9 @@ function updateFileInCache(
//
if (!instance.rootFileNames.has(filePath)) {
instance.version!++;
if (options.onlyCompileBundledFiles) {
instance.rootFileNames.add(filePath);
Copy link
Author

Choose a reason for hiding this comment

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

Need to add the file manually, otherwise rootFileNames will just be declarations

}
}

if (instance.watchHost !== undefined && contents === undefined) {
Expand Down
9 changes: 7 additions & 2 deletions src/instances.ts
Expand Up @@ -22,6 +22,7 @@ import {
appendSuffixesIfMatch,
ensureProgram,
formatErrors,
getOriginalFilePath,
isUsingProjectReferences,
makeError
} from './utils';
Expand Down Expand Up @@ -100,7 +101,8 @@ function successfulTypeScriptInstance(
compiler,
configFile,
basePath,
configFilePath
configFilePath,
loaderOptions
);

if (configParseResult.errors.length > 0 && !loaderOptions.happyPackMode) {
Expand Down Expand Up @@ -254,7 +256,10 @@ function successfulTypeScriptInstance(
instance.rootFileNames.forEach(filePath => {
normalizedFilePath = path.normalize(filePath);
files.set(normalizedFilePath, {
text: fs.readFileSync(normalizedFilePath, 'utf-8'),
text: fs.readFileSync(
getOriginalFilePath(normalizedFilePath, loaderOptions),
'utf-8'
),
version: 0
});
});
Expand Down
5 changes: 2 additions & 3 deletions src/servicesHost.ts
Expand Up @@ -96,8 +96,7 @@ export function makeServicesHost(

getProjectReferences: () => projectReferences,

getScriptFileNames: () =>
[...files.keys()].filter(filePath => filePath.match(scriptRegex)),
getScriptFileNames: () => Array.from(instance.rootFileNames),
Copy link
Author

Choose a reason for hiding this comment

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

This was the change I mentioned before that fixes TypeStrong#949. Doing this exposed a couple other problems, like needing to add the file to instance.rootFileNames when onlyCompileBundledFiles is true.


getScriptVersion: (fileName: string) => {
fileName = path.normalize(fileName);
Expand Down Expand Up @@ -351,7 +350,7 @@ export function makeWatchHost(
return watchHost;

function getRootFileNames() {
return [...files.keys()].filter(filePath => filePath.match(scriptRegex));
return Array.from(instance.rootFileNames);
}

function readFileWithCachingText(fileName: string, encoding?: string) {
Expand Down
76 changes: 76 additions & 0 deletions src/utils.ts
Expand Up @@ -136,6 +136,67 @@ export function makeError(
};
}

export function getPossiblyRenamedFilePath(
rawFilePath: string,
loaderOptions: LoaderOptions
) {
return loaderOptions.appendTsSuffixTo.length > 0 ||
loaderOptions.appendTsxSuffixTo.length > 0
? appendSuffixesIfMatch(
{
'.ts': loaderOptions.appendTsSuffixTo,
'.tsx': loaderOptions.appendTsxSuffixTo
},
rawFilePath
)
: rawFilePath;
}

export function getOriginalFilePath(
possiblyRenamedFilePath: string,
loaderOptions: LoaderOptions
) {
if (
loaderOptions.appendTsSuffixTo.length &&
possiblyRenamedFilePath.endsWith(typescript.Extension.Ts)
) {
const maybeOriginal = stripEnd(
possiblyRenamedFilePath,
typescript.Extension.Ts.length
);
const renamedAgain = appendSuffixIfMatch(
loaderOptions.appendTsSuffixTo,
maybeOriginal,
typescript.Extension.Ts
);
if (renamedAgain === possiblyRenamedFilePath) {
return maybeOriginal;
}
}
if (
loaderOptions.appendTsxSuffixTo.length &&
possiblyRenamedFilePath.endsWith(typescript.Extension.Tsx)
) {
const maybeOriginal = stripEnd(
possiblyRenamedFilePath,
typescript.Extension.Tsx.length
);
const renamedAgain = appendSuffixIfMatch(
loaderOptions.appendTsxSuffixTo,
maybeOriginal,
typescript.Extension.Tsx
);
if (renamedAgain === possiblyRenamedFilePath) {
return maybeOriginal;
}
}
return possiblyRenamedFilePath;
}

export function stripEnd(str: string, lengthToRemove: number) {
return str.slice(0, str.length - lengthToRemove);
}

export function appendSuffixIfMatch(
patterns: RegExp[],
filePath: string,
Expand Down Expand Up @@ -255,6 +316,21 @@ export function ensureProgram(instance: TSInstance) {
return instance.program;
}

export function isRootFileOrExempt(
fileName: string,
instance: TSInstance
): boolean {
// node_modules checking handled separately, later
if (
instance.rootFileNames.has(fileName) ||
fileName.indexOf('node_modules') > -1
) {
return true;
}

return false;
}

export function supportsProjectReferences(instance: TSInstance) {
const program = ensureProgram(instance);
return program && !!program.getProjectReferences;
Expand Down
5 changes: 1 addition & 4 deletions test/comparison-tests/appendSuffixTo/tsconfig.json
@@ -1,7 +1,4 @@
{
"compilerOptions": {
},
"files": [
"index.vue.ts"
]
}
}
@@ -1,8 +1 @@
{
"compilerOptions": {
"include": [
"node_modules",
"app.ts"
]
}
}
{}
3 changes: 2 additions & 1 deletion test/execution-tests/3.0.1_projectReferences/tsconfig.json
@@ -1,6 +1,7 @@
{
"files": [
"./src/app.ts"
"./src/app.ts",
"./test/app.tests.ts"
],
"references": [
{ "path": "./lib" }
Expand Down