module: add --experimental-strip-private-modules

[marco-ippolito]

Followup of #63853
Adds a flag to allow type stripping inside node_modules ONLY if private.
I think I covered #63853 (comment) concerns about smuggling private package.json's in a non private package

[nodejs-github-bot]

Review requested:

• @nodejs/config
• @nodejs/loaders
• @nodejs/typescript

[aduh95]

We'd need to document private in https://nodejs.org/api/packages.html#nodejs-packagejson-field-definitions

[marco-ippolito]

Adding the blocked label to make sure @nodejs/typescript reviews and agrees with this change

[mcollina]

lgtm

[aduh95]

I don't think it's worth it, also I don't think that we can use private to reliably guess if a package was downloaded from a registry or not, and it adds arguably a lot of complexity (in comparison for e.g. adding a loader, or not putting the code inside node_modules)

[RyanCavanaugh]

Weighing in from the TypeScript team: We're OK with this, and don't have any concerns about this eventually going unflagged either.

That said, we wanted to re-emphasize @ljharb's and others' comments in the prior PR thread. Our top priority in this space remains ensuring that un-compiled .ts is never the entry point for a package in the public registry. The fundamental technical motivations behind that haven't changed, and we don't see any path forward where they could change.

Keeping this restricted to private: true is a great compromise that enables that goal while still making monorepo dev/deploy more straightforward, we just want to have absolute clarity and consensus that this is not a stepping stone to enable type stripping of public registry packages.

[marco-ippolito]

we just want to have absolute clarity and consensus that this is not a stepping stone to enable type stripping of public registry packages.

To be explicit for @nodejs/typescript: this is not a stepping stone to public-registry TS.
I have it documented as a non goal
https://github.com/nodejs/node/pull/63869/changes#diff-7349c17bce4d97526eefcb6f8291d617e7edb92ed9927fd3b9264e8014fc4e7cR1423

@aduh95 I kindly ask you to reconsider.
The design relies on the contract that registries refuse to publish a "private": true root.
Im sure there are a million ways to work around that, but for the vast majority of people it doesn't matter.
And just like people didnt want to run TypeScript through an external loader, they want a built-in solution.
The real complexity today is in the hoops people have to go through to bypass this limitation that you mentioned here: #63853 (comment)
This PR is small and self-contained in comparison, and it removes that burden.
I also think this opens the door to experimenting in ways that were harder before.

[andrewbranch]

It would also be good to have one of the test fixtures' "private": true packages include a relative import of another TypeScript file in the same package, so it's clear that finding the "private": true package.json that allows type stripping doesn't depend on that same package.json being looked up as part of module resolution. I think the implementation already works this way, but it wasn't clear from reading the tests.

[GeoffreyBooth]

Our top priority in this space remains ensuring that un-compiled .ts is never the entry point for a package in the public registry. The fundamental technical motivations behind that haven't changed, and we don't see any path forward where they could change.

Just to be clear: I don't think Node has any technical issues with TypeScript packages in node_modules, whether from the public registry or other sources. So really the ask here is to discourage public publishing of TypeScript packages because of some performance problem of tsc. Correct?

And if I remember correctly, the performance problem had to do with tsc parsing all the types of those packages, and their potential publishing with different tsconfig settings. And if they're published to the public registry as plain JavaScript, they're parsed as such and so no problem. Correct so far?

So wouldn't then a potential solution to the tsc performance problem be for tsc to treat TypeScript packages under node_modules the way Node would, by stripping the types and parsing as plain JavaScript? That would seem to be the same as the current approach of insisting that users publish as plain JavaScript.

[DanielRosenwasser]

Sorry, I don't understand that suggestion and how it would address any of the concerns we've raised every time this discussion comes up.

[GeoffreyBooth]

Adding to the TSC agenda as I think we should land this. @nodejs/tsc please chime in

I think it's a bit premature. Let's see what the TypeScript team replies to my last comment. I've never found the 'the ecosystem will suffer' argument all that convincing because it relies on a series of hypotheticals all going a certain way, which I don't think is inevitable. I think there's probably still room for consensus here.

[aduh95]

Correct me if I'm wrong, but I tihnk there's no tool out there that will strip out the private: true when publishing, so this is giving incentives to not do open-source, which is a weird move. It's still unclear to me what is the use-case (monorepos? but some claim monorepos are already supported...), so I'm not convinced this flag is a solution, or even part of the solution. I could be convinced if some monorepo tool maintainer was able to chime in saying this is something they need/want

[kdy1]

I think private-only is a good direction, and will be useful enough

[anonrig]

What if I publish my packages but to a private registry to be used on a project of ours, just like a monorepo?

I'm sorry but this is just a bandaid.

[kdy1]

It depends on the private: true field in the package.json.
I think publish commands would stop working if it has the field, though

[ljharb]

You can't publish anything with private: true in any npm-ish client I'm aware of, nor would any registry I'm aware of serve a package with private: true.

Certainly you could go to some lengths to add private:true to things postinstall, but at that point you've spent more effort than just using a loader.

As for the "ecosystem will suffer" argument, it's already empirically demonstrated by React Native - the entire RN ecosystem has to always be on the latest version of Metro or everything breaks. I'll happily bet real wads of cash that it will happen if we create a scenario where raw TS can be published, and I'll make out like a bandit. Bear in mind that tsconfigs vary far more than Metro configs can, so the problem will be a thousand times worse here.

[marco-ippolito]

@aduh95 This doesn't incentivize anyone to make code private. The flag changes nothing about the publish decision, there's no upside to marking a package you want public as private: true, because that just stops you from publishing it. It only affects how already-private code (code that was never going to the registry) is consumed locally.

The use case is when a workspace package is copied into node_modules rather than linked: pnpm deploy, Docker build contexts, vendoring as described here #63853 (comment)

Let's not let perfect be the enemy of good.
This is a strict improvement over the status quo, it's opt-in and the private: true contract makes it conservative in the safe direction. It doesn't have to solve EVERY monorepo topology to be worth landing.

[arcanis]

I'd wait for #63653 to be landed first as you're adding a new binding that directly calls the filesystem and would otherwise bypass the vfs.

[aduh95]

Let's not let perfect be the enemy of good.

Yeah but let's not discriminate open-source software in favor of closed-source, I'm strongly -1 for a solution that leaves open-source out.

I suppose that upon copying, the monorepo tool could add private: true – but at this point, why not strip the types?

[marco-ippolito]

Let's not let perfect be the enemy of good.

Yeah but let's not discriminate open-source software in favor of closed-source, I'm strongly -1 for a solution that leaves open-source out.

I suppose that upon copying, the monorepo tool could add private: true – but at this point, why not strip the types?

A software can have private: true and be open source. It means it shoulnt be published at all. Non that it can be published on a closed source registry, or be non accessible. It means its not meant to be published on npm, or any registry. If you ship a docker image for example or a binary like desktop apps, you dont want to need to publish on npm. Not all Node.js user code is a package that needs to be published.

[GeoffreyBooth]

A software can have private: true and be open source. It means it shoulnt be published at all.

Couldn't it be a regular GitHub repo? And those can be installed directly.

[marco-ippolito]

A software can have private: true and be open source. It means it shoulnt be published at all.

Couldn't it be a regular GitHub repo? And those can be installed directly.

Maybe yes I havent tried.
https://docs.npmjs.com/cli/v11/configuring-npm/package-json#private
Documentation just says npm will refuse to publish it

[GeoffreyBooth]

Documentation just says npm will refuse to publish it

Sure but it doesn’t need to be published to the public registry. Like you can do npm install https://github.com/nodejs/undici and that will install Undici directly from its GitHub repo. It appears in your package.json dependencies as "undici": "github:nodejs/undici". And the package.json in that GitHub repo could easily have private: true.

[ljharb]

True, but as of npm 12, you won't be able to do that by default (non-registry deps are highly discouraged and insecure).

[RyanCavanaugh]

The same place they would today when people publish JavaScript, in .d.ts files

Great. Let's circle back to figuring out what problem we're trying to solve.

If you never publish your package, then you can set private: true, and you can type-strip from within node_modules. This PR accomplishes that scenario.

If you don't publish your package, what's gained by publishing un-stripped TS (with the necessary .d.ts)?

Missing upsides:

• You don't save a build step in the local loop, because you still have zero build steps for local dev (you're not in node_modules during dev time)
  • Local monorepos work on realpath semantics, so nothing needs to change here either
