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.

Sign up for GitHub

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

internal: remove usage of hand crafted webpack typings #927

Merged
merged 4 commits into from
May 1, 2019
Merged

internal: remove usage of hand crafted webpack typings #927

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
db629db
internal: remove usage of hand crafted webpack typings
LukeSheard Apr 28, 2019
2cbbdf3
patch: fix duplicate copies of node typings
LukeSheard Apr 28, 2019
ca07454
update webpack typings and removed patched versions
LukeSheard Apr 28, 2019
7a609f8
prepare 5.4.5 release
LukeSheard Apr 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions examples/react-babel-karma-gulp/yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@
resolved "https://registry.yarnpkg.com/@types/jasmine/-/jasmine-2.6.0.tgz#997b41a27752b4850af2683bc4a8d8222c25bd02"

"@types/node@*":
version "8.0.45"
resolved "https://registry.yarnpkg.com/@types/node/-/node-8.0.45.tgz#89fad82439d5624e1b5c6b42f0f5d85136dcdecc"
version "11.13.8"
resolved "https://registry.yarnpkg.com/@types/node/-/node-11.13.8.tgz#e5d71173c95533be9842b2c798978f095f912aab"
integrity sha512-szA3x/3miL90ZJxUCzx9haNbK5/zmPieGraZEe4WI+3srN0eGLiT22NXeMHmyhNEopn+IrxqMc7wdVwvPl8meg==

"@types/[email protected]":
version "0.31.6"
Expand Down
5 changes: 3 additions & 2 deletions examples/thread-loader/yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@


"@types/node@*":
version "9.4.6"
resolved "https://registry.yarnpkg.com/@types/node/-/node-9.4.6.tgz#d8176d864ee48753d053783e4e463aec86b8d82e"
version "11.13.8"
resolved "https://registry.yarnpkg.com/@types/node/-/node-11.13.8.tgz#e5d71173c95533be9842b2c798978f095f912aab"
integrity sha512-szA3x/3miL90ZJxUCzx9haNbK5/zmPieGraZEe4WI+3srN0eGLiT22NXeMHmyhNEopn+IrxqMc7wdVwvPl8meg==

