baseUrl has a side effect on the used dependency export #133

katywings · 2020-07-22T18:28:35Z

Is

If I use ts-node together with tsconfig-paths/register and a tsconfig baseUrl of ./node_modules the runtime seems to "ignore" package.json exports of dependencies and uses the wrong file. As the bottom example shows it uses colorette/index.js but it should use colorette/index.cjs.

Should

It should use the correct dependency dist as defined by its package.json exports, so for colorette it should use colorette/index.cjs.

Reproduction

tsconfig-paths-exports-repro-master.zip
npm install && npm start

Trace

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: xyz/node_modules/colorette/index.js
require() of ES modules is not supported.
require() of xyz/node_modules/colorette/index.js from xyz/node_modules/autoprefixer/lib/autoprefixer.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from xyz/node_modules/colorette/package.json.

    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1217:13)
    at Module.load (internal/modules/cjs/loader.js:1050:32)
    at Function.Module._load (internal/modules/cjs/loader.js:938:14)
    at Module.require (internal/modules/cjs/loader.js:1090:19)
    at require (internal/modules/cjs/helpers.js:75:18)
    at Object.<anonymous> (xyz/node_modules/autoprefixer/lib/autoprefixer.js:5:17)
    at Module._compile (internal/modules/cjs/loader.js:1201:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1221:10)
    at Module.load (internal/modules/cjs/loader.js:1050:32)
    at Function.Module._load (internal/modules/cjs/loader.js:938:14)

Background:

At first I thought this is related to ts-node fails when ES Modules are in the dependency graph in Node.js 13+ TypeStrong/ts-node#935 / ESM support: soliciting feedback TypeStrong/ts-node#1007 but after working out a reproduction case I figured out that the error only happens with tsconfig-paths and the mentioned baseUrl value 🙈.
The reason why I used a baseUrl of "./node_modules" in the first place was because my root folder has some folders with the same name as some dependencies, a baseUrl of "." resulted in typescript importing my /xyz instead of /node_modules/xyz ;).

The text was updated successfully, but these errors were encountered:

cspotcode · 2020-07-22T18:47:42Z

Sharing in case it's helpful:

tsconfig-paths hooks into node's resolver here: https://github.com/dividab/tsconfig-paths/blob/master/src/register.ts#L73-L96

And it checks for package.json "main" field here: https://github.com/dividab/tsconfig-paths/blob/master/src/match-path-sync.ts#L85-L106

...but doesn't check for the new "exports" nor "module" fields.

I assume it needs to be updated to understand node's new ESM/CJS resolver behaviors.

EDIT: I may be off-track with these observations, though, since I see that @katywings narrowed this down to a specific baseUrl setting.

jonaskello · 2020-07-23T09:01:23Z

I think we want to keep tsconfig-paths logic as close as possible to how tsc resolves modules. Some thoughts on this:

The exports field of package.json is not supported by tsc yet but it will be eventually as it is part of the new standard for modules in node. The reason typescript does not support it yet seems to be they don't really know exactly how to implement it as there is still discussion on some edge cases of the standard. See this comment. I think tsconfig-paths should support this but maybe wait until we see how tsc will implement it.
From what I recall the module field in package.json is not part of the new node standard. The way I remember it is that it was something that was invented by webpack, rollup etc. to support bundling of ESM. Unless it is really part of the new standard I don't think tsconfig-paths should support it. I don't think tsc supports this field or ever will but I might be wrong?
The use of new extensions like .cjs is part of the new standard. I'm not sure if tsc is supporting this yet? If it is supported in tsc then I think we should implement it.

I ran the repro above and it seems this issue is really stuck on (3) above, not (1) or (2)?

jonaskello · 2020-07-23T09:13:53Z

I'm not really sure of the use case for the repro. Seems to do some bundling, but why use tsconfig-paths with it? Removing the baseUrl from tsconfig.json and the use of tsconfig-paths from npm start makes it run and produce output in the dist folder. I'm not sure if it is the correct output though :-).

jonaskello · 2020-07-23T09:26:03Z

Regarding resolving cjs/mjs there is an interesting comment from the typescript team here. Seems like there is going to be multiple resolution modes in the future. Not sure how this will affect tsconfig-paths.

katywings · 2020-07-23T10:48:36Z

@jonaskello

I ran the repro above and it seems this issue is really stuck on (3) above, not (1) or (2)?

All of those options have one question in common: why does it only fail with the baseUrl "./node_modules" but works without issues with baseUrl: "."?

I'm not really sure of the use case for the repro...

I am not sure if this question is a troll, but the use case of an src for issue reproduction is to give you a way to reproduce the problem 🙈. microbundle was literally the first project that came to my mind that is having this issue in combination with tsconfig-paths and the node_modules baseUrl... If you are asking because you try to find a workaround for me: I already know a workaround: I can get rid of baseUrl "./node_modules" in my projects with some slight changes ;).

jonaskello · 2020-07-23T11:14:49Z

@katywings

All of those options have one question in common: why does it only fail with the baseUrl "./node_modules" but works without issues with baseUrl: "."?

Yes that is a good question. I guess we would need to debug through it to see the difference?

I am not sure if this question is a troll, but the use case of an src for issue reproduction is to give you a way to reproduce the problem

No trolling, sorry if it came across that way :-) I was not referring to reproductions in general but what the code actually does in this particular repro. From what I understand it does bundling? Why would you want to involve tsconfig-paths when bundling? The bundler should be able to resolve everything by itself better than tsconfig-paths can? For example most bundlers take into account the module field in package.json which tsconfig-paths does not. In my mind the normal use-case for tsconfig-paths is running application code or tests so I just thought using it for bundling was kind of strange. Perhaps your real-world case is something else?

katywings · 2020-07-23T16:26:09Z

@jonaskello Thanks for the detailled answer :)

From what I understand it does bundling? Why would you want to involve tsconfig-paths when bundling?

I use ts-node / tsconfig-paths during development for the express http server with ssr. Also during dev microbundle is ran by the node server to bundle client assets - the microbundle result/errors are outputted in the frontend by the express server, all of this happens in one node process.

jonaskello · 2020-07-23T18:17:30Z

All of those options have one question in common: why does it only fail with the baseUrl "./node_modules" but works without issues with baseUrl: "."?

I thought about it a bit more and I think I know why there is a difference, although I have not debugged it so I'm not sure.

When you do ts-node -r tsconfig-paths/register the Module. _resolveFilename function is patched so all calls to it will use the tsconfig-paths logic regardless where the call comes from as you can see here.

If you use baseUrl: "." then tsconfig-paths will not find something installed in node_modules that is required by name, for example require("foo"). When tsconfig-paths does not find a path it will delegate to the default implementation of Module. _resolveFilename as you can see here. If you are running a version of node that supports the new resolution with .cjs, .mjs etc. it will succeed in this case.

If you use baseUrl "./node_modules" then require("foo") will look for nodes_modules/foo and it will be found and tsconfig-paths will try to resolve it with the logic that typescript currently uses, that is without support for .cjs, .mjs etc.

I think you have a mix of requirements in your setup. Currently typescript (and tsc) does not support .cjs extensions so by extension neither does ts-node or tsconfig-paths. So for the part that uses ts-node to execute server-side code you need to use typescript's current approach to node resolution because it is the only option supported. However when you do bundling of client assets, your bundler seems to require the new style node resolution (using .cjs etc). If you run all of this in a single node process and patch the whole process to use the current typescript resolution approach (through using -r tsconfig-paths/register) it is not going to work out since your bundler will not work with that approach.

Perahaps you can reconfigure the bundler to not use ESM style bundling? Is there any specific reason you need ESM style bundling in development mode? Another option might be to manually bootstrap tsconfig-paths so it does only affect the server side execution and not the bundling. Yet another approach would be to use two separate node processes, one for the server and one for the client assets bundling (this is the way I usually handle it starting them in parallel with npm-run-all).

katywings · 2020-07-23T18:42:59Z

If you use baseUrl: "." then tsconfig-paths will not find something installed in node_modules that is required by name, for example require("foo"). When tsconfig-paths does not find a path it will delegate to the default implementation of Module. _resolveFilename as you can see here. If you are running a version of node that supports the new resolution with .cjs, .mjs etc. it will succeed in this case.

This would makes a Iot of sense 😍