• You don't save a build step in the publish flow, because you have to remember to re-generate .d.ts

Actual downsides:

• You have a new problem that it's easy to forget to update your .d.ts when your .ts changes
• Users looking in your package will see both .d.ts and .ts and not understand which is authoritative for type information
• You have a syntax versioning hazard if you start using type syntax from a too-new version of TS, which could happen in the future

What's the scenario that concretely benefits from type stripping in published packages?

• I don't really buy the "users can edit the original source in node_modules" argument very much. This is contingent on too many other factors (downleveling, minification, bundling, erasableSyntaxOff packages, etc), is rarely satisfied in practice even under this proposal, and this will continue to be the case for the foreseeable future
• What else is there?

[GeoffreyBooth]

What’s the scenario that concretely benefits from type stripping in published packages?

I think there are several use cases that would benefit from TypeScript files in published packages, and I think that requiring .d.ts declaration files alongside them should address the concerns. @RyanCavanaugh please take a look at #63936 and we can discuss in that thread.

[jakebailey]

There's a lot of chat here about TS being unable to make breaking changes, which is a concern and avoided by people not shipping TS for sure, but the problems are deeper.

A main concern was the prospect of us loading way more code in places like the editor or compilation, because that's what we'd read out of these packages. We've linked the analysis every time (I'm sure it's somewhere in the chain of many threads about this), but the impact would be pretty bad.

A method like:

So wouldn't then a potential solution to the tsc performance problem be for tsc to treat TypeScript packages under node_modules the way Node would, by stripping the types and parsing as plain JavaScript?

Does not work, because the whole point of TS is to parse out those types and do something with them. Imagine a lib whose function returns are inferred; we have to do the analysis!

Then, on top of that are the differing compiler options and how loading someone else's TS impacts that; if your tsconfig bans unused variables, but you load someone else's code that has those, your compilation is now broken. The same goes for other more serious type checking things like strictFunctionTypes, the lib directives, etc.

These are all of the concerns we raised when type stripping was added those years ago and was the reason why we pushed so hard to not create any scenario by which TS source would make it onto the registry.

The "adjacent .d.ts files" idea I do not think actually provides the right signal (I commented over there as have others), but if you're publishing .d.ts files, you must have run TS or some other tool, so I don't know how that plays into things at all; there must be some build process, or someone handwriting a d.ts file (why?).

[ljharb]

(fwiw i hand-write dts files, but that's because i use tsdoc to import their types into .js files - if i was authoring ts files, i can't imagine ever hand-writing a dts)

[RyanCavanaugh]

I think there are several use cases that would benefit from TypeScript files in published package

Can you explain what they are? Maybe there are other ways to accomplish this.

[GeoffreyBooth]

Can you explain what they are? Maybe there are other ways to accomplish this.

Sure. There are the ones I listed in #63936: first-party code shared across repos (private registry, Git deps, internal CLIs), and TypeScript-first packages that Deno and Bun run directly from source, where the author wants them to run on Node too without adding a transpile step solely for Node.

Regarding other ways, I can think of a few:

1. Ship type-stripped .js files alongside .d.ts files: same execution, and the .d.ts resolves cleanly with no .ts to shadow it. This is already possible today, but it’s still a build step and so it won’t satisfy the users looking to have the same files run in production as run in development, or the package authors looking to avoid a build step.
2. Change tsc‘s behavior to prefer .d.ts over a co-located .ts for files under node_modules (where the .ts is the runtime artifact, not editable source), which would make the approach in module: allow .ts in node_modules when .d.ts is present #63936 viable.
3. Only strip a node_modules .ts when it’s valid under --isolatedDeclarations: no .d.ts needed at all; the consumer’s tsc reads the .ts directly, and its types are quickly determinable per file, so no slow types and no build step (proposed in module: allow .ts in node_modules when .d.ts is present #63936 (comment)).

[ljharb]

Git deps are an antipattern and a security risk, which is why npm >= 12 will refuse to install them by default, so I think we can scratch that one off the list.

The "run directly from source" pattern in Bun and Deno is very brittle, since "the ideal TS config" constantly changes over time, and TS itself sometimes ships breaking changes even in minor releases - I'm not sure we want to endorse/encourage that pattern by making it easier for people to do an unwise thing?

[mcollina]

lgtm

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/74207/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/74210/