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

[discuss] cli,doc: doc deprecate --preserve-symlinks flags #41540

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bmeck
Copy link
Member

@bmeck bmeck commented Jan 15, 2022

These flags are horribly unmaintained, and a decidedly few number of places use them (even package managers that use symlinks like pnpm and yarn don't by default and their issue trackers have hints of it being broken to users). The burden of dealing with their edge cases/design concerns isn't being kept up with even with effort to do so and testing of these flags is not existent.

I think we should at least doc deprecate them due to lack of support from the maintenance side of things unless something changes in order to allow maintainers to focus on features that are used/actively maintained when designing features.

I am not proposing we remove them at this time or anytime in the future, but we don't really test that these things work in any reliable way since they tend to break things like CITGM nor do we have real world examples of heavy usage from which to gather CI tests. We equally haven't found some person actively keeping this feature itself maintained outside of efforts to ensure they keep working to the best of our knowledge.

In addition, we likely should audit these to see security concerns and write down if they are affected by various CWE concerns with symlinks.

CC: @nodejs/modules

@bmeck bmeck added doc Issues and PRs related to the documentations. module Issues and PRs related to the module subsystem. discuss Issues opened for discussions and feedbacks. cli Issues and PRs related to the Node.js command line interface. deprecations Issues and PRs related to deprecations. labels Jan 15, 2022
@ljharb
Copy link
Member

ljharb commented Jan 15, 2022

We equally haven't found some person actively keeping this feature itself maintained

resolve v1 preserves symlinks by default, and both v1 and v2 have an option to control the behavior, and every test case it has (or will have, when it gets "exports" support will) pass for both preserving and following symlinks.

It's not node core, but it's a data point.

@GeoffreyBooth
Copy link
Member

Why not go further and actually deprecate or even remove them in a semver major? We’re not too far off from v18. (I would suggest doing that in its own PR, so that we can ship the docs deprecation immediately.)

@andreialecu
Copy link

andreialecu commented Jan 20, 2022

We're making heavy use of this to be able to use portals in Yarn 2+. portals are similar to npm link which also requires --preserve-symlinks afaik (unless it changed recently).

I don't think there's any way around it when using monorepos with the node_modules linker. Our use case is being able to use a shared package from within a different package in the monorepo.

Additionally, the Typescript compiler itself and Angular also have specific flags to preserve symlinks, which work in conjunction with nodejs' built in support for this flag.

See:
https://www.typescriptlang.org/tsconfig#preserveSymlinks
https://angular.io/cli/build#options (--preserveSymlinks)

As for datapoints, see:
https://stackoverflow.com/questions/58260202/preserve-symlinks-in-angular-libraries (17k views)

This flag is generally in active use and if NodeJS removed it, it would probably stop working:
https://github.com/angular/angular-cli/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+preserve-symlinks (47 issues mention it in one way or another)

In my experience, the flag becomes mandatory with certain monorepo setups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Issues and PRs related to the Node.js command line interface. deprecations Issues and PRs related to deprecations. discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants