baseUrl has a side effect on the used dependency export

[katywings]

trafficstars

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 TypeStrong/ts-node#935 / 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 :see_no_evil:.
• 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 ;).

[cspotcode]

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]

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

1. 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.

2. 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?

3. 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]

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]

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]

@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]

@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]

@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]

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]

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]

@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]

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

It's already been published, and feedback is being tracked here. https://github.com/TypeStrong/ts-node/issues/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]

@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]

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]

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: https://github.com/nodejs/modules/issues/351#issuecomment-621257543 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]

@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]

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]

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

[jonaskello]

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]

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

[katywings]

@jonaskello Btw. in the original commit message for removeExtension you already added a hint "but why?": https://github.com/dividab/tsconfig-paths/pull/27/commits/847d31475665166aa6f30dc01e759435f4fb13a4, 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

[cspotcode]

@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:

1. https://github.com/dividab/tsconfig-paths/blob/720465935a11f394c525b98f65ad829409768ff8/src/match-path-sync.ts#L133-L136 It should not return Filesystem.removeExtension(mainFieldMappedFile) It should return TryPath.getStrippedPath(tryPath)

2. https://github.com/dividab/tsconfig-paths/blob/720465935a11f394c525b98f65ad829409768ff8/src/try-path.ts#L71-L72 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]

@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.

https://github.com/dividab/tsconfig-paths/blob/720465935a11f394c525b98f65ad829409768ff8/src/match-path-sync.ts#L64-L70

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]

@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:

https://github.com/dividab/tsconfig-paths/blob/720465935a11f394c525b98f65ad829409768ff8/src/try-path.ts#L63-L74

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]

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]

@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 ^^.

[jonaskello]

@katywings It is already covered by this test:

https://github.com/dividab/tsconfig-paths/blob/2461cc9c0aa6f02f27c679d1344adaf35ff05cf7/test/data/match-path-data.ts#L128-L142

[cspotcode]

@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]

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]

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]

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?