Add support for --preserve-symlinks

[gaearon]

This is a thorny issue but I believe I'm beginning to understand it.

Basically there's two ways to treat symlinks:

• "Expand" their paths during resolution (aka "not preserving symlinks")
• Don't "expand" their paths during resolution (aka "preserving symlinks")

There are downsides to both (if my understanding is right).

Node by default does not preserve symlinks. They tried to change this behavior in Node 6 (nodejs/node#5950) but that backfired with non-trivial breakages and they reverted it soon and put behind a --preserve-symlinks flag (nodejs/node#6537). There is also an argument that the behavior isn't a bug or needs a different fix (https://github.com/isaacs/node6-module-system-change).

Regardless, it seems like some people prefer one behavior (with its set of tradeoffs) and other people prefer the other behavior (with another set of tradeoffs). Node currently supports both. Webpack does too (https://webpack.js.org/configuration/resolve/#resolve-symlinks).

Jest currently seems to support a mixture of both behaviors (?). I fixed at least some of it to match Node in #4761 (ironically despite "preserve" in the PR name the change aligned with Node's default behavior of not preserving them). I still think that PR was a good idea. However, apparently there are places where Jest still doesn't match node: #5228, #5085.

Assuming those get fixed, and Jest always does not preserve symlinks by default (just like Node), what do you think about allowing the "preserve symlinks" behavior as an opt-in, just like Node and webpack already offer?

[SEAPUNK]

Is there a reason Jest ignores the NODE_PRESERVE_SYMLINKS=1 exported envvar? Does it spawn worker processes ignoring the environment variables of the parent process?

Maybe it is, and I'm doing something wrong.

[lennartjansson]

We use Jest at Dropbox and we are also in need of the ability to preserve symlinks when Jest resolves file paths.

All of our automated test runs and frontend asset build steps are performed using Bazel. Notably, Bazel runs build tools and binaries inside sandboxes, temporary directories inside which every file is actually a read-only symlink that points outside of the sandbox. Bazel expects the test binary to treat the symlinks opaquely — as normal files relative to the sandbox root.

This has a couple consequences for Jest:

• When Jest performs test discovery, we need it to discover tests inside files that are actually symlinks. Unfortunately, the filesystem walk (find) used to discover test files will not return symlinks at all.

• Jest uses fs.realpath (or similar) to resolve symlinks in the default resolver and the script transformer — when these are resolved to paths outside the Bazel sandbox (i.e. outside of Jest's current working directory), code coverage reports will contain duplicated paths: one set of paths from inside the sandbox and another from outside.

We've started an internal fork of Jest at Dropbox, in which we've removed the realpath calls and changed the find call to also pick up symlinks — these changes were sufficient for us to get Jest playing nicely with Bazel. However, I assume symlink-preserving behavior would be useful not only to us or anyone hoping to integrate Jest with Bazel, but also anyone using Jest in any other kind of test or build environment in which source files and test files may actually be symlinks.

We'd be happy to contribute these changes back upstream behind a --preserve-symlinks flag (or perhaps two distinct flags if changes for the find call and the realpath calls are considered orthogonal). Are there any other concerns or details we should be aware of or take into account when implementing a PR for this?

[Globegitter]

@lennartjansson Hi Lenart, we at Ecosia are also using Bazel and just started integrating jest, running into the same issue. A PR would be really useful to us and also others it seems, given that the first issue on this topic has been opened already in 2016. Or if you could even just open-source the fork, then others could work around this issue for the time being.

On a different note, if you already have some Bazel jest rules and would be willing to open source it that would also be great, as this would be the next natural step for us also. Otherwise we would also be happy to collaborate on this.

[rodrigoalmeidaee]

In case it helps anyone, we are also waiting for jest to support symlinks out of the box. Until that happens, we wrote a collection of patches (mainly inspired in @lennartjansson's post and in an abandoned PR - https://github.com/facebook/jest/pull/4387/files). We apply these patches using a postinstall hook:

https://gist.github.com/rodrigoalmeidaee/a12d0c1e72f277d8c125c3c086607449

Of course these may break at any time with jest updates, so they aren't robust at all :-/

(Note: There is one extra patch there that is unrelated, it has to do with webpack-dev-server with we also had to patch to deal well with symlinks)

[Globegitter]

I just opened up #7364 which is functionally almost complete to implement this feature. If anyone wants to help out tu push this over the finish line quicker that would be much appreciated.

[lennartjansson]

@Globegitter I would be interested in helping to pick up work on your PR #7364 if you don't expect to be working more on it soon. I was planning eventually to work on a PR myself implementing what I described here but

@SimenB do you have any overall feedback or suggestions for the proposed behavior of the --preserve-symlinks flag? Namely,

• If --preserve-symlinks is on, symlinks that appear to be test files will be captured during the filesystem walk for test discovery. (without the flag, all symlinks will be skipped during test discovery)
• If --preserve-symlinks is on, source file paths that are actually symlinks will not be resolved to their underlying real file paths -- the symlink paths will be used as is. (without the flag, resolvers will continue resolving sourcemaps with fs.realpath)

(@Globegitter if the above doesn't match your intended implementation please correct me / let me know so we can design a behavior that will work for all of us.)

[Globegitter]

@lennartjansson yep the behaviour you described is the intention of #7364 (so it is a bit of an overload of the flag). I should actually have some time for it today, otherwise it will probably quite some time until I would get around to it again.