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

Implements a custom resolveModuleName option #862

Merged
merged 11 commits into from
Oct 31, 2018
Merged
Show file tree
Hide file tree
Changes from 8 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: 2 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,8 @@ const validLoaderOptions: ValidLoaderOptions[] = [
'experimentalWatchApi',
'allowTsInNodeModules',
'experimentalFileCaching',
'projectReferences'
'projectReferences',
'resolveModuleName'
];

/**
Expand Down
16 changes: 16 additions & 0 deletions src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,21 @@ export interface ReverseDependencyGraph {

export type LogLevel = 'INFO' | 'WARN' | 'ERROR';

export type ResolveModuleName = (
moduleName: string,
containingFile: string,
compilerOptions: typescript.CompilerOptions,
moduleResolutionHost: typescript.ModuleResolutionHost,
) => typescript.ResolvedModuleWithFailedLookupLocations;

export type CustomResolveModuleName = (
moduleName: string,
containingFile: string,
compilerOptions: typescript.CompilerOptions,
moduleResolutionHost: typescript.ModuleResolutionHost,
parentResolver: ResolveModuleName
) => typescript.ResolvedModuleWithFailedLookupLocations;

export interface LoaderOptions {
silent: boolean;
logLevel: LogLevel;
Expand All @@ -320,6 +335,7 @@ export interface LoaderOptions {
allowTsInNodeModules: boolean;
experimentalFileCaching: boolean;
projectReferences: boolean;
resolveModuleName?: CustomResolveModuleName;
}

export interface TSFile {
Expand Down
56 changes: 45 additions & 11 deletions src/servicesHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as typescript from 'typescript';

import * as constants from './constants';
import {
CustomResolveModuleName,
ModuleResolutionHost,
ResolvedModule,
ResolveSync,
Expand Down Expand Up @@ -36,7 +37,11 @@ export function makeServicesHost(
compiler,
compilerOptions,
files,
loaderOptions: { appendTsSuffixTo, appendTsxSuffixTo }
loaderOptions: {
appendTsSuffixTo,
appendTsxSuffixTo,
resolveModuleName: customResolveModuleName
}
} = instance;

const newLine =
Expand Down Expand Up @@ -148,7 +153,8 @@ export function makeServicesHost(
instance,
moduleNames,
containingFile,
getResolutionStrategy
getResolutionStrategy,
customResolveModuleName
),

getCustomTransformers: () => instance.transformers
Expand Down Expand Up @@ -426,7 +432,8 @@ function resolveModuleNames(
instance: TSInstance,
moduleNames: string[],
containingFile: string,
resolutionStrategy: ResolutionStrategy
resolutionStrategy: ResolutionStrategy,
customResolveModuleName?: CustomResolveModuleName
) {
const resolvedModules = moduleNames.map(moduleName =>
resolveModuleName(
Expand All @@ -438,7 +445,8 @@ function resolveModuleNames(
instance,
moduleName,
containingFile,
resolutionStrategy
resolutionStrategy,
customResolveModuleName
)
);

Expand All @@ -457,6 +465,21 @@ function isJsImplementationOfTypings(
);
}

function applyTsResolver(
compiler: typeof typescript,
moduleName: string,
containingFile: string,
compilerOptions: typescript.CompilerOptions,
moduleResolutionHost: typescript.ModuleResolutionHost
) {
return compiler.resolveModuleName(
moduleName,
containingFile,
compilerOptions,
moduleResolutionHost
);
}

function resolveModuleName(
resolveSync: ResolveSync,
moduleResolutionHost: ModuleResolutionHost,
Expand All @@ -466,7 +489,8 @@ function resolveModuleName(
instance: TSInstance,
moduleName: string,
containingFile: string,
resolutionStrategy: ResolutionStrategy
resolutionStrategy: ResolutionStrategy,
customResolveModuleName?: CustomResolveModuleName
) {
const { compiler, compilerOptions } = instance;

Expand Down Expand Up @@ -496,12 +520,22 @@ function resolveModuleName(
// tslint:disable-next-line:no-empty
} catch (e) {}

const tsResolution = compiler.resolveModuleName(
moduleName,
containingFile,
compilerOptions,
moduleResolutionHost
);
const tsResolution =
customResolveModuleName !== undefined
? customResolveModuleName(
moduleName,
containingFile,
compilerOptions,
moduleResolutionHost,
applyTsResolver.bind(null, compiler)
Copy link
Member

Choose a reason for hiding this comment

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

Seeing a .bind my perf spider sense is tingling!

https://stackoverflow.com/questions/42117911/lambda-functions-vs-bind-memory-and-performance

I wonder if we could switch this to a lamba instead? It'll be slightly more verbose but hopefully should perform better...

)
: applyTsResolver(
compiler,
moduleName,
containingFile,
compilerOptions,
moduleResolutionHost
);

if (tsResolution.resolvedModule !== undefined) {
const resolvedFileName = path.normalize(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
/*! no static exports found */
/***/ (function(module, exports) {

eval("throw new Error(\"Module build failed (from C:/source/ts-loader/index.js):/nError: ts-loader was supplied with an unexpected loader option: notRealOption/n/nPlease take a look at the options you are supplying; the following are valid options:/nsilent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules / experimentalFileCaching / projectReferences/n/n at validateLoaderOptions (C://source//ts-loader//dist//index.js:151:19)/n at getLoaderOptions (C://source//ts-loader//dist//index.js:110:5)/n at Object.loader (C://source//ts-loader//dist//index.js:16:21)\");\n\n//# sourceURL=webpack:///./app.ts?");
eval("throw new Error(\"Module build failed (from C:/source/ts-loader/index.js):/nError: ts-loader was supplied with an unexpected loader option: notRealOption/n/nPlease take a look at the options you are supplying; the following are valid options:/nsilent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules / experimentalFileCaching / projectReferences / resolveModuleName/n/n at validateLoaderOptions (C://source//ts-loader//dist//index.js:152:19)/n at getLoaderOptions (C://source//ts-loader//dist//index.js:110:5)/n at Object.loader (C://source//ts-loader//dist//index.js:16:21)\");\n\n//# sourceURL=webpack:///./app.ts?");

/***/ })

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
Asset Size Chunks Chunk Names
bundle.js 4.58 KiB main [emitted] main
Asset Size Chunks Chunk Names
bundle.js 4.6 KiB main [emitted] main
Entrypoint main = bundle.js
[./app.ts] 835 bytes {main} [built] [failed] [1 error]
[./app.ts] 855 bytes {main} [built] [failed] [1 error]

