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

Asset paths (httpServerLocation) invalid when asset is in watchFolders #290

Open
treyp opened this issue Oct 10, 2018 · 25 comments
Open

Asset paths (httpServerLocation) invalid when asset is in watchFolders #290

treyp opened this issue Oct 10, 2018 · 25 comments

Comments

@treyp
Copy link

treyp commented Oct 10, 2018

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

What is the current behavior?
When I specify a directory in watchFolders from outside of my project root, the path reported via httpServerLocation is broken. For example, /symlinked-watch-folder/subdir/[email protected].

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.

  1. Create a React Native project in one directory.
  2. Create a sibling directory called symlinked-watch-folder with a Node module in it named symlinked-module.
  3. Add assets to this module, specifically one in subdir/[email protected].
  4. In this module, reference the image like so: <Image source={require('../subdir/image.png')} />
  5. Now from the RN project directory, use npm to install the module: npm install ../symlinked-watch-folder.
  6. Make a component in your project root that displays the module's component with the image in it.
  7. Run the React Native project. The image will not appear and a request for the image will 404 because the path is invalid.

What is the expected behavior?
It looks like there are different valid locations, including:
/assets/symlinked-module/subdir/[email protected]
/assets/subdir/[email protected]
/subdir/[email protected]

More info
Because this repo isn't very publicly documented, I can't tell exactly what's going on. But it looks like getAssetData is trying to path.join these two paths:
/assets
../symlinked-watch-folder/subdir/[email protected]

let assetUrlPath = path.join('/assets', path.dirname(localPath));

The ../ wipes out the /assets and we end up with a result that isn't valid:
/symlinked-watch-folder/subdir/[email protected]

I believe the relative path is coming from here:

path.relative(options.projectRoot, module.path),

The asset is being served correctly, it's just that the httpServerLocation that's being generated is not correct.
I can access the asset just fine if I correct the path to one of these:
/assets/symlinked-module/subdir/[email protected]
/assets/subdir/[email protected]
/subdir/[email protected]

If there is some way to handle this using the documented options, please let me know. (cc: @rafeca )

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.
I've tried both Metro 0.47.1 and Metro 0.48.0
Node: 10.11.0
npm: 6.4.1
OS: macOS High Sierra

@adrianha
Copy link

adrianha commented Oct 11, 2018

Hi,
I also encountered this issue when using symlink to resolve asset.
Im using metro-with-symlinks to resolve symlink packages.

Here's the repo to reproduce the issue:
https://github.com/adrianha/metro-symlink - Works fine
react-native: 0.57.1

https://github.com/adrianha/metro-symlink/tree/0.57.2 - Assets path incorrect
react-native: 0.57.2

@treyp
Copy link
Author

treyp commented Oct 11, 2018

Thanks @adrianha, moving back to RN 0.57.1 (with metro 0.45.6) fixed this for me. Must be a regression in metro 46 or 47, and/or RN 0.57.2.

@ivan-jorge001
Copy link

@treyp how did you fix the issue can you share your rn-cli.config.js, i have not been able to get it working

@treyp
Copy link
Author

treyp commented Oct 11, 2018

@ivan-jorge001 I generate my rn-cli.config.js using metro-with-symlinks. Here's a simplified version:

const path = require('path');

const extraNodeModules = {
  'react': path.resolve(__dirname, 'node_modules/react'),
  'react-native': path.resolve(__dirname, 'node_modules/react-native')
};
const blacklistRegexes = [
  /[/\\]path[/\\]to[/\\]code[/\\]rn-component-one[/\\]node_modules[/\\]react-native[/\\].*/,
  /[/\\]path[/\\]to[/\\]code[/\\]rn-component-two[/\\]node_modules[/\\]react-native[/\\].*/,
  /[/\\]path[/\\]to[/\\]code[/\\]rn-component-three[/\\]node_modules[/\\]react-native[/\\].*/
];
const watchFolders = [
  path.resolve('/path/to/code/rn-component-one'),
  path.resolve('/path/to/code/rn-component-two'),
  path.resolve('/path/to/code/rn-component-three')
];

module.exports = {
  resolver: {
    extraNodeModules,
    blacklistRE: require('metro-config/src/defaults/blacklist')(blacklistRegexes)
  },
  watchFolders
};

My directory structure is:

- path/to/code
  - rn-component-one
  - rn-component-two
  - rn-component-three
  - iOS repo
  - Android repo

@lbonanni
Copy link

lbonanni commented Mar 8, 2019

I'm facing the same problem. I can't display local images if they are on my watchFolders.
I'm not familiar with metro but when I bundle my app it works, so I guess there is a problem in my metro config...

Here is my project stucture :

- native
- web
- shared (here are my assets)

And here is my metro config, present in the native folder :

module.exports = {
  getTransformModulePath() {
    return require.resolve("react-native-typescript-transformer");
  },
  getSourceExts() {
    return ["ts", "tsx"];
  },
  resolver: {
    extraNodeModules: {
      react: path.resolve(__dirname, "node_modules/react"),
      "react-native": path.resolve(__dirname, "node_modules/react-native")
    }
  },
  projectRoot: path.resolve(__dirname),
  watchFolders: [path.resolve(__dirname, "../shared")]
};

Do you see where is my problem here ? 😕
(I can import shared components, it only failed with assets)

React-Native : 0.57.4
Metro : 0.48.5

@dapicester
Copy link

I bumped into this problem too! Currently as a workaround I did what suggested in #322.

Also, for Androd I need the patch only in dev mode (using metro server) while for iOS I have to do the opposite (when I build the bundle). 😕

React-Native: 0.59.0
Metro: 0.51.1

@brentvatne
Copy link
Contributor

brentvatne commented Aug 30, 2019

I created a simple repro for this using yarn workspaces: https://github.com/brentvatne/metro-issue-290

we're running into this in the expo monorepo, only solution for us for now is to use nohoist :\

perhaps the path should be given a uri param? eg: http://localhost:1234/assets?path=../lol/whatever.png&platform=android&so-on&so-forth

@treyp
Copy link
Author

treyp commented Sep 5, 2019

Yep, I think changing the asset path to be given as a query param is the only way forward here.

Brief recap for others:

  • In dev mode, assets are requested from the Metro server using a path sent to the server, so something like http://localhost:8081/assets/images/foo.png
  • When watchFolders is used in Metro config, as is used in multi-repo or mono-repo projects, relative paths are used for assets (e.g. ../../../common/assets/images/image.png) These may be way outside of the root.
  • In iOS, the code sending the request off sends the path exactly as it gets it. In this example, that would be http://localhost:8081/assets/../../../common/assets/images/image.png. The Metro server handles it fine.
  • In Android, which uses okhttp to send requests, relative paths are stripped (per the URI spec section 5.2.4) before the request is made. That means the request ends up being http://localhost:8081/images/image.png which is invalid.

Unless there's some way to configure okhttp to stop removing dot segments, the only way forward (and the more sane approach) would be to send asset paths off as query params.

More info was posted in #322 (comment) and a fix was supposed to be in 0eaa741 but looks like that didn't fully solve it. Unfortunately, the author @rafeca left Facebook earlier this year, so probably won't get much more attention from him.

@vitramir
Copy link

vitramir commented Oct 18, 2019

I got this issue while using yarn workspaces and here is how I solved it:

1)Our first issue is resources destination path outside of /assets directory. It breaks both debug server and bundle.

There is config transformer.publicPath which default value is /assets. So, I changed it to /assets/dir1/dir2/dir3 (because my watchDir is '../../../'). Now it will generate valid destination path for resources outside of projectRoot. For example /assets/dir1/dir2/dir3/../../common/images/image.png. react-native bundle will work too.

2)The second issue with Metro Server. It should resolve requests for
IOS (http://localhost:8081/assets/dir1/dir2/dir3/../../common/images/image.png) and
Android (http://localhost:8081/assets/dir1/common/images/image.png) the same way.

I changed server.enhanceMiddleware config this way:

server: {
    enhanceMiddleware: (middleware, server) => {
      return (req, res, next) => {
        if(req.url.startsWith("/assets/dir1/dir2/dir3")){
          req.url = req.url.replace('/assets/dir1/dir2/dir3', '/assets');
        }else if(req.url.startsWith("/assets/dir1/dir2")){
          req.url = req.url.replace('/assets/dir1/dir2', '/assets/..');
        }else if(req.url.startsWith("/assets/dir1")){
          req.url = req.url.replace('/assets/dir1', '/assets/../..');
        }else if(req.url.startsWith("/assets")){
          req.url = req.url.replace('/assets', '/assets/../../..');
        }
        return middleware(req, res, next);
      };
    },
  },

I want to notice here two more things:

1)This code doesn't use config value transformer.publicPath. I think it is wrong.

const assetPath =
urlObj && urlObj.pathname && urlObj.pathname.match(/^\/assets\/(.+)$/);

2)And another solution of this issue is to update code of assetUrlPath generation and replace ../ into something like __/ or _.._/.