abbrev@1:
version "1.1.1"
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@
},
"devDependencies": {
"@types/micromatch": "^3.1.0",
"@types/node": "^10.0.0",
"@types/node": "*",
"@types/semver": "^5.4.0",
"@types/webpack": "^4.4.29",
"babel": "^6.0.0",
"babel-core": "^6.0.0",
"babel-loader": "^7.0.0",
Expand Down
45 changes: 25 additions & 20 deletions src/after-compile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import * as path from 'path';
import * as webpack from 'webpack';

import * as constants from './constants';
import { getEmitOutput } from './instances';
import {
TSFile,
TSFiles,
TSInstance,
WebpackCompilation,
WebpackError,
WebpackModule
} from './interfaces';
Expand All @@ -23,7 +23,10 @@ export function makeAfterCompile(
let getCompilerOptionDiagnostics = true;
let checkAllFilesForErrors = true;

return (compilation: WebpackCompilation, callback: () => void) => {
return (
compilation: webpack.compilation.Compilation,
callback: () => void
) => {
// Don't add errors for child compilations
if (compilation.compiler.isChild()) {
callback();
Expand Down Expand Up @@ -76,7 +79,7 @@ export function makeAfterCompile(
*/
function provideCompilerOptionDiagnosticErrorsToWebpack(
getCompilerOptionDiagnostics: boolean,
compilation: WebpackCompilation,
compilation: webpack.compilation.Compilation,
instance: TSInstance,
configFilePath: string | undefined
) {
Expand All @@ -103,23 +106,25 @@ function provideCompilerOptionDiagnosticErrorsToWebpack(
* this is used for quick-lookup when trying to find modules
* based on filepath
*/
function determineModules(compilation: WebpackCompilation) {
// TODO: Convert to reduce
const modules = new Map<string, WebpackModule[]>();
compilation.modules.forEach(module => {
if (module.resource) {
const modulePath = path.normalize(module.resource);
const existingModules = modules.get(modulePath);
if (existingModules !== undefined) {
if (existingModules.indexOf(module) === -1) {
existingModules.push(module);
function determineModules(compilation: webpack.compilation.Compilation) {
return compilation.modules.reduce<Map<string, WebpackModule[]>>(
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

(modules, module) => {
if (module.resource) {
const modulePath = path.normalize(module.resource);
const existingModules = modules.get(modulePath);
if (existingModules !== undefined) {
if (existingModules.indexOf(module) === -1) {
existingModules.push(module);
}
} else {
modules.set(modulePath, [module]);
}
} else {
modules.set(modulePath, [module]);
}
}
});
return modules;

return modules;
},
new Map<string, WebpackModule[]>()
);
}

function determineFilesToCheckForErrors(
Expand Down Expand Up @@ -163,7 +168,7 @@ function determineFilesToCheckForErrors(
function provideErrorsToWebpack(
filesToCheckForErrors: TSFiles,
filesWithErrors: TSFiles,
compilation: WebpackCompilation,
compilation: webpack.compilation.Compilation,
modules: Map<string, WebpackModule[]>,
instance: TSInstance
) {
Expand Down Expand Up @@ -255,7 +260,7 @@ function provideErrorsToWebpack(
function provideDeclarationFilesToWebpack(
filesToCheckForErrors: TSFiles,
instance: TSInstance,
compilation: WebpackCompilation
compilation: webpack.compilation.Compilation
) {
for (const filePath of filesToCheckForErrors.keys()) {
if (filePath.match(constants.tsTsxRegex) === null) {
Expand Down
5 changes: 2 additions & 3 deletions src/compilerSetup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as semver from 'semver';
import * as typescript from 'typescript';

import * as constants from './constants';
import { LoaderOptions } from './interfaces';
import * as logger from './logger';

Expand Down Expand Up @@ -66,9 +65,9 @@ export function getCompilerOptions(
if (
compilerOptions.module === undefined &&
(compilerOptions.target !== undefined &&
compilerOptions.target < constants.ScriptTargetES2015)
compilerOptions.target < typescript.ScriptTarget.ES2015)
) {
compilerOptions.module = constants.ModuleKindCommonJs;
compilerOptions.module = typescript.ModuleKind.CommonJS;
}

return compilerOptions;
Expand Down
6 changes: 4 additions & 2 deletions src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { Chalk } from 'chalk';
import * as path from 'path';
import * as typescript from 'typescript';
import { LoaderOptions, Webpack, WebpackError } from './interfaces';
import * as webpack from 'webpack';

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

Expand All @@ -13,7 +15,7 @@ interface ConfigFile {
export function getConfigFile(
compiler: typeof typescript,
colors: Chalk,
loader: Webpack,
loader: webpack.loader.LoaderContext,
loaderOptions: LoaderOptions,
compilerCompatible: boolean,
log: logger.Logger,
Expand Down
4 changes: 0 additions & 4 deletions src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ export const LineFeed = '\n';
export const CarriageReturnLineFeedCode = 0;
export const LineFeedCode = 1;

export const ScriptTargetES2015 = 2;
Copy link
Member

Choose a reason for hiding this comment

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

So interestingly the moving of using locally defined constants to directly depending on TypeScript would mark the first direct runtime (rather than compilation time) dependency on the typescript module. All the others (I think) are type dependencies.

I believe, back in the mists of time, this was an intentional choice by ts-loader's @jbrantly. I think the reasons were to do with ts-loader working with multiple versions of TypeScript - not bolted to a specific one that you build with. It may been related to the ability to supply your own version of TypeScript. Remember ntypescript?

https://github.com/TypeStrong/ts-loader/blob/master/README.md#compiler-string-defaulttypescript

See also:

ts-loader/src/compilerSetup.ts

Line 15 in 48626a9

compiler = require(loaderOptions.compiler);

I don't know if this matters anymore. The values of those enums are pretty much set in stone now.

I'd appreciate any thoughts on this - it's probably fine. But I've a feeling that other views may be available too.

Also @arcanis - would this create issues for yarn PnP support?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also @arcanis - would this create issues for yarn PnP support?

Not if typescript is defined as either dependencies or peerDependencies (devDependencies won't do since they won't be considered part of the dependency tree when installed by consumers).

Copy link
Member

Choose a reason for hiding this comment

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

Cool - it's already a peerDependency so I guess we're good

ts-loader/package.json

Line 94 in 48626a9

"peerDependencies": { 

  "peerDependencies": {
    "typescript": "*"
  }


export const ModuleKindCommonJs = 1;

export const extensionRegex = /\.[^.]+$/;
export const tsxRegex = /\.tsx$/i;
export const tsTsxRegex = /\.ts(x?)$/i;
Expand Down
28 changes: 13 additions & 15 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
import * as loaderUtils from 'loader-utils';
import * as path from 'path';
import * as typescript from 'typescript';
import * as webpack from 'webpack';

import * as constants from './constants';
import { getEmitOutput, getTypeScriptInstance } from './instances';
import {
AsyncCallback,
Compiler,
LoaderOptions,
LoaderOptionsCache,
LogLevel,
TSFile,
TSInstance,
Webpack
TSInstance
} from './interfaces';
import {
appendSuffixesIfMatch,
Expand All @@ -23,16 +21,16 @@ import {
validateSourceMapOncePerProject
} from './utils';

const webpackInstances: Compiler[] = [];
const webpackInstances: webpack.Compiler[] = [];
const loaderOptionsCache: LoaderOptionsCache = {};

/**
* The entry point for ts-loader
*/
function loader(this: Webpack, contents: string) {
function loader(this: webpack.loader.LoaderContext, contents: string) {
// tslint:disable-next-line:no-unused-expression strict-boolean-expressions
this.cacheable && this.cacheable();
const callback = this.async();
const callback = this.async() as webpack.loader.loaderCallback;
const options = getLoaderOptions(this);
const instanceOrError = getTypeScriptInstance(options, this);

Expand All @@ -51,9 +49,9 @@ function loader(this: Webpack, contents: string) {
}

function successLoader(
loaderContext: Webpack,
loaderContext: webpack.loader.LoaderContext,
contents: string,
callback: AsyncCallback,
callback: webpack.loader.loaderCallback,
options: LoaderOptions,
instance: TSInstance
) {
Expand Down Expand Up @@ -157,10 +155,10 @@ function makeSourceMapAndFinish(
outputText: string | undefined,
filePath: string,
contents: string,
loaderContext: Webpack,
loaderContext: webpack.loader.LoaderContext,
options: LoaderOptions,
fileVersion: number,
callback: AsyncCallback
callback: webpack.loader.loaderCallback
) {
if (outputText === null || outputText === undefined) {
const additionalGuidance =
Expand Down Expand Up @@ -198,7 +196,7 @@ function makeSourceMapAndFinish(
* either retrieves loader options from the cache
* or creates them, adds them to the cache and returns
*/
function getLoaderOptions(loaderContext: Webpack) {
function getLoaderOptions(loaderContext: webpack.loader.LoaderContext) {
// differentiate the TypeScript instance based on the webpack instance
let webpackIndex = webpackInstances.indexOf(loaderContext._compiler);
if (webpackIndex === -1) {
Expand Down Expand Up @@ -390,7 +388,7 @@ function getEmit(
rawFilePath: string,
filePath: string,
instance: TSInstance,
loaderContext: Webpack
loaderContext: webpack.loader.LoaderContext
) {
const outputFiles = getEmitOutput(instance, filePath);

Expand Down Expand Up @@ -460,7 +458,7 @@ function getTranspilationEmit(
fileName: string,
contents: string,
instance: TSInstance,
loaderContext: Webpack
loaderContext: webpack.loader.LoaderContext
) {
const {
outputText,
Expand Down Expand Up @@ -495,7 +493,7 @@ function makeSourceMap(
outputText: string,
filePath: string,
contents: string,
loaderContext: Webpack
loaderContext: webpack.loader.LoaderContext
) {
if (sourceMapText === undefined) {
return { output: outputText, sourceMap: undefined };
Expand Down
6 changes: 3 additions & 3 deletions src/instances.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import chalk, { Chalk } from 'chalk';
import * as fs from 'fs';
import * as path from 'path';
import * as typescript from 'typescript';
import * as webpack from 'webpack';

import { makeAfterCompile } from './after-compile';
import { getCompiler, getCompilerOptions } from './compilerSetup';
Expand All @@ -13,7 +14,6 @@ import {
TSFiles,
TSInstance,
TSInstances,
Webpack,
WebpackError
} from './interfaces';
import * as logger from './logger';
Expand All @@ -38,7 +38,7 @@ const instances = {} as TSInstances;
*/
export function getTypeScriptInstance(
loaderOptions: LoaderOptions,
loader: Webpack
loader: webpack.loader.LoaderContext
): { instance?: TSInstance; error?: WebpackError } {
if (instances.hasOwnProperty(loaderOptions.instance)) {
const instance = instances[loaderOptions.instance];
Expand Down Expand Up @@ -67,7 +67,7 @@ export function getTypeScriptInstance(

function successfulTypeScriptInstance(
loaderOptions: LoaderOptions,
loader: Webpack,
loader: webpack.loader.LoaderContext,
log: logger.Logger,
colors: Chalk,
compiler: typeof typescript,
Expand Down
Loading