ERROR in ./app.ts
Module build failed (from /index.js):
Error: ts-loader was supplied with an unexpected loader option: notRealOption

Please take a look at the options you are supplying; the following are valid options:
silent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules / experimentalFileCaching / projectReferences
silent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules / experimentalFileCaching / projectReferences / resolveModuleName

at validateLoaderOptions (dist\index.js:151:19)
at validateLoaderOptions (dist\index.js:152:19)
at getLoaderOptions (dist\index.js:110:5)
at Object.loader (dist\index.js:16:21)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import {HelloWorld} from 'foo';

export type HelloBuilder = (hello: number, world: number) => HelloWorld;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "bar",
"version": "1.0.0",
"typings": "./index.d.ts"
}
6 changes: 6 additions & 0 deletions test/execution-tests/3.0.1_resolveModuleName/baz-pkg/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import {HelloBuilder} from 'bar';

export const makeHello: HelloBuilder = (hello: number, world: number) => ({
hello,
world,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "baz",
"version": "1.0.0",
"main": "index.ts"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export type HelloWorld = {
hello: number,
world: number,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "foo",
"version": "1.0.0",
"typings": "./index.d.ts"
}
47 changes: 47 additions & 0 deletions test/execution-tests/3.0.1_resolveModuleName/karma.conf.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/* eslint-disable no-var, strict */
'use strict';
var path = require('path');
var webpack = require('webpack');
var webpackConfig = require('./webpack.config.js');
var reporterOptions = require('../../reporterOptions');

module.exports = function(config) {
config.set({
browsers: [ 'ChromeHeadless' ],

files: [
// This loads all the tests
'main.js'
],

port: 9876,

frameworks: [ 'jasmine' ],

logLevel: config.LOG_INFO, //config.LOG_DEBUG

preprocessors: {
'main.js': [ 'webpack', 'sourcemap' ]
},

webpack: {
devtool: 'inline-source-map',
mode: webpackConfig.mode,
module: webpackConfig.module,
resolve: webpackConfig.resolve,

// for test harness purposes only, you would not need this in a normal project
resolveLoader: webpackConfig.resolveLoader
},

webpackMiddleware: {
quiet: true,
stats: {
colors: true
}
},

// reporter options
mochaReporter: reporterOptions
});
};
2 changes: 2 additions & 0 deletions test/execution-tests/3.0.1_resolveModuleName/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const testsContext = require.context('./', true, /\.tests\.ts(x?)$/);
testsContext.keys().forEach(testsContext);
10 changes: 10 additions & 0 deletions test/execution-tests/3.0.1_resolveModuleName/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "basic",
"license": "MIT",
"version": "1.0.0",
"main": "index.js",
"devDependencies": {
"@types/jasmine": "^2.5.35",
"jasmine-core": "^2.3.4"
}
}
4 changes: 4 additions & 0 deletions test/execution-tests/3.0.1_resolveModuleName/src/app.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import {HelloWorld} from 'foo';
import {makeHello} from 'baz';

export const def: HelloWorld = makeHello(1, 2);
13 changes: 13 additions & 0 deletions test/execution-tests/3.0.1_resolveModuleName/test/app.tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import * as app from '../src/app';

// We don't actually care about the result of the following operations;
Copy link
Member

Choose a reason for hiding this comment

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

This is not true actually; we do care about the result of the following operations! Can we amend the comment to make test intent clearer please? We care both about type resolution and that the code executes...

// what we care about is typescript resolving modules using our custom
// logic, allowing this code to compile without errors, hence this:
// "noEmitOnError": true
// in tsconfig.json
describe("app", () => {
it("app.def to have been setup", () => {
expect(app.def.hello).toEqual(1);
expect(app.def.world).toEqual(2);
});
});
5 changes: 5 additions & 0 deletions test/execution-tests/3.0.1_resolveModuleName/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"compilerOptions": {
"noEmitOnError": true
}
}
30 changes: 30 additions & 0 deletions test/execution-tests/3.0.1_resolveModuleName/webpack.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
var path = require('path')

