facebook / metro

🚇 The JavaScript bundler for React Native
https://metrobundler.dev
MIT License
5.23k stars 625 forks source link

Follow symlinks? #1

Closed jeanlauliac closed 1 year ago

jeanlauliac commented 7 years ago

Do you want to request a feature or report a bug?

Feature

What is the current behavior?

I'm removing a test from DependencyGraph-test that looked like so, but wasn't actually working after I fixed problems with the fs mocks:

    it('should work with packages with symlinked subdirs', function() {
      var root = '/root';
      setMockFileSystem({
        'symlinkedPackage': {
          'package.json': JSON.stringify({
            name: 'aPackage',
            main: 'main.js',
          }),
          'main.js': 'lol',
          'subdir': {
            'lolynot.js': 'lolynot',
          },
        },
        'root': {
          'index.js': [
            '/**',
            ' * @providesModule index',
            ' */',
            'require("aPackage/subdir/lolynot")',
          ].join('\n'),
          'aPackage': { SYMLINK: '/symlinkedPackage' },
        },
      });

      var dgraph = new DependencyGraph({
        ...defaults,
        roots: [root],
      });
      return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) {
        expect(deps)
          .toEqual([
            {
              id: 'index',
              path: '/root/index.js',
              dependencies: ['aPackage/subdir/lolynot'],
              isAsset: false,
              isJSON: false,
              isPolyfill: false,
              resolution: undefined,
              resolveDependency: undefined,
            },
            {
              id: 'aPackage/subdir/lolynot.js',
              path: '/root/aPackage/subdir/lolynot.js',
              dependencies: [],
              isAsset: false,
              isJSON: false,
              isPolyfill: false,
              resolution: undefined,
              resolveDependency: undefined,
            },
          ]);
      });
    });

What is the expected behavior?

Reintroduce the test and verify symlinks work.

lacker commented 7 years ago

symlinks not working in React Native is one of the biggest RN complaints, so if my understanding is correct and this would fix that, this would be great!

jeanlauliac commented 7 years ago

I think we won't have time to deal with this anytime soon unfortunately. I'd love to reopen once we have a strong case.

ericwooley commented 7 years ago

Can we leave this open until it's resolved?

This is a huge PITA for code sharing, and developing modules. I'm not sure what your looking for in terms of a strong case, but it's definitely a major pain point for me, and module developers.

Currently I am trying to use lerna, and react-primitives to reorganize a project boilerplate to maximize code reuse, while maintaining upgradability by rn.

Lerna works by symlinking and installing your packages in a mono repo, which is an incredibly helpful pattern for code reuse, and completely broken when the react native packager won't follow symlinks.

cpojer commented 7 years ago

Yeah, it should be open. We should find a fix but we are not actively invested in making this change ourselves right now. If the community can find a reasonable solution, we should merge it and make it work.

ericwooley commented 7 years ago

I suspect it has to do with this line according to the docs you may also need to check if it's a symlink. EG

//...
 this._moduleResolver = new ModuleResolver({
      dirExists: filePath => {
        try {
           var stats = fs.lstatSync(filePath);
          return  stats.isDirectory() || stats.isSymbolicLink();
        } catch (e) {}
        return false;
      },
// ...

Though I am short on time for digesting how to get started with this project at the moment. I may have more time over the weekend for a proper PR.

Or if someone else gets the chance to play around with it, that would be awesome.

I never have enough time to accomplish all the things I want to do :(

Edit: added "may", as the docs don't explicitly say you isDirectory() won't return true on a symlink, but I believe that is the case.

Edit^2: https://github.com/ericwooley/symlinkTest You do need to explicitly check on OSX Sierra at least, not sure about linux or windows.

Edit^3: the commit where the test was removed https://github.com/facebook/metro-bundler/commit/c1a81571fca6c5256546cdcb7c6be3ef9bdc0588

haggholm commented 7 years ago

I'd love to reopen once we have a strong case.

With npm 5, all local installs (npm i ../foo) are now symlinked. This makes it pretty rough to work with projects that have local dependencies. (That’s on top of the monorepo scenario.)

jeanlauliac commented 7 years ago

Edit^3: the commit where the test was removed c1a8157

The test was removed because it was already broken for a long time, unfortunately (I forgot to put that in the changeset description apparently). Now I realised I shouldn't have removed it completely, only make verify the broken behavior.

I suspect it has to do with this line

dirExists is only used for debugging, not for resolution. The resolution is done based on the HasteFS instance provided by the jest-haste-map module. I believe, but may well be wrong, that a fix for symlinks will have to be implemented in jest-haste-map, both for its watchman and plain crawlers, as well for its watchman and plain watch modes. Since, last time I've heard (might have changed!), watchman doesn't handle symlink, we were hampered in these efforts.

ericwooley commented 7 years ago
dirExists is only used for debugging, not for resolution. The resolution is done based on the HasteFS instance provided by the jest-haste-map module. I believe, but may well be wrong, that a fix for symlinks will have to be implemented in jest-haste-map, both for its watchman and plain crawlers, as well for its watchman and plain watch modes. Since, last time I've heard (might have changed!), watchman doesn't handle symlink, we were hampered in these efforts.

Interesting.

I am pretty confused by this project, as I am not sure how it is imported by RN. But I manually edited this change into node_modules/react-native/packager/src/node-haste/dependencyGraph.js to use my suggested code and no longer got errors about symlinked modules not being found. I got a ton of other errors, i think related to multiple installs of react-native, but I seemed to have gotten much further in the process.

I also put a console log there to offer some insight, and it was definitely logging. So that line appears to be used by the react-native project while doing real work (as opposed to debugging).

After much frustration with this issue, it ended up being easier to PR the libs i need to work with haul, rather than try to tackle this problem. I wish I had more time for this issue, but it appears to be a pretty rough one to solve, given what @jeanlauliac said.

Hopefully this gets officially resolved soon. as I would much prefer to use the built in solution.

On a side note, haul seems to have fixed all the symlink issues. Is there a downside to haul that you know of? It seems like it could be something that could be officially brought into the fold.

ghost commented 7 years ago

Something that half-works is not symbolic linking but hard linking. You can't hard link directories, but you can use cp to copy all the directories and hard link every file rather than copy it with -al, e.g. cp -al <sourceDirectory> <destinationDirectory>.

The only catch is that it seems in order to trigger rebundling I have to save the changed file (even though the action of saving isn't changing the contents of the file).

ghost commented 7 years ago

@ericwooley Can you explain how your PR, haul, and storybooks (?) relate to the react-native symbolic link issue?

ericwooley commented 7 years ago

@bleighb

Haul is an alternative to the react-native packager that doesn't have issues with symlinks. They reference this issue because those PR's and issues are about issues with symlinking and the metro bundler.

As to your cp -r issue, you might have better luck with rsync. Which is something I considered at one point.

kschzt commented 7 years ago

I'm in the lerna boat, too. The way I finally worked around this issue was to just explicitly add the packages in our monorepo to rn-cli.config.js, instead of having them as dependencies in the RN project's package.json (and symlinked into node_modules)

var config = {
  getProjectRoots() {
    return [
      path.resolve(__dirname), 
      path.resolve(__dirname, '../some-other-package-in-lerna-monorepo')
    ]
  }
}

May have changed for Metro. HTH

ericwooley commented 7 years ago

@kschzt I will have to try that solution tonight.

I imagine that there will be more pressure on this issue once yarn workspaces are released. Until then, your solution may be the only way forward.

haggholm commented 7 years ago

I was unable to get things to work in something like a monorepo when trying @kschzt’s approach. If I have module A as a dependency and point rn-cli.config.js to it explicitly, then it will not be found by packages requiring it from inside the ‘root’ repo. In fact, the packager doesn’t even look! The structure is probably fairly obvious from a log excerpt:

Looking for JS files in
   /mnt/scratch/shared/petter/owl-devenv/client/root/local
   /mnt/scratch/shared/petter/owl-devenv/client/root/local/@client
   /mnt/scratch/shared/petter/owl-devenv/client/root/local/@client/util
   /mnt/scratch/shared/petter/owl-devenv/client/root 

React packager ready.

Loading dependency graph, done.
warning: the transform cache was reset.
error: bundling: UnableToResolveError: Unable to resolve module `@client/util/memoizeOnce` from `/mnt/scratch/shared/petter/owl-devenv/client/root/node_modules/@client/store/bind.js`: Module does not exist in the module map or in these directories:
  /mnt/scratch/shared/petter/owl-devenv/client/root/node_modules/@client/util

Why does it not check to see if the module exists inside the local/ directory? Beats me.

(react-native v0.45.0.)

ericwooley commented 7 years ago

@kschzt @haggholm

How are you importing your files

  1. import whatever from '../../otherPackage/someModules'?
  2. import whatever from 'otherPackage/someModules'?
  3. import whatever from 'someModules'?

I'm not really sure how roots work in this context, but the way your importing may be a factor. Or are you using the @providesModule syntax?

haggholm commented 7 years ago

@ericwooley 2(ish): import whatever from '@foo/bar/baz, in this case, with baz having .js, .android.js, and .ios.js versions for different platforms.

Both the project and I are new to React Native (migrating an existing web app); I’m not familiar with the @providesModule syntax. Perhaps I should look into that and see if it works…

pocesar commented 7 years ago

funny, it used to work (import { something } from 'symlinked-package'), now it doesn't anymore after upgrading react native from 0.43 to 0.45, anything changed in the packager from there?

AshCoolman commented 7 years ago

This lerna project seems to work with Symlinks too (react-native 0.40)

https://github.com/samcorcos/learna-react-native

ericwooley commented 7 years ago

I have played with that too @AshCoolman, and it does seem to work, but setting it up in a similar way with other versions has not worked.

AshCoolman commented 7 years ago

@ericwooley My experience too

ktj commented 7 years ago

Is the only solution at the moment to use haul or some custom shell scripts?

Alazoral commented 7 years ago

We're trying to use yalc to test packages before we release them, but this deficiency means we have to publish, then test, which is pretty crazy.

bakso commented 7 years ago

In China, 90% local project development use cnpm which is build with npminstall, they are using symlinks to save disk volume. We think not supporting symlinks is not a good idea.

joearasin commented 7 years ago

Punting on this issue is making a mess out of the ecosystem. By forcing dependencies on alternative builders for reasonably common setups, we end up stuck on brittle build stacks that are not tested as part of the official release pipeline. Upgrades then become dangerous.

joearasin commented 7 years ago

^ Looks like there's progress being made over in jest on fixing the underlying issue by exposing a parameter to allow passing on watchman and following symlinks. If/when that's resolved, it may be possible to parameterize metro-bundler with something that passes through.

jeanlauliac commented 7 years ago

I would be very happy if someone would like to take over this task and takes the time to investigate and implement the necessary fixes, both in dependencies and in metro-bundler. Let's make sure efforts are not duplicated. We will not have bandwidth to implement this ourselves in the mid future I'm afraid, but I welcome PRs for review.

Titozzz commented 7 years ago

I can't use react-native 0.48 because I have to use haul to fix this issue and haul is not working properly with 0.48 right now... 😢

macrozone commented 7 years ago

I forked a npm package and want to work on it locally by using yarn link. But it's not possible with react-native because of this issue. Is there a workaround for this?

sverrejoh commented 7 years ago

@macrozone

I belive the problem is two-fold: One is that watchman, the tool in charge of monitoring files and triggering a compile whenever a file changes, doesn't work cross symlinks. So a file change within a symlinked folder will never trigger a re-compile.

The other problem is that React Native doesn't seem to follow symlinks outside the project root, but this you can fix by adding another project root through the react native config.

So first npm link like you did before, and then create a file called rn-cli-config.js, and put it in your root react native folder.

var path = require("path");
var config = {
    getProjectRoots() {
        return [
            // Keep your project directory.
            path.resolve(__dirname),

            // Include your forked package as a new root.
            path.resolve(__dirname, "../../../your-forked-package")
        ];
    }
}
module.exports = config;

Use the config together with your normal react-native command. Eg: react-native start --config rn-cli.config.js

I believe this is what made it work for me (you just have to restart the bundler for it to recompile the linked package). Tell me if you have any issues, and good luck!

macrozone commented 7 years ago

thanks @sverrejoh, will try that out

joonhocho commented 7 years ago

@sverrejoh Thanks it worked for me. I am using the following command instead in case your global react-native-cli does not match your project react-native version:

node node_modules/react-native/local-cli/cli.js start --config ../../../../rn-cli-config.js
joearasin commented 7 years ago

@sverrejoh This almost works for me. I'm using yarn workspaces, my config looks like:

const config = {
  getProjectRoots() {
    return [path.resolve(__dirname), path.resolve(__dirname, '..')];
  },
};

react-native start runs fine. Attempting to run react-native link breaks down with "Cannot find module". Is there some way I can set the module search paths?

sverrejoh commented 7 years ago

@joearasin Not sure what your problem is, but it is possible to also add modules explicitly with the extraNodeModules setting:

var path = require("path");
var config = {
    extraNodeModules: {
        react: path.resolve(__dirname, "node_modules/react")
    }
}
module.exports = config;
joearasin commented 7 years ago

@sverrejoh The specific situation I'm dealing with is that I have a monorepo set up (ideally using yarn workspaces) with a react-native app, a react-web app, and a shared library (that needs to be babelified). I'm trying to figure out how to make the shared library (+ dependencies) accessible to the react-native app.

sebirdman commented 7 years ago

I've managed to link two react-native projects together with everything (at least react-native start and live reloading) working.

I have a library with common components, using typescript, that is npm linked to my main application

1) npm install on both items 2) npm link on YOUR_RN_LIBRARY 3) npm link YOUR_RN_LIBRARY on the main project 4) create rn-cli.config.js in the main project:

var path = require("path");
const metroBundler = require('metro-bundler');

var config = {
  extraNodeModules: {
    "react-native": path.resolve(__dirname, "node_modules/react-native"),
  },
  getProjectRoots() {
    return [
      // Keep your project directory.
      path.resolve(__dirname),
      path.resolve(__dirname, "LIBRARY_RELATIVE_PATH"),
    ];
  },
  getBlacklistRE: function() {
    return metroBundler.createBlacklist([
        /USER[/\\]PATH[/\\]TOLIBRARY[/\\]node_modules[/\\]react-native[/\\].*/
    ]);
  }
}
module.exports = config;

It seems like the blacklist must be the absolute path to your library on your file system. also for some reason i have to specify exactly where react-native is with extraNodeModules 😕

finally. npm link is working.

(note that i got this working on RN 0.47.2 - i think bundler changed createBlacklist in 48.X)

alexmngn commented 7 years ago

I've been using the above and for regular stuffs it's been working pretty well. Although I have issues when a dependency of my package is react-native.

As an example, I'm using Lerna to manage a RN project which looks like this:

/packages
  /ui
    /node_modules
    /src
      /Spinner.js
  /main
    /node_modules
    /src
      /index.js
    /rn-cli.config.js

I use import Spinner from 'ui'; in main/src/index.js. Spinner.js uses react-native-spinkit to display a loading indicator. react-native-spinkit tries to load react-native from /ui/node_modules, although because I've black-listed it to avoid the naming collision, it cannot find it. Is there any solution?