Perahaps you can reconfigure the bundler to not use ESM style bundling?

Nonono 🙈, the bundler has nothing to do with the source of problem. In the reproduction you literally could replace the microbundle stuff with a require of autoprefixer and you would get the same error. Changing the output format of microbundle doesnt help, because neither does it change the fact that microbundle will require autoprefixer->colorette, nor do I even require any esm output of microbundle in a node process... I know, well I hope, you just try to help with a workaround, thank you for that, but I think we better should just focus on the debugging/validation/follow up of your amazing thought at the top :).

katywings · 2020-07-23T19:00:57Z

@cspotcode Did you already start with the exports/esm stuff in some hidden place? How do we coordinate us? Who makes what? - do you even need my help? 😂

cspotcode · 2020-07-23T19:29:23Z

ts-node added experimental support for node's (also experimental) native ESM support.

It's already been published, and feedback is being tracked here. TypeStrong/ts-node#1007

However, ts-node doesn't need to hook into node's CommonJS resolver, so we don't touch that. Unfortunately, to make our ESM loader hooks work, we need to duplicate the functionality of node's built-in ESM resolver. So we've copy-pasted the source code from node, and tweaked as needed.
https://github.com/TypeStrong/ts-node/blob/master/dist-raw/node-esm-resolve-implementation.js

jonaskello · 2020-07-23T19:54:37Z

@katywings If the problem has nothing to do with the bundling perhaps it is possible to create a simpler repro that does not involve a bundler? For example does it work to directly require colorette without any bundler? (I think I will give that a try as colorette is using .cjs in it's main field which may be causing the problem but should probably work with old style resolution as the extension is explicit).

@cspotcode If ts-node supports the new ESM resolution that would mean that ts-node can compile and run code that tsc cannot currently compile? Or am I missing something here?

jonaskello · 2020-07-23T20:20:23Z

I did an experiment/repro here that does not involve ts-node or any bundling. It just uses tsc and node. It works without tsconfig-paths loaded but not with it.

Looking into colorette I found it has "type": "module" set in package.json. That means that every .js file should be treated as a ESM module. So the index.js file is in ESM format. To be backwards compatible with the old node resolution it also has "main": "index.cjs". Since older versions of node only check the main field they will require index.cjs and it works. This means tsconfig-paths should also require this file but somehow it seems to require index.js instead. I think this is a bug since if there is an explicit file extension for the file in the main field it should be used as-is. (Note that the module field is not used at all by node as far as I know, it is only webapck that checks that field.)

cspotcode · 2020-07-23T20:42:56Z

I'm referring specifically to node's new native ESM mode, where you have to use a special CLI flag to use a custom loader hook which intercepts ESM imports / exports. https://nodejs.org/api/esm.html#esm_hooks

I explained exactly how this works here: nodejs/modules#351 (comment)
Normally I would re-explain here, but I'm a little busy today. I hope this explanation is sufficient in the short-term.

tl;dr is that you must put the .js extension into your import statements. So we need a custom resolver to detect when you're trying to import a .js file but the filesystem actually contains a .ts file. This makes us compatible with what TypeScript already does.

jonaskello · 2020-07-23T20:48:35Z

@cspotcode Yes, I know all of that, no explanation necessary :-). My point was that if ts-node compile and runs stuff by looking at the exports or type field in package.json then it is compiling and running stuff that tsc does not compile today since tsc does not look at those fields at all as far as I know.

EDIT: The reason I mentioned it was because then there is a inconsistency how tsc work vs how ts-node works regarding path resolving. I think we should keep tsconfig-paths as close to tsc as possible. However it will of course not hurt to experiment with new options before tsc supports them.

cspotcode · 2020-07-23T21:09:49Z

Perhaps, I'm not sure. I don't think that's a use-case we need to prioritize. In other words, if you're writing code that works with tsc and with the language service, then ts-node should be able to run it. But users are expected to write their code in a way that tsc supports.

Do you have an example of what you're describing, perhaps a sample package.json plus some source files? I think that will help me better understand the situation you're trying to avoid.

jonaskello · 2020-07-23T21:13:44Z

@cspotcode Yes, an example would be good, I'll try to put one together.

jonaskello · 2020-07-23T21:20:25Z

Having investigated further, the problem in this issue seems to be unrelated to the new node resolution of ESM. Instead it is related to tsconfig-paths not respecting the file extensions of the file found in the main field. The reason the problem appears together with new ESM resolution stuff is because now it is common to have other extensions than .js (for example .cjs).

If we don't strip the extension in this line the repro that I provided above works. As the comment says, I'm not sure why we are stripping the extension there :-).

If anyone wants to take a stab at at PR that adds tests for other extensions than .js and removes the stripping of the extension that would be welcome.

katywings · 2020-07-24T16:28:14Z

@jonaskello I gonna try to create that pr now ;)

katywings · 2020-07-24T16:50:50Z

@jonaskello Btw. in the original commit message for removeExtension you already added a hint "but why?": 847d314, you don't remember why you added this commit in the first place? :D

Edit: it looks like it has something todo with json require handling, still investigating this ;)
Edit 2: forget the json require handling, thought i had to run build:test before running tests, but build:test actually destroys the tests xD

Originally during the file path resolution, file extensions were removed without explicit reason. This commit changes the resolution logic to keep file extensions, with the goal to add support for modern non-js extensions like cjs. The change however keeps /xyz/index resolves as is as they still should resolve to /xyz. Refs 847d314

cspotcode · 2020-07-26T02:27:46Z

@jonaskello I'm reviewing #135 and looking at the behavior of the code immediately prior to when #27 was merged.

It looks like when you require a directory containing a package.json, tsconfig-paths is supposed to return the path to the directory, not to the index file that it finds in the package.json's "main" field. Is that correct?

Specifically I think these 2 places in the code are the bug:

tsconfig-paths/src/match-path-sync.ts

Lines 133 to 136 in 7204659

    
           if (mainFieldMappedFile) { 
        
             // Not sure why we don't just return the full path? Why strip it? 
        
             return Filesystem.removeExtension(mainFieldMappedFile); 
        
           }

It should not return Filesystem.removeExtension(mainFieldMappedFile)
It should return TryPath.getStrippedPath(tryPath)

tsconfig-paths/src/try-path.ts

Lines 71 to 72 in 7204659

: tryPath.type === "package"

? tryPath.path

It should return dirname(tryPath.path) which will be the path to the directory containing package.json.

Combining those 2x modifications, tsconfig-paths will read the package.json, make sure it points to something that can be loaded, and then return the path to the module's directory. node will receive the path to the directory and take care of reading the package.json and loading the correct file.

jonaskello · 2020-07-26T09:27:52Z

@cspotcode Yes, I think you are on to something here. The original intent of the stripping of some stuff like extensions were probably that node should be able to re-resolve with it's own logic.

However I'm not sure that is a good idea anymore. If tsconfig-paths has found the exact file we want to resolve to, why should it then return less information to node, like removing the extension, or the returning the dir instead of the actual file in the case of package.json? This is probably why I left comments in some places questioning the removal of information.

Regarding package.json, there is a case where you can provide other fields to look at instead of (or in addition to) main when using the API. This function has a mainFields parameter.

tsconfig-paths/src/match-path-sync.ts