module.exports = {
mode: 'development',
entry: './app.ts',
output: {
filename: 'bundle.js'
},
resolve: {
extensions: ['.ts', '.js'],
alias: { baz: path.join(__dirname, 'baz-pkg') },
},
module: {
rules: [
{ test: /\.ts$/, loader: 'ts-loader', options: {
resolveModuleName: (moduleName, containingFile, compilerOptions, compilerHost, parentResolver) => {
switch (moduleName) {
case 'foo': return parentResolver(path.join(__dirname, 'foo-pkg'), containingFile, compilerOptions, compilerHost);
case 'bar': return parentResolver(path.join(__dirname, 'bar-pkg'), containingFile, compilerOptions, compilerHost);
case 'baz': return parentResolver(path.join(__dirname, 'baz-pkg'), containingFile, compilerOptions, compilerHost);
default: return parentResolver(moduleName, containingFile, compilerOptions, compilerHost);
}
},
} }
]
}
}

// for test harness purposes only, you would not need this in a normal project
module.exports.resolveLoader = { alias: { 'ts-loader': path.join(__dirname, "../../../index.js") } }
11 changes: 11 additions & 0 deletions test/execution-tests/3.0.1_resolveModuleName/yarn.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


"@types/jasmine@^2.5.35":
version "2.8.5"
resolved "https://registry.yarnpkg.com/@types/jasmine/-/jasmine-2.8.5.tgz#96e58872583fa80c7ea0dd29024b180d5e133678"

jasmine-core@^2.3.4:
version "2.9.1"
resolved "https://registry.yarnpkg.com/jasmine-core/-/jasmine-core-2.9.1.tgz#b6bbc1d8e65250d56f5888461705ebeeeb88f22f"