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.

[ljharb]

I don't think it's really about performance at all; it's that it would effectively prevent the TS team from ever evolving the language in a breaking way without breaking users.

[RyanCavanaugh]

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?

TS generally doesn't read the JS source of npm packages at all. I think you're confused about two entirely different processes. Where do types for the package come from in this proposal?

[GeoffreyBooth]

Where do types for the package come from in this proposal?

The same place they would today when people publish JavaScript, in .d.ts files. From the last time a TypeScript team member explained the concern:

#58429 (comment) Every package author writing .ts will publish .ts, then tell us (the TypeScript team) that TS should not require .d.ts files in this situation, since TS "can" infer the types from their implementation files. ... But doing this is catastrophic from a performance and memory perspective.

So just don't. When people complain that they shouldn't need to punish .d.ts files since they're publishing TypeScript source, just refuse to infer the types of packages without .d.ts files and explain why. The performance problem simply doesn't need to happen.

In other words, tsc doesn't change at all, and the performance problem you fear just doesn't happen. People still need to punish .d.ts files as they do today, and they have the option of publishing either JavaScript or TypeScript sources alongside.