feat: add LoaderContext to types

[johnnyreilly]

This is a speculative PR to resolve #13162 by strongly typing LoaderContext in webpack. - This is possibly not complete yet but I wanted to have a go at what this might look like and see if I was headed in the right direction.

Related PR where ts-loader starts to consume webpack 5 types: TypeStrong/ts-loader#1251

What kind of change does this PR introduce?

It adds a type.

Did you add tests for your changes?

It's a type that's exposed rather than a runtime code change - I don't think there are tests for this beyond compilation?

Does this PR introduce a breaking change?

No

What needs to be documented once your changes are merged?

Nothing - just the types that webpack exposes will be richer.

[webpack-bot]

For maintainers only:

• This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
• This needs to be backported to webpack 4 (issue will be created when merged)

[johnnyreilly]

This isn't a complete type yet as it misses the async method on the loader context. You can see what usage of that looks like here:

https://github.com/TypeStrong/ts-loader/blob/bada1e41e8e484e3fd88fd77ab4217745f2083ee/src/index.ts#L38

Could anyone point me in the direction of where that gets created in webpack? I've done a bunch of code spelunking but haven't found it yet.

Found it in loader-runner here: https://github.com/webpack/loader-runner/blob/6221befd031563e130f59d171e732950ee4402c6/lib/LoaderRunner.js#L108

So not actually in webpack itself. This means that the type is created in part outside of webpacks direct codebase.

BTW I'm not sure why there's some code coverage failures - this change shouldn't affect that and the reported errors are hidden behind a 403 so I'm not too sure what the issue is.

[johnnyreilly]

These are the contents of the LoaderContext when supplied to ts-loader:

version: 2, getOptions: ƒ, emitWarning: ƒ, emitError: ƒ, getLogger: ƒ, …}
_compilation:Compilation {hooks: {…}, name: undefined, startTime: undefined, endTime: undefined, compiler: Compiler, …}
_compiler:Compiler {hooks: {…}, webpack: ƒ, name: undefined, parentCompilation: undefined, root: Compiler, …}
_module:NormalModule {dependencies: Array(0), blocks: Array(0), type: 'javascript/auto', context: '/workspaces/ts-loader/examples/vanilla/src', layer: null, …}
addBuildDependency:dep => {\n\t\t\t\tif (this.buildInfo.buildDependencies === undefined) {\n\t\t\t\t\tthis.buildInfo.buildDependencies = new LazySet();\n\t\t\t\t}\n\t\t\t\tthis.buildInfo.buildDependencies.add(dep);\n\t\t\t}
addContextDependency:ƒ addContextDependency(context) {\n\t\tcontextDependencies.push(context);\n\t}
addDependency:ƒ addDependency(file) {\n\t\tfileDependencies.push(file);\n\t}
addMissingDependency:ƒ addMissingDependency(context) {\n\t\tmissingDependencies.push(context);\n\t}
async:ƒ async() {\n\t\tif(isDone) {\n\t\t\tif(reportedError) return; // ignore\n\t\t\tthrow new Error("async(): The callback was already called.");\n\t\t}\n\t\tisSync = false;\n\t\treturn innerCallback;\n\t}
cacheable:ƒ cacheable(flag) {\n\t\tif(flag === false) {\n\t\t\trequestCacheable = false;\n\t\t}\n\t}
callback:ƒ () {\n\t\tif(isDone) {\n\t\t\tif(reportedError) return; // ignore\n\t\t\tthrow new Error("callback(): The callback was already called.");\n\t\t}\n\t\tisDone = true;\n\t\tisSync = false;\n\t\ttry {\n\t\t\tcallback.apply(null, arguments);\n\t\t} catch(e) {\n\t\t\tisError = true;\n\t\t\tthrow e;\n\t\t}\n\t}
clearDependencies:ƒ clearDependencies() {\n\t\tfileDependencies.length = 0;\n\t\tcontextDependencies.length = 0;\n\t\tmissingDependencies.length = 0;\n\t\trequestCacheable = true;\n\t}
context:'/workspaces/ts-loader/examples/vanilla/src'
currentRequest (get):ƒ () {\n\t\t\treturn loaderContext.loaders.slice(loaderContext.loaderIndex).map(function(o) {\n\t\t\t\treturn o.request;\n\t\t\t}).concat(loaderContext.resource || "").join("!");\n\t\t}
data (get):ƒ () {\n\t\t\treturn loaderContext.loaders[loaderContext.loaderIndex].data;\n\t\t}
dependency:ƒ addDependency(file) {\n\t\tfileDependencies.push(file);\n\t}
emitError:error => {\n\t\t\t\tif (!(error instanceof Error)) {\n\t\t\t\t\terror = new NonErrorEmittedError(error);\n\t\t\t\t}\n\t\t\t\tthis.addError(\n\t\t\t\t\tnew ModuleError(error, {\n\t\t\t\t\t\tfrom: getCurrentLoaderName()\n\t\t\t\t\t})\n\t\t\t\t);\n\t\t\t}
emitFile:(name, content, sourceMap, assetInfo) => {\n\t\t\t\tif (!this.buildInfo.assets) {\n\t\t\t\t\tthis.buildInfo.assets = Object.create(null);\n\t\t\t\t\tthis.buildInfo.assetsInfo = new Map();\n\t\t\t\t}\n\t\t\t\tthis.buildInfo.assets[name] = this.createSourceForAsset(\n\t\t\t\t\toptions.context,\n\t\t\t\t\tname,\n\t\t\t\t\tcontent,\n\t\t\t\t\tsourceMap,\n\t\t\t\t\tcompilation.compiler.root\n\t\t\t\t);\n\t\t\t\tthis.buildInfo.assetsInfo.set(name, assetInfo);\n\t\t\t}
emitWarning:warning => {\n\t\t\t\tif (!(warning instanceof Error)) {\n\t\t\t\t\twarning = new NonErrorEmittedError(warning);\n\t\t\t\t}\n\t\t\t\tthis.addWarning(\n\t\t\t\t\tnew ModuleWarning(warning, {\n\t\t\t\t\t\tfrom: getCurrentLoaderName()\n\t\t\t\t\t})\n\t\t\t\t);\n\t\t\t}
fs:CachedInputFileSystem {fileSystem: {…}, _lstatBackend: CacheBackend, lstat: ƒ, lstatSync: ƒ, _statBackend: CacheBackend, …}
getContextDependencies:ƒ getContextDependencies() {\n\t\treturn contextDependencies.slice();\n\t}
getDependencies:ƒ getDependencies() {\n\t\treturn fileDependencies.slice();\n\t}
getLogger:name => {\n\t\t\t\tconst currentLoader = this.getCurrentLoader(loaderContext);\n\t\t\t\treturn compilation.getLogger(() =>\n\t\t\t\t\t[currentLoader && currentLoader.loader, name, this.identifier()]\n\t\t\t\t\t\t.filter(Boolean)\n\t\t\t\t\t\t.join("|")\n\t\t\t\t);\n\t\t\t}
getMissingDependencies:ƒ getMissingDependencies() {\n\t\treturn missingDependencies.slice();\n\t}
getOptions:schema => {\n\t\t\t\tconst loader = this.getCurrentLoader(loaderContext);\n\n\t\t\t\tlet { options } = loader;\n\n\t\t\t\tif (typeof options === "string") {\n\t\t\t\t\tif (options.substr(0, 1) === "{" && options.substr(-1) === "}") {\n\t\t\t\t\t\ttry {\n\t\t\t\t\t\t\toptions = parseJson(options);\n\t\t\t\t\t\t} catch (e) {\n\t\t\t\t\t\t\tthrow new Error(`Cannot parse string options: ${e.message}`);\n\t\t\t\t\t\t}\n\t\t\t\t\t} else {\n\t\t\t\t\t\toptions = querystring.parse(options, "&", "=", {\n\t\t\t\t\t\t\tmaxKeys: 0\n\t\t\t\t\t\t});\n\t\t\t\t\t}\n\t\t\t\t}\n\n\t\t\t\tif (options === null || options === undefined) {\n\t\t\t\t\toptions = {};\n\t\t\t\t}\n\n\t\t\t\tif (schema) {\n\t\t\t\t\tlet name = "Loader";\n\t\t\t\t\tlet baseDataPath = "options";\n\t\t\t\t\tlet match;\n\t\t\t\t\tif (schema.title && (match = /^(.+) (.+)$/.exec(schema.title))) {\n\t\t\t\t\t\t[, name, baseDataPath] = match;\n\t\t\t\t\t}\n\t\t\t\t\tvalidate(schema, options, {\n\t\t\t\t\t\tname,\n\t\t\t\t\t\tbaseDataPath\n\t\t\t\t\t});\n\t\t\t\...
getResolve:getResolve(options) {\n\t\t\t\tconst child = options ? resolver.withOptions(options) : resolver;\n\t\t\t\treturn (context, request, callback) => {\n\t\t\t\t\tif (callback) {\n\t\t\t\t\t\tchild.resolve({}, context, request, getResolveContext(), callback);\n\t\t\t\t\t} else {\n\t\t\t\t\t\treturn new Promise((resolve, reject) => {\n\t\t\t\t\t\t\tchild.resolve(\n\t\t\t\t\t\t\t\t{},\n\t\t\t\t\t\t\t\tcontext,\n\t\t\t\t\t\t\t\trequest,\n\t\t\t\t\t\t\t\tgetResolveContext(),\n\t\t\t\t\t\t\t\t(err, result) => {\n\t\t\t\t\t\t\t\t\tif (err) reject(err);\n\t\t\t\t\t\t\t\t\telse resolve(result);\n\t\t\t\t\t\t\t\t}\n\t\t\t\t\t\t\t);\n\t\t\t\t\t\t});\n\t\t\t\t\t}\n\t\t\t\t};\n\t\t\t}
loaderIndex:0
loaders:(1) [{…}]
loadModule:(request, callback) => {…}
mode:'development'
previousRequest (get):ƒ () {\n\t\t\treturn loaderContext.loaders.slice(0, loaderContext.loaderIndex).map(function(o) {\n\t\t\t\treturn o.request;\n\t\t\t}).join("!");\n\t\t}
query (get):ƒ () {\n\t\t\tvar entry = loaderContext.loaders[loaderContext.loaderIndex];\n\t\t\treturn entry.options && typeof entry.options === "object" ? entry.options : entry.query;\n\t\t}
remainingRequest (get):ƒ () {\n\t\t\tif(loaderContext.loaderIndex >= loaderContext.loaders.length - 1 && !loaderContext.resource)\n\t\t\t\treturn "";\n\t\t\treturn loaderContext.loaders.slice(loaderContext.loaderIndex + 1).map(function(o) {\n\t\t\t\treturn o.request;\n\t\t\t}).concat(loaderContext.resource || "").join("!");\n\t\t}
request (get):ƒ () {\n\t\t\treturn loaderContext.loaders.map(function(o) {\n\t\t\t\treturn o.request;\n\t\t\t}).concat(loaderContext.resource || "").join("!");\n\t\t}
resolve:ƒ resolve(context, request, callback) {\n\t\t\t\tresolver.resolve({}, context, request, getResolveContext(), callback);\n\t\t\t}
resource (get):ƒ () {\n\t\t\tif(loaderContext.resourcePath === undefined)\n\t\t\t\treturn undefined;\n\t\t\treturn loaderContext.resourcePath.replace(/#/g, "\\0#") + loaderContext.resourceQuery.replace(/#/g, "\\0#") + loaderContext.resourceFragment;\n\t\t}
resource (set):ƒ (value) {\n\t\t\tvar splittedResource = value && parsePathQueryFragment(value);\n\t\t\tloaderContext.resourcePath = splittedResource ? splittedResource.path : undefined;\n\t\t\tloaderContext.resourceQuery = splittedResource ? splittedResource.query : undefined;\n\t\t\tloaderContext.resourceFragment = splittedResource ? splittedResource.fragment : undefined;\n\t\t}

a complete type should encompass this.

[johnnyreilly]

Okay, so it turns out that LoaderContext is constructed in two places:

• in webpacks NormalModule:

  webpack/lib/NormalModule.js

  Line 426 in 49890b7

  createLoaderContext(resolver, options, compilation, fs) {

• in loader-runners LoaderRunner: https://github.com/webpack/loader-runner/blob/6221befd031563e130f59d171e732950ee4402c6/lib/LoaderRunner.js#L289

Not quite sure how to tackle combining together a type from two packages; or if it makes sense... Will ponder.

For now I've created a more up to date type in ts-loader itself: TypeStrong/ts-loader@acbc71f

Incididentally, this PR does improve the internal type definitions of webpack - just it doesn't help outside consumers such as ts-loader a great deal due to the dual constructions sites of LoaderContext

[webpack-bot]

Hi @johnnyreilly.

Just a little hint from a friendly bot about the best practice when submitting pull requests:

Don't submit pull request from your own master branch. It's recommended to create a feature branch for the PR.

You don't have to change it for this PR, just make sure to follow this hint the next time you submit a PR.

[sokra]

To export the type from webpack add it to the typedefs in lib/index.js.

Also export a LoaderDefinition<ContextAdditions = EmptyContextAdditions> type which can be added to loaders like that:

/** @type {import("webpack").LoaderDefinition} */
module.exports = function(source) {
  // ...
}

and do that for loaders in test/... to test that.

See tsconfig.test.json (yarn typings-lint) and add *.loader.js and rename all loaders to this format.

This should ensure that the loader context typings are (at least a little bit) tested.

[johnnyreilly]

Thanks for these points @sokra - I'll take a look.

[webpack-bot]

@johnnyreilly Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

[johnnyreilly]

Hey @sokra,

I've had a go at implementing the suggestions you made. However, attaching the /** @type {import("webpack").LoaderDefinition} */ to loaders doesn't seem to be working...

Also export a LoaderDefinition<ContextAdditions = EmptyContextAdditions> type which can be added to loaders like that:

/** @type {import("webpack").LoaderDefinition} */
module.exports = function(source) {
// ...
}
and do that for loaders in test/... to test that.

Am I doing something incorrect here? (I could well be using JSDoc wrong)

I suspect that it's probably something to do with how the types are generated by yarn special-lint-fix; the types.d.ts ends up generating this type:

	export type LoaderDefinition = LoaderDefinition<EmptyContextAdditions>;

Which results in a linting issue:

tsc -p tsconfig.test.json
Error: types.d.ts(12010,14): error TS2456: Type alias 'LoaderDefinition' circularly references itself.
Error: types.d.ts(12010,33): error TS2315: Type 'LoaderDefinition' is not generic.

[sokra]

I've had a go at implementing the suggestions you made. However, attaching the /** @type {import("webpack").LoaderDefinition} */ to loaders doesn't seem to be working...

Also export a LoaderDefinition<ContextAdditions = EmptyContextAdditions> type which can be added to loaders like that:
/** @type {import("webpack").LoaderDefinition} */
module.exports = function(source) {
// ...
}
and do that for loaders in test/... to test that.

Am I doing something incorrect here? (I could well be using JSDoc wrong)

Ah sorry. Within webpack it's more like /** @type {import("../../../../").LoaderDefinition} */.

[sokra]

If the generic LoaderDefinition doesn't work, you may omit that for now and we can add this in a future PR.

[sokra]

Hopefully these changes will lead to export { LoaderDefintion } being generated...

But maybe there is also a bug in the type generation. We didn't export generic types yet...

[sokra]

You could steal the Schema type from schema-utils:

Parameters<typeof import("schema-utils").validate>[0]

[johnnyreilly]

Will steal!

[sokra]

Suggested change

	emitWarning(warning: Error | string): void;
	emitError(error: Error | string): void;
	emitWarning(warning: Error): void;
	emitError(error: Error): void;

[johnnyreilly]

Looking at usage it seems these can be strings:

webpack/lib/NormalModule.js

Line 521 in 85fe6ac

if (!(error instanceof Error)) {

			emitError: error => {
				if (!(error instanceof Error)) {
					error = new NonErrorEmittedError(error);
				}
				this.addError(
					new ModuleError(error, {
						from: getCurrentLoaderName()
					})
				);
			},

[sokra]

Yes, but that's only for backward-compat. The NonErrorEmittedError says that you made an error in error reporting... so it should not be in the typings

[sokra]

same arguments as for async() return value

[sokra]

I'll finish that... The tooling need to be improved a bit for that...

[webpack-bot]

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

[alexander-akait]

@sokra Should we add @types/soure-map to our dependencies?

[alexander-akait]

Maybe we can export ValidationError to avoid using package directly, I am afraid it can be changed in future

[sokra]

I think we can get the Schema from the root types too: Parameters<import("schema-utils").validate>[1]

[sokra]

Ok, I improved the tooling a bit so we can export generic types now.
Types in declarations/index.d.ts are now exposed too (in addition to lib/index.js), since it's not possible to expose generic types with defaults for the type parameters from jsdocs 😢

All (hopefully all) loaders in the test suite are now tested against the new declarations and all have type annotations.
I needed to fix a few type problems there too...

[sokra]

🎉

[sokra]

@alexander-akait This doesn't seem to be exported from the root package. It would help if you could export it from there.

[sokra]

Thanks

[johnnyreilly]

This is tremendous! Thanks @sokra!

[orta]

v.cool