For example, this line

let assetUrlPath = localPath.startsWith('..')
? publicPath.replace(/\/$/, '') + '/' + path.dirname(localPath)
: path.join(publicPath, path.dirname(localPath));

may become:

let assetUrlPath =path.join(publicPath, path.dirname(localPath).replace(/..\//g, '__/'));

Also we should add reverse operation at Server module.
I can make PR with this changes if community approve the idea.


I am currently using this config and it works with yarn workspaces as expected:

module.exports = {
  watchFolders: [path.resolve(__dirname, '../../../')],
  server: {
    enhanceMiddleware: (middleware, server) => {
      return (req, res, next) => {
        if(req.url.startsWith("/assets/dir1/dir2/dir3")){
          req.url = req.url.replace('/assets/dir1/dir2/dir3', '/assets');
        }else if(req.url.startsWith("/assets/dir1/dir2")){
          req.url = req.url.replace('/assets/dir1/dir2', '/assets/..');
        }else if(req.url.startsWith("/assets/dir1")){
          req.url = req.url.replace('/assets/dir1', '/assets/../..');
        }else if(req.url.startsWith("/assets")){
          req.url = req.url.replace('/assets', '/assets/../../..');
        }
        return middleware(req, res, next);
      };
    },
  },
  transformer: {
    publicPath: '/assets/dir1/dir2/dir3',
    babelTransformerPath: path.resolve(
      __dirname,
      './metro-transformer/index.js'
    ),
    getTransformOptions: async () => ({
      transform: {
        experimentalImportSupport: false,
        inlineRequires: false,
      },
    }),
  },
};

I don't provide babelTransformer. Nothing interesting there.

@papigers
Copy link

I'm having the same issue: iOS not showing watchFolder images only on release build - debug build works fine.

@karlvr
Copy link

karlvr commented Jun 9, 2020

@papigers I have found that react-native bundle ... emits httpServerLocation values in the .bundle with /assets/../blah/..., which of course doesn't work, as the assets are actually output as /assets/_blah/.... So someone is changing ../ to _.

I've got it working by making a small patch to Assets.js in metro:

Original:

    // If the path of the asset is outside of the projectRoot, we don't want to
    // use `path.join` since this will generate an incorrect URL path. In that
    // case we just concatenate the publicPath with the relative path.
    let assetUrlPath = localPath.startsWith("..")
      ? publicPath.replace(/\/$/, "") + "/" + path.dirname(localPath)
      : path.join(publicPath, path.dirname(localPath)); // On Windows, change backslashes to slashes to get proper URL path from file path.

New:

    // If the path of the asset is outside of the projectRoot, we don't want to
    // use `path.join` since this will generate an incorrect URL path. In that
    // case we just concatenate the publicPath with the relative path.
    let assetUrlPath = localPath.startsWith("..")
      ? publicPath.replace(/\/$/, "") + "/" + path.dirname(localPath).replace(/^\.\.\//, '_')
      : path.join(publicPath, path.dirname(localPath)); // On Windows, change backslashes to slashes to get proper URL path from file path.

That's fixed it for me... is this something about the treatment of out-of-root paths that could be standardised?

@karlvr
Copy link

karlvr commented Jun 9, 2020

@papigers Actually, a nicer solution is to use an asset plugin.

I created metro-asset-plugin.js:

module.exports = function(assetData) {
	if (assetData.httpServerLocation.indexOf('../') !== -1) {
		assetData.httpServerLocation = assetData.httpServerLocation.replace(/\.\.\//, '_')
	}
	return assetData
}

Then in metro.config.js I include it thus:

...
transformer: {
    assetPlugins: [require.resolve('./metro-asset-plugin')],
},

@vitramir
Copy link

@karlvr I think there are at least two issues with this solution.

  1. What about Metro Server? I think it can’t resolve paths like http://localhost:8081/assets/_/_/_/common/images/image.png without additional changes of the server code.

  2. What if you have _ directory in your assets? It may be in node_modules, so, you can’t control it. You can overwrite files from _ directory by files from .. directory.

You can check my note about it

@karlvr
Copy link

karlvr commented Jun 10, 2020

@vitramir thanks for your comment. My workflow has only just changed to include the asset plugin solution above, but so far it has worked for release builds of iOS and Android versions (ie. with the javascript bundled) and for debug builds of iOS and Android versions running in simulators (with Metro Bundler as a server). So at the moment I'm satisfied for my usage.

Perhaps the asset plugin could be improve to collapse multiple ../s? I'm honestly not sure where the _ in _common comes from in the bundle, I just know that it works :-D

Oh, one other thing I had to do. Sorry, this is important:

I had to make a symlink named _common to ../common in the root directory of my project.

That way the Metro Bundler server could find my assets.

I'm not sure about other possible collisions, but agree they could be a problem!

@vitramir
Copy link

Oh, one other thing I had to do. Sorry, this is important:

I had to make a symlink named _common to ../common in the root directory of my project.

That way the Metro Bundler server could find my assets.

Cool, now I understand how you made it work. If you have single directory with assets outside of your project’s root, then it is pretty simple solution.

Do you have any idea how to make the solution more general and remove symlink?

@karlvr
Copy link

karlvr commented Jun 11, 2020

@vitramir sorry no I don't. I feel like this whole symlinking area is so off-label as far as Metro Bundler and jest haste or whatever it is that I'm just pleased when it works and isn't too complex!

@Ashoat
Copy link

Ashoat commented Jul 13, 2020

If you have a monorepo setup, and are experiencing issues resolving assets in node_modules on iOS release builds after updating to React Native 0.62, it is likely that you are experiencing react-native-community/upgrade-support#26. The easiest solution is to patch react-native with facebook/react-native@7deeec7.

@xanderdeseyn
Copy link

Thanks @Ashoat , implementing that patch with patch-package worked perfectly for me.

@mmazzarolo
Copy link

mmazzarolo commented Sep 12, 2021

FYI, I abstracted @vitramir fix in a module that I'm using in a monorepo: get-metro-android-assets-resolution-fix.js.

You can use it this way:

const {
  getMetroAndroidAssetsResolutionFix,
} = require("@rnup/build-tools");

const androidAssetsResolutionFix = getMetroAndroidAssetsResolutionFix({
  depth: 3,
});

module.exports = {
  transformer: {
    // Apply the Android assets resolution fix to the public path...
    publicPath: androidAssetsResolutionFix.publicPath,
  },
  server: {
    // ...and to the server middleware.
    enhanceMiddleware: (middleware) => {
      return androidAssetsResolutionFix.applyMiddleware(middleware);
    },
  }
};

Wondering if we could just default to a huge depth (e.g. 20) without drawbacks.

Edit: I also published it in react-native-monorepo-tools.

@warrenmcquinn
Copy link

Thank you @mmazzarolo !!!

@mwmcode
Copy link

mwmcode commented Apr 26, 2022

Thanks @vitramir & @mmazzarolo 🙏

FYI @mmazzarolo the link in your answer results in 404, it needs to be updated to this :)

@Skyfiir78
Copy link

Thanks @mmazzarolo !!! You saved my day man :)

@bfricka
Copy link

bfricka commented May 3, 2024

@mmazzarolo did great work on this, but their repo hasn't been updated in a couple of years, and I noticed that enhanceMiddleware is now deprecated. However, there is now a rewriteRequestUrl field in the metro resolver config, so if enhanceMiddleware is removed, you can use that (I rewrote it to use that, and it's a bit cleaner).

@Aure77
Copy link

Aure77 commented May 17, 2024

There is metro-config from rnx-kit that provide metro config setup for monorepo (+ plugins):
https://microsoft.github.io/rnx-kit/docs/tools/metro-config

@pixelknitter
Copy link

@mmazzarolo did great work on this, but their repo hasn't been updated in a couple of years, and I noticed that enhanceMiddleware is now deprecated. However, there is now a rewriteRequestUrl field in the metro resolver config, so if enhanceMiddleware is removed, you can use that (I rewrote it to use that, and it's a bit cleaner).

Do you have an abstract solution to share for the rewrite?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests