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

feat: opaque symlink support #7549

Closed
wants to merge 4 commits into from
Closed

Conversation

aleclarson
Copy link
Contributor

@aleclarson aleclarson commented Dec 26, 2018

Summary

This is a follow-up to #6993.

Its goals include:

  • crawl any directory that's been linked to by a project root (eg: installed via yarn link)
  • cache all symlinks in jest-haste-map for later consumption
  • add a follow method to the HasteMap class (for lazily resolving symlinks)

More details in the commit messages and code comments.

What's missing

No support for symlinks in watch mode. 😢 But regular files in linked packages are watched.
More details on watch mode here: #7549 (comment)

Test plan

I've fixed most of the tests that were broken by my changes. Please provide guidance on the remaining unfixed tests. Also, we could use some automated tests for symlinks.

I've tested locally on projects using:

  • pnpm install
  • yarn link
  • npm install

For manual testing, you can use this repository:
https://github.com/aleclarson/repro/tree/jest-symlinks

@nicojs
Copy link
Contributor

nicojs commented Jan 17, 2019

@aleclarson you're my personal hero 🥇 ! Any updates on this PR?

@aleclarson
Copy link
Contributor Author

I suspect they won't review this PR until after the TypeScript rewrite, and then I'll need to redo it all. 😅

@fatfatson

This comment has been minimized.

@SimenB SimenB requested a review from mjesun January 28, 2019 07:20
@soundyogi

This comment has been minimized.

@nicojs
Copy link
Contributor

nicojs commented Feb 13, 2019

@mjesun when do you think you have time to review this branch?

@retyui
Copy link

retyui commented Feb 19, 2019

@SimenB ^

@SimenB
Copy link
Member

SimenB commented Feb 19, 2019

This is a thing FB devs have to review as it's for RN/Metro. I'm not sure if it's needed for anything Jest itself does?

@cpojer
Copy link
Member

cpojer commented Feb 20, 2019

I'll get to this soon.

@lukas1994
Copy link

Any updates with this?

@SimenB
Copy link
Member

SimenB commented Feb 4, 2020

Latest in this space is #9351

@phsantiago
Copy link

As @nicojs said, you're my personal hero too @aleclarson!
Please don't give up, you've gone this far so far. I saw this starting here facebook/metro#1 (comment) more than 1 year ago.
How could I help to get this done?

@egor-vorotnikov-m
Copy link

One of the biggest problem of adopting react-native-web and mono repo setup, definitely voting fo this one!

@henrymoulton
Copy link

henrymoulton commented May 27, 2020

Hi @SimenB thanks for all your help so far, I'm trying to understand what we need to do to get symlinks in Metro working nicely.

Does #9351 need to be merged to unblock this PR?

@aleclarson
Copy link
Contributor Author

Rebased onto v24.9.0

Every "node_modules" directory within a project root is searched for linked dependencies.

Each linked dependency can become a project root if its resolved path meets these criteria:
  - not inside a "node_modules" directory
  - not inside an existing project root

Once all linked dependencies are found, collect every file (including symlinks) in each of the project roots.

Two properties are added to the cache of "jest-haste-map":
  "links": Map of link paths (relative to rootDir) to their metadata
  "roots": Array of configured roots and linked roots

One method is added to the "HasteFS" class:
  "follow": Lazily resolves any symlink found by the crawler

⚠️  The JS crawler does *not* yet support symlinks; only the Watchman crawler does.

⚠️  Watch mode does *not* yet support symlinks.
@danvln
Copy link

danvln commented Aug 30, 2020

Probably the longest PR as duration I have seen. @aleclarson - nice job trying to address this!

mc0 added a commit to sharpspring/jest-test-repro that referenced this pull request Dec 10, 2020
This adds jest.bzl and uses it to pass in specific tests.

See jestjs/jest#7549
@ajanuar
Copy link

ajanuar commented Mar 17, 2021

Any update from this PR?

@SimenB
Copy link
Member

SimenB commented Apr 2, 2021

Landed (and released as beta) #9351 which adds support for crawling symlinks (which fixes support for Bazel at least). Is the extra features of this PR needed? If yes, can you rebase so we can re-evaluate?

@aleclarson
Copy link
Contributor Author

Is the extra features of this PR needed?

Metro users can't use linked dependencies without this PR. (facebook/metro#257)

If yes, can you rebase so we can re-evaluate?

I'm hesitant to rebase, because of past concerns here and here.
But I will rebase if you tell me it will be merged. :)

@SimenB
Copy link
Member

SimenB commented Apr 2, 2021

But I will rebase if you tell me it will be merged. :)

I cannot make that promise (or even that it'd be reviewed in a timely manner), unfortunately ☹️

@kopax-polyconseil
Copy link

Is the extra features of this PR needed?

Metro users can't use linked dependencies without this PR. (facebook/metro#257)

I am one of them, any update on this one?

@ishowta
Copy link

ishowta commented Jun 4, 2022

Since a custom fork of jest-haste-map has been added to metro, it may be appropriate to work on there.

https://github.com/facebook/metro/tree/main/packages/metro-file-map

@github-actions
Copy link

github-actions bot commented Jun 4, 2023

This PR is stale because it has been open 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Jun 4, 2023
@aleclarson aleclarson closed this Jun 4, 2023
@github-actions
Copy link

github-actions bot commented Jul 5, 2023

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.