file-type icon indicating copy to clipboard operation
file-type copied to clipboard

Can't access node functions when tsconfig moduleResolution is set to "bundler"

Open alex-e-leon opened this issue 1 year ago • 5 comments

I'm using tsup to bundle my node project into a single file, and have moduleResolution: "bundler" set in my tsconfig because a) it feels like the right choice as I'm using a bundler, and b) it allows me to ignore writing out file type extensions with esm.

Unfortunately it looks like file-type only exposes node functions like fileTypeFromFile when moduleResolution is set to "node". I get that this is done to add extra safety and prevent consumers from importing functions that don't work in the environment that they are running, and 99% of projects with moduleResolution: "bundler" are likely headed for the browser, but considering that bundling is valid for node projects as well, I wonder whether the exports should be relaxed for this?

alex-e-leon avatar Aug 08 '24 00:08 alex-e-leon

Unfortunately it looks like file-type only exposes node functions like fileTypeFromFile when moduleResolution is set to "node".

file-type exports Node.js specific functions, like fileTypeFromFile, to JavaScript engines of type "node".

The moduleResolution, in your project, determines the module resolution strategy, and does, or could potentially, impact if the ESM exports (field in package.json) is correctly interpreted. More info: TypeScript: Module Resolution

Looks like "moduleResolution" = "bundler" is desgined for Bundlers transpiling to CommonJs. Maybe give "node16" a try.

functions that don't work in the environment that they are running, and 99% of projects with moduleResolution: "bundler" are likely headed for the browser, but considering that bundling is valid for node projects as well

That would break browser projects, they are not only ending up with functions they cannot use, but more problematic are the Node.js API dependencies.

This is a problem your project, possibly your bundler, not in file-type.

Borewit avatar Aug 08 '24 07:08 Borewit

Hey @Borewit unfortunately the issue is either a mismatch between nodes spec and typescripts spec, and has nothing to do with the bundler itself. I haven't actually checked how the bundler behaves in this case, because I get a typescript error which is preventing me from bundling at all.

Node says for exports "node" - matches for any Node.js environment. Can be a CommonJS or ES module file. In most cases explicitly calling out the Node.js platform is not necessary.

While typescript says for moduleResolution: 'bundler' for use with bundlers. Like node16 and nodenext, this mode supports package.json "imports" and "exports", but unlike the Node.js resolution modes, bundler never requires file extensions on relative paths in imports.

However in spite of those 2 specs agreeing that bundler should behave like node16/nodenext most use cases for moduleResolution: bundler typescript is not reading the node export, I believe because most use cases for moduleResolution: bundler are to bundle for the browser.

You can setup a quick example project to see that it is just this property in tsconfig which is preventing it from being imported.

In my case I've "fixed" the issue by switching from an ESM import to require, but I thought I'd raise it because commonJS isn't great and maybe there's a better way to expose the different functions in this situation, like exporting the "node" functions at a different path/name when moduleResolution is bundler.

alex-e-leon avatar Aug 08 '24 09:08 alex-e-leon

@alex-e-leon Can you try this branch? explicit-node-subpath-export, b75fc026c899eb6be1e192d96032ad7a10737460

npm install 'sindresorhus/file-type#b75fc026c899eb6be1e192d96032ad7a10737460'

Using subpath import file-type/node

I doubt it will solve your problem, as some of the sub-dependencies also rely on the node engine dependency.

Borewit avatar Aug 08 '24 11:08 Borewit

You need to open an issue on the TypeScript repo about this. This package is defined correctly. The problem is that moduleResolution: bundler assumes not node, so they need to add a way to override that. It doesn't make sense to do anything here.

sindresorhus avatar Aug 08 '24 11:08 sindresorhus

Hey, sorry for taking a while to get to this. Like I said I've already worked around it so it took me a while to get the time to come back to this.

@Borewit - your branch almost works. as there's no exported node.js file it doesn't resolve, but if instead the exports look like:

"./node": {
	"types": "./index.d.ts",
	"import": "./index.js"
}

Then yes I can import successfully with import {fileTypeFromFile} from 'file-type/node';

@sindresorhus - you're right it's really an issue with typescript, but considering the current state of the landscape I figured you'd want to know about the issue and potentially include a work around until typescript + node figure it out. With a bit of tweaking @Borewit 's solution seems to work fine and I don't think leaves too big a footprint for a workaround. But it's not blocking me at all, so I'll leave it up to you to make the call.

Thanks both for being so responsive!

alex-e-leon avatar Aug 13 '24 15:08 alex-e-leon

Would you mind we add an explicit sub-path for node @sindresorhus ?

See: https://github.com/sindresorhus/file-type/tree/explicit-node-subpath-export

Borewit avatar Sep 11 '24 14:09 Borewit

I would prefer not to. If all packages just work around the problem, the TypeScript team has no incentive to prioritize fixing this.

sindresorhus avatar Sep 11 '24 18:09 sindresorhus

I'll now close this issue. If there are any additional details we may have overlooked, feel free to reopen it.

Borewit avatar Sep 11 '24 19:09 Borewit

I'm leaving this here to document my workaround given the issue mentioned in https://github.com/sindresorhus/file-type/issues/652#issuecomment-2275594523

this packages has two export definitions in package.json

. and ./core

. resolves the node types assuming your ecosystem properly infers it's a node environment, and it defaults to the web ./core types otherwise.

./core resolves directly to the web types regardless (I'd like to point out this is already a type of "workaround" sanctioned by the package)

current best practices in the TS world have tsconfig.json setting moduleResolution: "bundler" for bundled libraries in a project (we run a TS monorepo with our libraries using this flag because we need the behavior that this flag provides)

the issue here, and as mentioned in the linked comment above, is that this does not properly resolve the . export to node and you end up with Web types (essentially ReadableStream vs Readable) I don't need to go into detail on the problems this causes in the rest of our project

to work around this we are using pnpm patch file-type and adding a ./node exports property to resolve to the proper types, similar to how the author already does this for ./core web types

"./node": {
    "types": "./index.d.ts",
    "import": "./index.js"
}

our project then uses file-type/node and all is well.

please note, that we'll be removing this patch from our project as soon as there is a better way to go about using the benefits of moduleResolution: "bundler" in a bundled node library when libraries have structured their package.json exports in the way this package does

I hope this helps someone in the future!

danalloway avatar Feb 21 '25 02:02 danalloway

Can you set the Node condition by hand?:

{
  "compilerOptions": {
    "target": "es2022",
    "moduleResolution": "bundler",
    "customConditions": ["node"]
  }

}

Borewit avatar Feb 21 '25 12:02 Borewit

Can you set the Node condition by hand?:

{ "compilerOptions": { "target": "es2022", "moduleResolution": "bundler", "customConditions": ["node"] } }

closing the loop here @Borewit, setting "customConditions": ["node"] worked for me, using it in our project now as a workaround.

thank you, super appreciate the collaboration!

danalloway avatar Mar 25 '25 16:03 danalloway

Since version v20.4.1 in may even work without the "node" condition set (#744).

Borewit avatar Mar 25 '25 21:03 Borewit