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

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

Open
walisc opened this issue Aug 13, 2022 · 7 comments
Open

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

walisc opened this issue Aug 13, 2022 · 7 comments

Comments

@walisc
Copy link

walisc commented Aug 13, 2022

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 webpack/enhanced-resolve#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

} catch (e) {}
)

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 walisc changed the title Ts-loader never uses the enhanced-resolver as of enhanced-resolve v5.3.1 Ts-loader never uses enhanced-resolve as of enhanced-resolve v5.3.1 Aug 13, 2022
@walisc
Copy link
Author

walisc commented Aug 13, 2022

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.

@johnnyreilly
Copy link
Member

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?

@walisc
Copy link
Author

walisc commented Aug 15, 2022

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?

@johnnyreilly
Copy link
Member

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

@walisc
Copy link
Author

walisc commented Aug 15, 2022

Okay, great. Will do! Enjoy the break 👌

@walisc
Copy link
Author

walisc commented Sep 14, 2022

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.

@johnnyreilly
Copy link
Member

No worries!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants