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

Support type stripping in node_modules #57215

Open
daquinoaldo opened this issue Feb 26, 2025 · 39 comments
Open

Support type stripping in node_modules #57215

daquinoaldo opened this issue Feb 26, 2025 · 39 comments
Labels
feature request Issues that request new features to be added to Node.js. strip-types Issues or PRs related to strip-types support

Comments

@daquinoaldo
Copy link

What is the problem this feature will solve?

The ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING arises if you import a TypeScript file from another package. I understood this is to discourage having TypeScript-only packages. However, this also applies to private packages, where the organization decides whether a TS-only ecosystem fits for them. This is particularly annoying in monorepos, where imports are local, because it forces you to build the changed package before using it instead of changes just propagating naturally.

What is the feature you are proposing to solve the problem?

We could have an opt-in flag, environment variable, or package.json field that enables type stripping in node_modules. In this way, we keep discouraging TypeScript-only public packages, while allowing their usage in contexts where it makes sense.

What alternatives have you considered?

  • pnpm Workspaces: In a monorepo, pnpm symlinks local packages, so development works fine. But when building an artifact (like a container image), pnpm prune copies local packages into node_modules, forcing you to copy the entire monorepo—which increases the artifact size.

  • Turbo.build: Turbo lets you selectively copy only the necessary packages, avoiding the need to hardcode local packages in node_modules. The trade-off is that it introduces extra tooling and only works in monorepos; using local packages from cloned repositories still poses challenges (TS in development vs. JS in builds).

  • File Watcher: A file watcher can rebuild changed dependencies and restart the server on file changes. However, this requires complex logic for monitoring files and managing dependencies.

@daquinoaldo daquinoaldo added the feature request Issues that request new features to be added to Node.js. label Feb 26, 2025
@github-project-automation github-project-automation bot moved this to Awaiting Triage in Node.js feature requests Feb 26, 2025
@aduh95
Copy link
Contributor

aduh95 commented Feb 26, 2025

You can use a loader/module hook to workaround that. /cc @JakobJingleheimer

@JakobJingleheimer
Copy link
Member

However, this also applies to private packages, where the organization decides whether a TS-only ecosystem fits for them. This is particularly annoying in monorepos, where imports are local, because it forces you to build the changed package before using it instead of changes just propagating naturally.

I believe this was specifically addressed, the package just needs "private": true to be set in its package.json.

You can use a loader/module hook to workaround that. /cc @JakobJingleheimer

Yes, you can use a loader like @nodejs-loaders/tsx. It's pretty trivial to set up 🙂

@marco-ippolito
Copy link
Member

I think a flag to enable it would be reasonable

@marco-ippolito marco-ippolito added the strip-types Issues or PRs related to strip-types support label Feb 26, 2025
@ljharb
Copy link
Member

ljharb commented Feb 26, 2025

For private true only packages, this seems fine to me; bit i don’t think node should ever provide anything that directly allows stripping types from published packages (not counting the tools so a loader can be easily built)

@marco-ippolito
Copy link
Member

For private true only packages, this seems fine to me; bit i don’t think node should ever provide anything that directly allows stripping types from published packages (not counting the tools so a loader can be easily built)

I created a PR that read the package.json private property. But private apparently does not allow publishing at all

@ljharb
Copy link
Member

ljharb commented Feb 26, 2025

Correct, that’s why it’s a viable escape hatch - because nothing can ever be published with private true, so the only way to have one of those in node_modules is if it’s bundled or linked in.

@aduh95
Copy link
Contributor

aduh95 commented Feb 26, 2025

Correct, that’s why it’s a viable escape hatch - because nothing can ever be published with private true

… on the public registry of npm, I’m not sure it makes sense to rely on it and assume it’s always going to be the case

@ljharb
Copy link
Member

ljharb commented Feb 26, 2025

Certainly an unknown npm client, or an unknown private registry, could violate that rule (all known ones comply with it afaik), but the concern is package ecosystem leakage, and anything in those buckets isn't part of the package ecosystem.

@aduh95
Copy link
Contributor

aduh95 commented Feb 26, 2025

Node.js doesn't read the private property at all atm IIRC, it would feel a bit too magical to start now, I'd much prefer a flag – or even keep loaders as the only way to get this to work, the loader itself can choose to read the private field or not

@marco-ippolito
Copy link
Member

Id create a flag and keep it opt in forever.

@ljharb
Copy link
Member

ljharb commented Feb 26, 2025

As long as the flag didn't work in NODE_OPTIONS or any committable config file, that'd be fine - otherwise, it would still cause ecosystem leakage since package authors who want to ship TS directly will just tell people to commit that config somewhere.

@daquinoaldo
Copy link
Author

Hi! Thanks everyone for your answers. 🙏

@JakobJingleheimer, I have "private": true in all package.json, but it seems not to work for my setup.
I created a reproducible demo with instructions in daquinoaldo/pnpm-monorepo-type-stripping.
I'm using pnpm deploy which to the best of my knowledge is the recommended strategy for Docker deployments of workspaces.

I suspected it was because of the symlink, but even adding node-linker=hoisted in the .npmrc it doesn't work.

@daquinoaldo
Copy link
Author

By the way, @aduh95 seems to be right: Node doesn't check the private property of package.json.

References:

function stripTypeScriptModuleTypes(source, filename, emitWarning = true) {
if (emitWarning) {
emitExperimentalWarning('Type Stripping');
}
assert(typeof source === 'string');
if (isUnderNodeModules(filename)) {
throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(filename);
}

node/lib/internal/util.js

Lines 516 to 518 in 269c851

function isUnderNodeModules(filename) {
return filename && (RegExpPrototypeExec(kNodeModulesRE, filename) !== null);
}

@marco-ippolito
Copy link
Member

This is the PR that would have allowed it (I closed it) #55385
I can confirm node.js does not check private

@JakobJingleheimer
Copy link
Member

Ah, my mistake. I thought Marco's PR had landed.

A loader like @nodejs-loaders/tsx it is for now then.

@DanielRosenwasser
Copy link
Member

I don't think any of the concerns we raised last time have good answers, and adding a flag just adds pressure to main-lining this into Node.js.

From the user-side, ensuring every invocation of node uses such a flag requires the same amount of precision as remembering to pass something like --import="amaro/register" or --import=@nodejs-loaders/tsx to those same invocations of node. So while that might not require a dependency, the win here feels marginal.

So can we continue with the current recommendation of using a loader?

@marco-ippolito
Copy link
Member

marco-ippolito commented Feb 27, 2025

What if gave it a try as a pure experiment? Maybe IDE can find a workaround and we can start publishing TypeScript source code in the future? The Node.js syntax constraint are strict enough that only a very small and stable subset is allowed

@ljharb
Copy link
Member

ljharb commented Feb 27, 2025

It would never be a good idea to allow people to easily publish untranspiled TypeScript code, unless TypeScript reached a point where it would never make a breaking change or change its recommended configuration ever again, which I suspect it's unlikely to ever do.

@marco-ippolito
Copy link
Member

It is already possible to publish ts.
Consuming it, can be opt in with a flag (cannot be set from node_options or config file).
I don't see the drawback of allowing some users to use it during development.

@RyanCavanaugh
Copy link

With --strip-types, we succeeded in greatly reducing friction in a scenario where there's nothing wrong with having TS code (i.e. in your local repo). We went from devs having to run a weird command to devs not needing to do anything, and that was great.

But by the same token, the intentional choice there was to not include stripping in node_modules, because we don't want unstripped TS entry points in the registry. That friction is there on purpose, and we don't want to remove it. With this feature we'd go from devs having to run a weird command to devs having to run a different weird command. I don't understand the point of that.

What's the goal in adding a second not-very-different way to do the same thing? The only endgame I can see here is allowing TS-only packages in the registry, and that's a conversation that should be had directly if we want to revisit it.

@marco-ippolito
Copy link
Member

I think the value is to publish source code that does not need a build step.
If done right it could benefit everyone. Allowing it through an experimental flag would maybe be a good test to see if its distruptive or can be worked around

@marco-ippolito
Copy link
Member

marco-ippolito commented Feb 27, 2025

I'm not gonna push on this direction because I trust your judgment of knowing whats the best for typescript ecosystem. My point is that maybe it wouldnt be too bad as we think and it could unlock some possibilities

@RyanCavanaugh
Copy link

I hear you. We really want TS authors to build into .d.ts as part of publish because we already know the effects of not doing that: https://gist.github.com/andrewbranch/6f11e6e0c3fb9a294590a061249264b0 . Allowing direct TS ingestion will directly contradict that goal.

I think there's a reasonable argument to say that e.g. npm publish should be more capable of figuring things like "there is a tsconfig here"; just because the build step has to happen doesn't mean it should be the kind of thing that's so easy to forget -- I know I've published stale JS files before because I was too lazy to set up a proper prepublish pipeline.

@GeoffreyBooth
Copy link
Member

we already know the effects of not doing that: gist.github.com/andrewbranch/6f11e6e0c3fb9a294590a061249264b0

This is an interesting analysis. The way I read it, it's saying that publishing .ts files instead of .d.ts files will result in slower tsc performance. Which is completely expected. But when we first shipped the type stripping feature, we did some measurements of how much the stripping impacted Node’s performance and we found it to be minimal. So it could very well be that authors publishing .ts files to the registry is bad for tsc users but not necessarily bad for Node users.

The gist points to a potential solution: the .d.ts files in this case are basically functioning like a compiled output of a .ts file, providing tsc with a faster-to-process version of the same content. So one way that tsc can handle this is to generate the .d.ts files on demand the first time it encounters a new .ts file under a node_modules folder. “Ah but using what tsconfig” I can hear you asking, but it would use whatever settings Node would use (that is, that mimics how Node would strip the types) or it would use a tsconfig.json if present, or only do this behavior at all if a tsconfig.json is present. This could also be an opt-in behavior on tsc’s part, where the default is the status quo of processing the .ts files. With this opt-in, users could get the benefits of .ts files published to the registry without the performance hit to tsc.

@RyanCavanaugh
Copy link

So one way that tsc can handle this is to generate the .d.ts files on demand the first time it encounters a new .ts file under a node_modules folder.

Let me list some of the obstacles here; this is not an exhaustive list.

The local developer might be using an older version of TS than what the published author was using, so the .ts file in the registry might contain syntax that can't even be parsed. It just doesn't work.

The local developer might be using a newer version of TS than what the published author was using, so the .ts file in the registry might generate an invalid (i.e. produced in the presence of type errors) declaration file due to bugs fixed in the newer version. Things will be unexpectedly any (bad).

Most projects out there can and do have custom build steps that are beyond just running tsc. You might need to e.g. generate a .d.ts schema from a file before tsc can run. We have a script for running the top 1,000 github repos and effectively none of them can get away with a bare tsc invocation to produce correct output.

I don't know where on disk these .d.ts files would go; it's sus to put them directly in node_modules (only package managers should be touching that folder). In fact it's wrong to do so, because different TS versions could be operating in the same node_modules context, and they will not agree on what a correct .d.ts output (e.g. a newer version could output something an older version can't parse). We would have to have some kind of sidecar directory, and it would need some kind of cache eviction strategy to stop from growing outlandishly large.

Correctly building a .d.ts from a .ts file requires knowing all its dependencies, and those dependencies can and do include things like a dev dependency on [email protected] (where the import wouldn't appear in your emitted .d.ts, but is required in the .ts code to e.g. resolve an overloaded call). That package doesn't even get installed by default, so we'd have to go to the network to get it, and put it somewhere rational on disk. I don't really like the idea of a world where, before you can run tsc, you have to do a subsequent npm install that pulls down the dev deps of each package - this is a combinatorially multiplicative process unless you're willing to make some rather dicey assumptions about type compatibility between two different package versions (people generally semver their package behavior, not their type surface). Again here, different TS configs mean that you can't safely reuse any module resolution results between packages, nor can you reuse any type computations.

Because you need the identical external .d.ts files that the author had, which can come from anywhere, you need to be using an identical package manager to the one the package author was using, and be invoking it the same way that the package author does, at the same version. If the package author was using e.g. a yarn-specific feature not supported by your local package manager, too bad, things aren't going to work.

The caching strategy here is also fraught. Correctly building a .d.ts from a .ts file requires knowing all its dependencies including everything that goes into the global scope, so the cache key here is not just "what is content of the input file", but "what are the contents of the entire transitive graph of files that this file causes to load", so each file really requires global knowledge of everything that was originally present during authoring. In this world, in a monorepo setting you don't really know if something from node_modules is actually user code or not, so you can't just optimistically assume that those external files can be skipped the second time around.

It gets worse from there, because you can't share this process between different packages, since it's not correct to just build one giant global scope - the global scope has to be computed for each project. especially because of config differences. So the first time you do this process, you're not just building the project you depend on, you're building all its dependencies, from scratch, in graph dependency order.

Assuming all these problems can be solved or ignored, I just think this is placing the burden in the wrong place from a numerical standpoint. We're talking about doing this on every dev machine on every install forever, instead of once on each publish. For most packages, the install to publish ratio is conservatively 200,000:1, maybe more? A normal npm install that would previously have you instantly at a working state now requires tsc to run 200-600 times (possibly more, if there are sub-packages) to get up-to-date, instead of zero? To me it's just super obvious which side of the publish boundary this belongs on and I don't think the friction of setting up a prepublish step, once, is that bad.

In fact, given the constraints, it's much cleaner to recommend "run tsc then publish" than to explain to them the entire set of caveats that would need to be met for "you don't need to run tsc" to work. You would need a tool that would validate that your npm package could be correctly .d.ts'd on the fly, and at the point you're running that tool, why not just run tsc instead and publish the .d.ts file that you know is correct (and save your downstream users the laptop fan spin)?

@GeoffreyBooth
Copy link
Member

Correctly building a .d.ts from a .ts file

In this scenario, the .d.ts file is basically a result of parsing something else. The fact that tsc can support .ts files under node_modules means that there is some result generated internally that represents parsing and checking a particular file. Even if that can't easily be saved out as .d.ts, presumably it can be saved somehow; perhaps in a cache folder similar to how eslint caches. So really the cost of .ts files could be incurred once for each file/hash, if you wanted to support this in a performant way.

To which I can hear to argue, why should we want to support that. Besides the monorepo use cases, there are those who simply want to live in a world where .ts replaces .js and has the same level of convenience: it's standardized and there's no need for configuration or sidecar files like .d.ts. That's part of the point of type stripping: to provide a way forward toward that future.

@DanielRosenwasser
Copy link
Member

The file load overhead thing is possibly solvable in ways you've described (though the idea that we'd make every machine out there do a compile unnecessarily isn't something I would advocate for, even if we could cache, and even through every effort to make TS faster). There are still lots of problems we've mentioned around compatibility which declaration files are good for because they hide some certain details and provide maximal compatibility guarantees.

@marco-ippolito
Copy link
Member

Id say we should come back to this topic when the typescript support in Node.js is more stable. For now I'd mark it as wont fix.

@daquinoaldo
Copy link
Author

daquinoaldo commented Feb 28, 2025

I confirm that using a loader works, and I strongly recommend amaro if that's the path.

However, I'm not fully convinced of this path:

  • The nodejs-loaders repository clearly states: "These should NOT be used in production". The problem with workspaces is that they don't work when built because the code is copied to node_modules (e.g., with pnpm deploy). In local development, they already work because the code is symlinked inside node_modules, but the actual path is different, and Node considers the resolved path, not the symlink one. If loaders shouldn't be used in production, they don't solve the problem.
  • The @nodejs-loaders/tsx loader that @JakobJingleheimer recommended states: "If you are using only TypeScript (not tsx), consider using Node.js's builtin type stripping. This can handle it, but the builtin may provide better/more consisent results." And I agree with that. Indeed, if a loader has to be used, I would go with amaro/strip.
  • To use a loader, I need to install it and then add it to the Node command as a flag. It's not as heavy as using tsx, but the developer experience is roughly the same. Also, why install and override the amaro version if it's already shipped with Node.js?

I personally would like the experimental flag to allow people to ship TS packages to see how it goes, but I hear your concerns about the risks for the ecosystem, and I respect that.

Can you instead reconsider @marco-ippolito's PR #55385?
It's safe for the ecosystem because "private": true forbids publishing, so there is no risk for TS-only packages on NPM. It was closed because of @jakebailey fair comment, who wondered how a private (therefore unpublishable) package could end up in node_modules without being symlinked. We now have a concrete use case: workspaces.

@JakobJingleheimer
Copy link
Member

Loaders should not be used in production because you pay the cost every time. That makes sense in development where files are changing every time. For production, they do not change at all, so you should instead pre-compile with a tool like esbuild.

You should also not use them in production because many of them provide only an inexpensive facsimile that is sufficient in their target environment. For example, nodejs-loaders/css-module provides a simple key-value pair of css identifiers like you would see used in jsx, and does not expose a CSSStyleSheet that you would need in production for a browser to render the page. The cost difference is huge. Again, esbuild for production.

"If you are using only TypeScript (not tsx), consider using Node.js's builtin type stripping. This can handle it, but the builtin may provide better/more consisent results."
And I agree with that. Indeed, if a loader has to be used, I would go with amaro/strip.

That's not what that text means, but sure. I'm not sure whether amaro/strip will go into node_modules though (I haven't checked, but I expect it to have the same restriction as node itself because it's the tool mode is using to do this internally).

Also, why install and override the amaro version if it's already shipped with Node.js?

Because you need to do different things. Unfortunate to have to pay the hard disk space twice.

@daquinoaldo
Copy link
Author

I'm not sure whether amaro/strip will go into node_modules though (I haven't checked, but I expect it to have the same restriction as node itself because it's the tool mode is using to do this internally).

It does, see daquinoaldo/pnpm-monorepo-type-stripping#1. The reason is that Node.js type stripping is a wrapper around amaro (ref), but the check about node_modules is in the wrapper, not in amaro (see #57215 (comment)).

Also, why install and override the amaro version if it's already shipped with Node.js?

Because you need to do different things.

Sorry, @JakobJingleheimer, I'm not very knowledgeable on loaders. Can you explain why I need to do different things?
The --import 'amaro/strip' directive seems to replicate import("amaro/strip") (#43942), which executes a registration of the same transformSync that Node.js uses. It's not clear to me the difference between that register and how Node.js is using it for type stripping. The core function is the same, though.

@JakobJingleheimer
Copy link
Member

but the check about node_modules is in the wrapper, not in amaro

Ah, good :) By the way, the code in node you cited is not where the check is. It's here:

if (isUnderNodeModules(filename)) {
throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(filename);
}

Sorry, @JakobJingleheimer, I'm not very knowledgeable on loaders. Can you explain why I need to do different things?

Sure! I'm happy to improve the documentation. Please let me know (perhaps a DM so we avoid cluttering this issue) what parts are unclear or need expanding 🙂 I could potentially also write up a Learn article if it's more about consumption.

@daquinoaldo
Copy link
Author

I'm trying to use esbuild in daquinoaldo/pnpm-monorepo-type-stripping#2, but it breaks on some dependencies, recommending to list them as external.
I worked it around for direct dependencies of my app, but it still breaks on dependencies of other private packages in the workspace. This would imply having to manually list all problematic dependencies recursively, which is a very annoying operation.
If we apply the same workaround in private packages, we would need to bundle them first and then the final app (which I think is not the purpose of the bundler), but at this point, it is the same DevX as compiling TypeScript (e.g., you needTurbo.build or similar tooling).

Why is running Node.js type stripping or a loader in production discouraged? From @GeoffreyBooth's comment, I understood that the overhead of type stripping is minimal.

@JakobJingleheimer
Copy link
Member

The problems you're encountering with esbuild are likely due to misconfiguration and/or misconsumption. I have almost never needed to mark a dependency as "external" for esbuild (basically, only for OS-specific packages). Esbuild is probably not recommending you mark them as external (I have never seen it do that), but rather suggesting that may be appropriate (it probably isn't).

Loaders in general are a development API, not a production API. Type stripping is relatively cheap but not free, and the only reason to pay¹ that cost in production is laziness 😜 (we're all as lazy as we can be, but this is not the place to be lazy). So amaro is just as inappropriate for production as nodejs-loaders 😉 Amaro may well suit your development needs better than nodejs-loaders/tsx.

¹ it will literally cost you money

@paulovieira
Copy link

Great discussion, I learned a lot with this issue. My conclusion is that typescript inside node_modules is discouraged and a loader should be used instead.

Just wanted to bring attention to an alternative type-stripping compiler that doesn't seem to be much talked about: https://github.com/bloomberg/ts-blank-space

This thing works great and can also be used as a loader. A quick test:

mkdir -p ./node_modules/the-package

echo "let x: number = 123; console.log({ x });" > ./node_modules/the-package/index.ts

node --import="ts-blank-space/register" ./node_modules/the-package/index.ts

One interesting thing about ts-blank-space is that it depends only on the typescript compiler, unlike other similar tools (which require esbuild, biome, etc). So if you like simplicity this might be a good choice.

@ljharb
Copy link
Member

ljharb commented Mar 1, 2025

@paulovieira pretty sure ts-blank-space is what node uses, via amaro.

@jakebailey
Copy link

Amaro is just SWC with some options set, and that's bundled with Node (but also on npm). ts-blank-space has the same idea (replacing types with spaces + ASI fixups etc) but using TS's APIs, largely for simplicity but also because it plugs well into Bloomberg's build system where they need the ASTs anyway. But they are different.

(I think ts-blank-space was being worked on before strip types?)

@ljharb
Copy link
Member

ljharb commented Mar 1, 2025

ah, fair - but i'm still pretty sure that SWC's implementation was inspired by ts-blank-space.

@paulovieira
Copy link

ah, fair - but i'm still pretty sure that SWC's implementation was inspired by ts-blank-space.

Yes, I think ts-blank-space was the first one with this approach (released to the public in september/2024) and SWC quickly followed. This thread has some context: https://x.com/acutmore/status/1836762324452975021

Another interesting thing: this benckmark by @acutmore shows it is 2x faster than SWC/wasm (which is what amaro is currently using?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. strip-types Issues or PRs related to strip-types support
Projects
Status: Awaiting Triage
Development

Successfully merging a pull request may close this issue.

10 participants