ts-loader icon indicating copy to clipboard operation
ts-loader copied to clipboard

Ts-loader never uses enhanced-resolve as of enhanced-resolve v5.3.1

Open walisc opened this issue 2 years ago • 7 comments

When resolving modules attempts are first made to use enhanced-resolver (which is also used by webpack), and if that fails we fallback to the typescript resolver (src/servicesHost.ts:resolveModule ). In doing this undefined is passed as the context to the enhanced-resolver's resolveSync method.

Prior to version 5.3.1 the context value was not validated, as opposed to v5.3.1 onward (see https://github.com/webpack/enhanced-resolve/pull/261), of which the first check is to see if it's an object or not. This results in a error being throw, of which this is handled silently (see https://github.com/TypeStrong/ts-loader/blob/b4b036325b4cd97fab8b3f01fc7bc849373839d4/src/servicesHost.ts#L1270)

This means enhanced-resolve is never used, of which this is hard to pick up as we handle all error silently and fallback to the typescript resolver.

Expected Behaviour

To use enhanced-resolve when resolving modules.

Actual Behaviour

The enhanced resolver is never used when resolving modules, and falls back to the typescript resolver.

Steps to Reproduce the Problem

Try to use a custom fileSystem, by specifying this in your webpack's resolve options. Rather than this option being passed on to the enhanced-resolver , it is neglected, meaning that ts-loader falls back to the default/standard typescript filesystem. This is highlighted by the snippet code below (or in the example project below)

webpack.config.js

const fs = require("fs")
const path = require("path")
const memoryFS = require('memory-fs');
const unionfs = require("unionfs").ufs

var mfs = new memoryFS();
mfs.mkdirpSync(__dirname)
mfs.writeFileSync(path.join(__dirname,'fileSystemType.ts'), `export default () => {
  return "Using memory filesystem"
}`);

const updatedFs = unionfs.use(fs).use(mfs)

module.exports = {
  entry: "./index.ts",
  output: {
    filename: 'main.js'
  },
  target: "node",
  resolve: {
    extensions: [".ts", ".tsx", ".js", ".jsx"],
    fileSystem: updatedFs
  },
  module: {
    rules: [
      { test: /\.tsx?$/, loader: "ts-loader" }
    ]
  },
  plugins: [
    new class {
     apply(compiler){
       compiler.hooks.beforeRun.tap("NodeEnvironmentPlugin", compiler => {
         compiler.inputFileSystem = updatedFs
       });
     }
    }
 ]
};


index.ts

import filesystemType from "./fileSystemType";
console.log(filesystemType())

Expected Output (running npx webpack and node dist/main.js) "Using memory fileSystem"

Actual Output (running npx webpack) TS2307: Cannot find module './fileSystemType' or its corresponding type declarations.

Location of a Minimal Repository that Demonstrates the Issue.

https://github.com/walisc/ts-loader-enhance-resolver-bug

Proposed Fix

Passing an empty object as opposed to undefined, to resolveSync. In addition, it would be good to distinguish between configuration errors and actual resolve errors. In the snippet below I do this through a resolver plugin. Do note, this is just a poc, and maybe more work /cleaning up might be required.

diff -ur ts-loader-enhance-resolver-bug/node_modules/ts-loader/dist/resolver.js ts-loader/node_modules/ts-loader/dist/resolver.js
--- ts-loader-enhance-resolver-bug/node_modules/ts-loader/dist/resolver.js	2022-08-13 09:21:51.220999000 +0200
+++ ts-loader/node_modules/ts-loader/dist/resolver.js	2022-08-14 00:59:11.408463176 +0200
@@ -2,8 +2,27 @@
 Object.defineProperty(exports, "__esModule", { value: true });
 exports.makeResolver = void 0;
 const enhanced_resolve_1 = require("enhanced-resolve");
+
+ class ResolveStatusPlugin {
+    setResolveSync(resolveSync){
+        this.resolveSync = resolveSync
+    }
+    apply(resolver){
+        resolver
+            .getHook("noResolve")
+            .tap("resolveStatusPlugin", (obj, error) => {               
+                this.resolveSync["resolveStatus"] = "failed"
+            });
+    }
+}
 function makeResolver(options) {
-    return enhanced_resolve_1.create.sync(options.resolve);
+    const resolveStatusPlugin = new ResolveStatusPlugin()
+    options.resolve["plugins"] = "plugins" in options.resolve ? [...plugins, resolveStatusPlugin] : [resolveStatusPlugin]
+
+    const resolveSync = enhanced_resolve_1.create.sync(options.resolve);
+    resolveStatusPlugin.setResolveSync(resolveSync)
+    return resolveSync
+
 }
 exports.makeResolver = makeResolver;
 //# sourceMappingURL=resolver.js.map
\ No newline at end of file
diff -ur ts-loader-enhance-resolver-bug/node_modules/ts-loader/dist/servicesHost.js ts-loader/node_modules/ts-loader/dist/servicesHost.js
--- ts-loader-enhance-resolver-bug/node_modules/ts-loader/dist/servicesHost.js	2022-08-13 09:21:51.220999000 +0200
+++ ts-loader/node_modules/ts-loader/dist/servicesHost.js	2022-08-14 00:59:51.609128561 +0200
@@ -732,7 +732,7 @@
 function resolveModule(resolveSync, resolveModuleName, appendTsTsxSuffixesIfRequired, scriptRegex, moduleName, containingFile, redirectedReference) {
     let resolutionResult;
     try {
-        const originalFileName = resolveSync(undefined, path.normalize(path.dirname(containingFile)), moduleName);
+        const originalFileName = resolveSync({}, path.normalize(path.dirname(containingFile)), moduleName);
         if (originalFileName) {
             const resolvedFileName = appendTsTsxSuffixesIfRequired(originalFileName);
             if (resolvedFileName.match(scriptRegex) !== null) {
@@ -740,7 +740,11 @@
             }
         }
     }
-    catch (e) { }
+    catch (e) { 
+        if (resolveSync.resolveStatus != "failed"){
+            throw e
+        }
+    }
     const tsResolution = resolveModuleName(moduleName, containingFile, redirectedReference);
     if (tsResolution.resolvedModule !== undefined) {
         const resolvedFileName = path.normalize(tsResolution.resolvedModule.resolvedFileName);

walisc avatar Aug 13 '22 23:08 walisc

Hi @johnnyreilly . Thanks for this awesome project! Let me know if you would want me to look into this/create a pull request for this.

walisc avatar Aug 13 '22 23:08 walisc

Yes please! I'm going to be on holiday here and there over the next couple of weeks and so might not be too responsive. Do you fancy raising a prospective PR and we can see where we go with it?

johnnyreilly avatar Aug 14 '22 08:08 johnnyreilly

Hi @johnnyreilly . Okay will do! Might only be able to get to it at the end of the week/early next week though. Hope that works?

walisc avatar Aug 15 '22 03:08 walisc

Whenever you like ! - I'm only going to be online sporadically until September anyway

johnnyreilly avatar Aug 15 '22 05:08 johnnyreilly

Okay, great. Will do! Enjoy the break 👌

walisc avatar Aug 15 '22 07:08 walisc

Hi @johnnyreilly . Just to update you. Things have been a bit manic my side, and I have not had time to properly look at this. Things should ease up around month end/early October though. Should be able to work on it then.

walisc avatar Sep 14 '22 13:09 walisc

No worries!

johnnyreilly avatar Sep 14 '22 13:09 johnnyreilly