Unable to resolve react-native from react-native-spinkit/index.js [...]. Module does not exist in ui/node_modules

sebirdman commented 7 years ago

@alexmngn do you have "extraNodeModules" specified to your main project? (like i have above)

If you're running react-native start from the main folder, it should be exactly the same path that i have.

alexmngn commented 7 years ago

@sebirdman I had it for "react" but not for "react-native". Having them both does the job for my config 👍 Thanks

achuvm commented 6 years ago

@Alazoral, another way you can test release packages is to do a npm pack which generates the .tgz file which is what is uploaded to npm. Then you can npm install ./path/to/modules.tgz and that should work just fine.

If you update, then you have to npm pack and npm install module.tgz again though.

GingerBear commented 6 years ago

based on https://github.com/facebook/metro-bundler/issues/1#issuecomment-333658773, I created a script that automated the workarounds with rn-cli-config.js to make npm link work https://gist.github.com/GingerBear/485f922a1e403739dc56d279925b216d

what it does is basically

so the workflow is, do regular npm link first, then run node ./react-native-start-with-link.js to start bundler with symlink

dariocravero commented 6 years ago

@GingerBear that's gold! Thanks so much for posting that script. I just used it on crna/expo to use external modules too. I'll do a write up before the end of the week on how to go about it there. react-community/create-react-native-app/issues/232

