internal: remove usage of hand crafted webpack typings #927
Conversation
src/after-compile.ts
Outdated
| @@ -16,14 +16,24 @@ import { | |||
| isUsingProjectReferences | |||
| } from './utils'; | |||
|
|
|||
| interface PatchedCompiler extends webpack.Compiler { | |||
There was a problem hiding this comment.
Bug #1 where the following properties are not available on a compiler instance.
src/instances.ts
Outdated
| @@ -29,6 +29,9 @@ import { makeWatchRun } from './watch-run'; | |||
|
|
|||
| const instances = {} as TSInstances; | |||
|
|
|||
| // TODO: workaround for issues with webpack typings | |||
| type PatchedHookCallback = any; | |||
LukeSheard
force-pushed
the
webpack-typings
branch
from
e162193
to
2cbbdf3
Compare
|
Nice! I've merged the changes in definitely typed... Let's see where this goes! 😀 |
I think we just merged them in the wrong place facepalm Moved to the correct place: DefinitelyTyped/DefinitelyTyped#35056 |
|
Merged! |
|
Updated the PR with the latest typings package! Passes locally for me, so should be good once Travis passes. |
| if (existingModules.indexOf(module) === -1) { | ||
| existingModules.push(module); | ||
| function determineModules(compilation: webpack.compilation.Compilation) { | ||
| return compilation.modules.reduce<Map<string, WebpackModule[]>>( |
| @@ -7,10 +7,6 @@ export const LineFeed = '\n'; | |||
| export const CarriageReturnLineFeedCode = 0; | |||
| export const LineFeedCode = 1; | |||
|
|
|||
| export const ScriptTargetES2015 = 2; | |||
There was a problem hiding this comment.
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
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?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Cool - it's already a peerDependency so I guess we're good
Line 94 in 48626a9
"peerDependencies": {
"typescript": "*"
}
|
So I think we're good to merge and ship! Could you update the |
|
Will do! |
|
Done! |
|
Nice! Thanks for your work! ❤️ |
|
Should be released shortly https://github.com/TypeStrong/ts-loader/releases/tag/v5.4.5 |
Some tidy up of the internals to use the
@types/webpackwebpack package for webpack specific typings.As far as I can see there are two bugs in the webpack typings which I'll try and upstream: DefinitelyTyped/DefinitelyTyped#35053