Lines 64 to 70 in 7204659

    
           export function matchFromAbsolutePaths( 
        
             absolutePathMappings: ReadonlyArray<MappingEntry.MappingEntry>, 
        
             requestedModule: string, 
        
             readJson: Filesystem.ReadJsonSync = Filesystem.readJsonFromDiskSync, 
        
             fileExists: Filesystem.FileExistsSync = Filesystem.fileExistsSync, 
        
             extensions: Array<string> = Object.keys(require.extensions), 
        
             mainFields: string[] = ["main"]

So for example you could add module to the fields to check to emulate what webpack and other bundlers does. If we just return the directory and let node decide which field to check in package.json, I think we would break this functionality?

jonaskello · 2020-07-26T09:36:13Z

@cspotcode Perhaps we could go in the direction of letting tsconfig-paths always decide the exact file and just pass that to node instead of having node re-resolve by removing info? That would probably increase performance by a small bit too since node has less work to do. Do you think there will be issues going in this direction?

Specifically I think we can remove this function:

tsconfig-paths/src/try-path.ts

Lines 63 to 74 in 7204659

    
           // Not sure why we don't just return the full found path? 
        
           export function getStrippedPath(tryPath: TryPath): string { 
        
             return tryPath.type === "index" 
        
               ? dirname(tryPath.path) 
        
               : tryPath.type === "file" 
        
                 ? tryPath.path 
        
                 : tryPath.type === "extension" 
        
                   ? removeExtension(tryPath.path) 
        
                   : tryPath.type === "package" 
        
                     ? tryPath.path 
        
                     : exhaustiveTypeException(tryPath.type); 
        
           }

For the "index" part, this means tsconfig-paths has found an index file that should be resolved, but it then removes the filename so node can re-resolve from the path only and re-find the index file that tsconfig-paths already found.

For the "file" part, it already returns the exact path found.

For the "extension" part, it only returns the filename so node needs to re-find the correct extension.

For the "package" part it already returns the exact path found (however in previous code it strips the extension of the found file causing the bug in this issue).

If we remove this function, tsconfig-paths will be 100% responsible for finding the exact file to use instead of relying on node to re-evalutate. I think removing this function will make things more predictable. The downside may be that any updates to node's algorithm will not be automatically used?

jonaskello · 2020-07-26T09:56:45Z

Thinking more on the extension stripping. When using tsconfig-paths together with ts-node ts-node will add .ts to require.extensions. This means that tsconfig-paths will resolve to .ts files. This may be the reason that tsconfig-paths do extension stripping? Or do ts-node patch Module._resolveFilename so it is safe to pass a .ts file to it?

katywings · 2020-07-26T10:28:34Z

@jonaskello

Regarding package.json, there is a case where you can provide other fields to look at instead of (or in addition to) main when using the API. This function has a mainFields parameter.

If this case exists, shouldnt it be added in the tests then? Since you are the one who knows about this case, it probably makes sense if you can add a test for it, then I can make sure in my pr that your new test still works with my changes ^^.

Originally during the file path resolution, file extensions were removed without explicit reason. This commit changes the resolution logic to keep file extensions, with the goal to add support for modern non-js extensions like cjs. The change however keeps /xyz/index resolves as is as they still should resolve to /xyz. Refs 847d314

jonaskello · 2020-07-26T11:13:24Z

@katywings It is already covered by this test:

tsconfig-paths/test/data/match-path-data.ts

Lines 128 to 142 in 2461cc9

    
             name: "should resolve from list of fields by priority in package.json", 
        
             absoluteBaseUrl: "/root/", 
        
             paths: { "lib/*": ["location/*"] }, 
        
             mainFields: ["missing", "browser", "main"], 
        
             packageJson: { main: "./main.js", browser: "./browser.js" }, 
        
             existingFiles: [ 
        
               join("/root", "location", "mylibjs", "main.js"), // mainFilePath 
        
               join("/root", "location", "mylibjs", "browser.js"), // browserFilePath 
        
             ], 
        
             extensions: [".ts", ".js"], 
        
             requestedModule: "lib/mylibjs", 
        
             expectedPath: removeExtension( 
        
               join("/root", "location", "mylibjs", "browser.js") 
        
             ), 
        
           },

cspotcode · 2020-07-26T17:16:51Z

@jonaskello

If tsconfig-paths has found the exact file we want to resolve to, why should it then return less information to node, like removing the extension, or the returning the dir instead of the actual file in the case of package.json?

To allow node to resolve the package's entry-point according to its configuration, which tsconfig-paths may not know about. In other words, I believe tsconfig-paths may not have found the exact file; it may have found the wrong one. It's better for tsconfig-paths to avoid resolving to a file, and let node do that work.

Regarding package.json, there is a case where you can provide other fields to look at instead of (or in addition to) main when using the API. This function has a mainFields parameter.

If we return the path to the module's root directory, then tsconfig-paths doesn't need to know about those additional fields because node will resolve from there. For example, node will consult the package.json "exports" field and do the right thing.

Thinking more on the extension stripping. When using tsconfig-paths together with ts-node ts-node will add .ts to require.extensions. This means that tsconfig-paths will resolve to .ts files. This may be the reason that tsconfig-paths do extension stripping? Or do ts-node patch Module._resolveFilename so it is safe to pass a .ts file to it?

ts-node does not patch Module._resolveFilename. We use node's default resolver which respects Object.keys(require.extensions) and resolves to .ts files when they're found.

@katywings sorry for the back-and-forth. Agreed that there's no way for an external contributor to know how this library is supposed to behave, so it's far too difficult to write a bugfix.

jonaskello · 2020-07-26T19:50:56Z

If we return the path to the module's root directory, then tsconfig-paths doesn't need to know about those additional fields because node will resolve from there.

Yes of course :-) However my point was that currently the API allows you to specify additional fields and if we remove that feature we are making a breaking change. See #45 for some more background.

jonaskello · 2020-07-26T20:02:31Z

I believe tsconfig-paths may not have found the exact file; it may have found the wrong one.

Interesting, could you provide an example when it would find the wrong file? I'm thinking only old style resolution here, not the new style for ESM.

My thinking is that the unclear rules for resolution are mainly caused by not having tsconfig-paths deterministically find a file. If we resolve to "something" and then let node resolve to the real thing I think it will be very hard to have clear rules on how the resolution should work. The fact that the stripping of paths and extensions is the main pain point right now seems to point in this direction. To be clear, the stripping is done because we wanted node to do part of the resolution. I can see how letting node do the work should in theory be more compatible, however that is actually the way it is already done, hence the stripping and nondeterministic stuff going on that causes this very issue. If we should let node handle some part of the resolution then we need to make some other rule-set than the actual resolution very clear. Some kind of rule-set for pre-resolution that works in tandem with node's internal resolution.

I guess we also need decide on the goals. I think my goals are more on the side of having the old style very clear and reliable, while you may be thinking about adding the new style resolution? I could work either way but it helps to have a common goal :-).

cspotcode · 2020-07-27T23:44:13Z

Agreed about common goals. My goal is to make it work seamlessly with ts-node, perhaps even add a ts-node flag to register it automatically. But I also understand the need to support other consumers like webpack.

I suppose I'm thinking of the new style of resolution. What happens if "paths" redirects an import/require to a directory that has package.json with "types" and "exports"? I assume TS is going to resolve to the "types" but node must obey "exports". Is that a realistic example?

Could we add an opt-in flag, something like resolveToPackageDirs, and then use that flag in tsconfig-paths/register? It would resolve to the package's root directory, allowing node to resolve from there, using "main" or "exports" with whatever extensions it has configured. Or would that be adding unnecessary complexity?

cspotcode · 2020-07-28T00:27:22Z

Supposing I have this paths configuration:

"baseUrl": "/baseUrl",
"paths": {
  "/*": ["/foo/*", "/bar/*"]
}

To resolve import from "/baz"; I know I need to check 2 places: /baseUrl/foo/baz and /baseUrl/bar/baz. Is is ok to do this in node?

for(const candidate in ['/baseUrl/foo/baz', '/baseUrl/bar/baz']) {
  try {
    return originalResolveFilename(candidate, parent);
  } catch(e) { /* handle errors so that last error is thrown */ }
}

If so, perhaps tsconfig-paths can expose an API to generate that list of candidates, then we can run them through a loop like above? I think node's resolver does FS caching internally, so this may improve performance.

jonaskello · 2020-07-29T15:37:28Z

What you describe is a bit similar to how it works in the current code.

The try-paths.ts module has code to generate a list of candidate paths to try. Specifically the function getPathsToTry generates this list. The try-paths module has (more or less) pure functions since it does not check anything in the filesystem. It returns a list of TryPath objects which is basically a string with the candidate path, but each entry also includes the type so the caller can know what generated each candidate path:

export interface TryPath {
  readonly type: "file" | "extension" | "index" | "package";
  readonly path: string;
}

The try-paths.ts module is called by the match-path-sync.ts or match-path-async.ts to retrieve the list of candidate path and then sync or async check them against the filesystem. This checking is similar to the loop you describe above but it call the filesystem directly instead of calling node's resolve function. Once an exact file is found in the filesystem as a match one of two things happen:

Call nodes resolve function with the exact file.
Strip some information off and then call node's resolve function with the stripped string.

I'm thinking the call to node's resolve function in the current code is not needed at all since we already know the exact file and can return it directly. At least the API functions (as documented here) could return the exact file and the code calling the API could then decide if it want to strip it and call node etc.

The above is just some description of how it currently works and how it could be improved with minimal changes. If we think about the new resolution things may have to be done differently. From what I understand the typescript team is considering adding more moduleResolution modes. Today tsconfig-paths is implicitly using the node mode. If node14 and deno modes are added I guess tsconfig-path somehow need to support them, probably by calling the API with which mode you want to use?

jonaskello · 2020-07-29T20:57:21Z

The new node14 resolution seems rather complex so I can see how avoiding implementing it all in tsconfig-paths could be helpful:

https://nodejs.org/api/modules.html#modules_all_together

cspotcode · 2020-08-10T17:58:26Z

I haven't really had any time for this, so I hope you'll forgive the terse, delayed response. I thought about what you said with tsconfig-paths returning the full, absolute path, always. And I thought about our needs in ts-node.

I think ideally tsconfig-paths should expose 2x ways to do resolution:

a) return a list of candidate paths with zero resolution attempted, allowing the execution environment such as node to attempt resolution for each one. ts-node can use this API. The returned array will not include /index candidates, or package.json files, candidates with added file extensions, or any of that. This also allows node to handle the more complex node14 resolution logic and uses its FS caching.
b) perform resolution completely, returning an absolute path. This will generate all candidates, perform FS calls, and determine the resolved file.

tl;dr I agree on (b) but also think that (a) should be exposed as an API.

jonaskello · 2020-08-10T21:58:07Z

Yes, this seems like a nice solution. So there will be one low-level API (a) where you only get the candidates and then a higher level API (b) that calls lower level but also does more work to make the resolution complete. Today it is done a bit like this where the getPathsToTry() is the lower level API but that part is not documented as a public API.

I also like the name candidate path. think the current name TryPath used in the code today should be renamed to CandidatePath, and getPathsToTry() to getCandidatePaths() etc.

its-dibo · 2021-09-07T16:55:24Z

I have this issue when I try to run webpack with webpack.config.ts written in typescript and ts-node has "require": ["tsconfig-paths/register"]

when I remove "require": ["tsconfig-paths/register"] from ts-node it runs, but webpack cannot detect aliases.

I already set module: commonjs in my tsconfig.json and removed type: module from my package.json

trace

> webpack