nicolascouvrat commented 6 years ago

@GingerBear Nice one!! Just made a slight modification as I wanted it to launch expo using the packager config you created through your script rather than just the packager: https://gist.github.com/nicolascouvrat/68c546860f8c7b8eaeabda18a9baed6b

dariocravero commented 6 years ago

In case anyone is stuck with this and until the different issues are fixed, I made a little guide on how to use yarn workspaces with Create React App and Create React Native App (Expo) to share common code across. Hope you find it handy! https://learn.viewsdx.com/how-to-use-yarn-workspaces-with-create-react-app-and-create-react-native-app-expo-to-share-common-ea27bc4bad62 https://medium.com/viewsdx/how-to-use-yarn-workspaces-with-create-react-app-and-create-react-native-app-expo-to-share-common-ea27bc4bad62

@GingerBear, thanks for that snippet! I reworked it a bit so it picks up the list of workspaces and published it as a package here https://www.npmjs.com/package/metro-bundler-config-yarn-workspaces

adamconservis commented 6 years ago

@GingerBear I'm having trouble getting this method to pick up dependencies of the sym linked dependency. Any suggestions or is this not possible?

GingerBear commented 6 years ago

@adamconservis what's the trouble you had? Originally I was testing with rn@v0.48.4, and I just created an empty project with rn@v0.50.4, both are working fine.

adamconservis commented 6 years ago

I have a React Native project that is working fine, and I would like to be able to develop my own modules that can be reused between this and our React front end. To test this, I took a dependency from the project and cloned it to my machine. Then, I symlinked the dependency into the main project and ran this script, with one change. I had the react-native execute run-ios instead of start and what happens is the system finds the dependency that was symlinked, but it has dependencies that the project can't find. I've tried 3 different things. Running just as explained. Then I tried running yarn on the dependency so it would grab all of it's dependencies, that didn't work. Then I tried adding the necessary dependencies of the dependency into the main project, and that didn't work.

GingerBear commented 6 years ago

@adamconservis try not using run-ios but use start, run-ios seems ignore the rn-cli-config that specified by ---config

adamconservis commented 6 years ago

@GingerBear using start doesn't run the app though, how do I trigger the build?

adamconservis commented 6 years ago

@GingerBear If I run-ios with the packager that was started by the script, I still run into the same issue. require can't find the child of the dependency.

GingerBear commented 6 years ago

@adamconservis you can try run react-native run-ios to run the app, then exit the packager that started by run-ios, then run the script (which start the packager again with --config ../../../../rn-cli-config-with-links.js), then ues cmd+r to reload the app.