(node:5932) Warning: require() of ES modules is not supported.
require() of /home/node_modules/colorette/index.js from /home/node_modules/webpack-cli/lib/utils/index.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename /home/node_modules/colorette/index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /home/node_modules/colorette/package.json.
(node:5932) UnhandledPromiseRejectionWarning: /home/node_modules/colorette/index.js:28
export const options = Object.defineProperty({}, "enabled", {
^^^^^^

SyntaxError: Unexpected token 'export'
    at new Script (vm.js:84:7)
    at NativeCompileCache._moduleCompile (/home/node_modules/v8-compile-cache/v8-compile-cache.js:240:18)
    at Module._compile (/home/node_modules/v8-compile-cache/v8-compile-cache.js:184:36)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:991:10)
    at Module.load (internal/modules/cjs/loader.js:811:32)
    at Function.Module._load (internal/modules/cjs/loader.js:723:14)
    at Module.require (internal/modules/cjs/loader.js:848:19)
    at require (/home/node_modules/v8-compile-cache/v8-compile-cache.js:159:20)
    at Object.get colors [as colors] (/home/node_modules/webpack-cli/lib/utils/index.js:3:16)
    at Object.error (/home/node_modules/webpack-cli/lib/utils/logger.js:5:58)
(node:5932) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:5932) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
`

Originally during the file path resolution, file extensions were removed without explicit reason. This commit changes the resolution logic to keep file extensions, with the goal to add support for modern non-js extensions like cjs. The change however keeps /xyz/index resolves as is as they still should resolve to /xyz. Refs 847d314 Co-authored-by: Alec Larson <1925840+aleclarson@users.noreply.github.com>

Originally during the file path resolution, file extensions were removed without explicit reason. This commit changes the resolution logic to keep file extensions, with the goal to add support for modern non-js extensions like cjs. The change however keeps /xyz/index resolves as is as they still should resolve to /xyz. Refs 847d314 Co-authored-by: Andrew Bradley <cspotcode@gmail.com> Co-authored-by: Alec Larson <1925840+aleclarson@users.noreply.github.com>

Originally during the file path resolution, file extensions were removed without explicit reason. This commit changes the resolution logic to keep file extensions, with the goal to add support for modern non-js extensions like cjs. The change however keeps /xyz/index resolves as is as they still should resolve to /xyz. Refs 847d314 Co-authored-by: Andrew Bradley <cspotcode@gmail.com> Co-authored-by: Alec Larson <1925840+aleclarson@users.noreply.github.com> Co-authored-by: Katja Lutz <mail@katjalutz.ch> Co-authored-by: Andrew Bradley <cspotcode@gmail.com>

bradennapier mentioned this issue Jul 22, 2020

Feature Request: Add support for "Solution Style" tsconfig.json file TypeStrong/ts-node#1057

Open

cspotcode mentioned this issue Jul 23, 2020

Becoming a maintainer #134

Closed

jonaskello added the help wanted label Jul 23, 2020

katywings added a commit to lufrai/tsconfig-paths that referenced this issue Jul 24, 2020

test: consider extensions in resolved paths (dividab#133)

8629fc6

katywings mentioned this issue Jul 24, 2020

fix: regard file extensions from package.json during path resolution (#133) #135

Closed

katywings added a commit to lufrai/tsconfig-paths that referenced this issue Jul 26, 2020

test: consider extensions in resolved paths (dividab#133)

2380f89

baseUrl has a side effect on the used dependency export #133

baseUrl has a side effect on the used dependency export #133

Comments

katywings commented Jul 22, 2020 • edited

Is

Should

Reproduction

Trace

Background:

cspotcode commented Jul 22, 2020 • edited

jonaskello commented Jul 23, 2020 • edited

jonaskello commented Jul 23, 2020 • edited

jonaskello commented Jul 23, 2020

katywings commented Jul 23, 2020

jonaskello commented Jul 23, 2020 • edited

katywings commented Jul 23, 2020 • edited

jonaskello commented Jul 23, 2020

katywings commented Jul 23, 2020 • edited

katywings commented Jul 23, 2020

cspotcode commented Jul 23, 2020

jonaskello commented Jul 23, 2020

jonaskello commented Jul 23, 2020

cspotcode commented Jul 23, 2020 • edited

jonaskello commented Jul 23, 2020 • edited

cspotcode commented Jul 23, 2020

jonaskello commented Jul 23, 2020

jonaskello commented Jul 23, 2020 • edited

katywings commented Jul 24, 2020

katywings commented Jul 24, 2020 • edited

cspotcode commented Jul 26, 2020 • edited

jonaskello commented Jul 26, 2020

jonaskello commented Jul 26, 2020

jonaskello commented Jul 26, 2020

katywings commented Jul 26, 2020

jonaskello commented Jul 26, 2020

cspotcode commented Jul 26, 2020

jonaskello commented Jul 26, 2020

jonaskello commented Jul 26, 2020 • edited

cspotcode commented Jul 27, 2020

cspotcode commented Jul 28, 2020

jonaskello commented Jul 29, 2020 • edited

jonaskello commented Jul 29, 2020

cspotcode commented Aug 10, 2020

jonaskello commented Aug 10, 2020

its-dibo commented Sep 7, 2021 • edited

katywings commented Jul 22, 2020 •

edited

cspotcode commented Jul 22, 2020 •

edited

jonaskello commented Jul 23, 2020 •

edited

jonaskello commented Jul 23, 2020 •

edited

jonaskello commented Jul 23, 2020 •

edited

katywings commented Jul 23, 2020 •

edited

katywings commented Jul 23, 2020 •

edited

cspotcode commented Jul 23, 2020 •

edited

jonaskello commented Jul 23, 2020 •

edited

jonaskello commented Jul 23, 2020 •

edited

katywings commented Jul 24, 2020 •

edited

cspotcode commented Jul 26, 2020 •

edited

jonaskello commented Jul 26, 2020 •

edited

jonaskello commented Jul 29, 2020 •

edited

its-dibo commented Sep 7, 2021 •

edited