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

After upgrade, the assets in other watchedFolders are not resolved correctly #322

Closed
tychota opened this issue Nov 30, 2018 · 10 comments
Closed

Comments

@tychota
Copy link

tychota commented Nov 30, 2018

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

A bug

What is the current behavior?

Images located in other watchedFolders are not resolved properly in react native after upgrade.

Upgrade details:

  • React Native 0.57.1 => 0.57.7
  • metro preset 0.45.3 => 0.49.2

The structure of my files looks like this:

 📂 affiny
 ├ 📂 affiny-app
 │ ├ 📂 assets
 │ │ └ 🖼 splashscreen.png
 │ └ 📂 src
 │   └ 📄 app.specific.stuff.ts
 ├ 📂 affiny-web
 │ ├ 📂 assets
 │ │ └ 🖼 web.specific.image.png
 │ └ 📂 src
 │   └ 📄 web.specific.stuff.ts
 └ 📂 affny-core
   ├ 📂 assets
   │ └ 🖼 not.specific.image.png
   └📂 src
     └ 📄 not.specific.stuff.ts

and I have a rn-cli:

const path = require('path');

module.exports = {
  watchFolders: [
    path.resolve(__dirname, '../affiny-core'),
  ],
};

Before upgrade, i can do:

<Image src={require('affiny-app/assets/splashscreen.png')} />

and

<Image src={require('affiny-core/assets/not.specific.image.png')} />

After upgrade, i can do:

<Image src={require('affiny-app/assets/splashscreen.png')} />

but image in my-project-core, eg:

<Image src={require('affiny-core/assets/not.specific.image.png')} />

are resulting in a gray screen:

image

What is the expected behavior?

Either:

  • no regression, everything working as before.
  • a deprecation documented so I can adapt my code

Investigation

I added a breakpoint here:

image

Before the upgrade, the packager url for assets was:

resolver.defaultAsset().uri;
// -> "http://localhost:8081/assets/assets/img/profile_capture/search.jpg?platform=ios&hash=51dac229328bf5d045e9401cb6cb3cb4"

after the upgrade:

resolver.defaultAsset().uri;
// -> "http://localhost:8081/affiny-core/assets/img/profile_capture/search.jpg?platform=ios&hash=51dac229328bf5d045e9401cb6cb3cb4"

Note above:

  • "affiny-core" instead of "assets"

When asking for http://localhost:8081/affiny-core/assets/img/profile_capture/search.jpg?platform=ios&hash=51dac229328bf5d045e9401cb6cb3cb4, I get a 404.

Note that http://localhost:8081/assets/assets/img/profile_capture/search.jpg?platform=ios&hash=51dac229328bf5d045e9401cb6cb3cb4 works on both old RN/metro and new RN/metro.

Idea

Seems like ea980fd#diff-71293ea379a7cf006cd4648701a7c568 (#299) is the root cause.

Maybe the RN packager is not aware of the public path.

Reproduction steps

I think it is a bit involving to do a repro here. Maybe you will have an idea with above investigation, else I will spend time doing a repro.

Configuration

Node: v8.11.3
React Native: 0.57.7
Metro: 0.49.2

@tychota tychota changed the title After upgrade, the assets in other watched directories are not resolved correctly After upgrade, the assets in other watchedFolders are not resolved correctly Nov 30, 2018
@rafeca
Copy link
Contributor

rafeca commented Nov 30, 2018

Thanks for reporting!

I'm gonna try to reproduce, from what I understand in your example the affiny-core folder is outside of the project root right?

@rafeca
Copy link
Contributor

rafeca commented Nov 30, 2018

Ok, I've been able to reproduce it, the issue was introduced in a711d73 😅

Thanks for the detailed explanation of the issue, it was very helpful to debug it 😃

Very soon we want to force all watch folders to be inside the project root of metro, this way Metro can safely use the project root as the root of the http server that provides the assets and this will fix this issue.

In order to make this change, we'll change the meaning of the projectRoot, which will just be a common folder where all the different watchFolders reside (so it's going to be a breaking change).

I'm gonna check now if there's any short-term workaround to fix this issue in the meantime

@tychota
Copy link
Author

tychota commented Nov 30, 2018

Whaa so quick.

Thanks for the detailed explanation of the issue, it was very helpful to debug it 😃

You are welcome ! Was a pleasure :) Quite a funny bug to investigate !

Very soon we want to force all watch folders to be inside the project root of metro, this way Metro can safely use the project root as the root of the http server that provides the assets and this will fix this issue.

Do you mean that with my folder structure, in the future the project root will be the parent, i.e., in my example "affiny" ?
Looks great, interested to see how RN will know which is the main directory !

I'm gonna check now if there's any short-term workaround to fix this issue in the meantime.

Thank you man ! I was thinking of patch-packaging react-native with a

function resolveAssetSource(source: any): ?ResolvedAssetSource {
  if (typeof source === 'object') {
    return source;
  }

  const asset = AssetRegistry.getAssetByID(source);
+ asset.httpServerLocation = asset.httpServerLocation.replace("affiny-core", "assets");
  if (!asset) {
    return null;
  }

  const resolver = new AssetSourceResolver(
    getDevServerURL(),
    getScriptURL(),
    asset,
  );
  if (_customSourceTransformer) {
    return _customSourceTransformer(resolver);
  }
  return resolver.defaultAsset();
}

or something uggy like this. Happy to have something less ugly ;)

@rafeca
Copy link
Contributor

rafeca commented Nov 30, 2018

Do you mean that with my folder structure, in the future the project root will be the parent, i.e., in my example "affiny" ?
Looks great, interested to see how RN will know which is the main directory !

Exactly! yeah in order to do that change we have to figure out what changes are going to be needed in the RN config side.

I'm preparing a fix for the issue, it's not going to be the most beautiful thing but will do the trick for now

@tychota
Copy link
Author

tychota commented Nov 30, 2018

I'm preparing a fix for the issue, it's not going to be the most beautiful thing but will do the trick for now.

😍

@tychota
Copy link
Author

tychota commented Nov 30, 2018

Well done sir.

@tychota
Copy link
Author

tychota commented Dec 4, 2018

For record, I tried hard to apply the fix in my project:

  • to the version of metro packaged in RN (0.48.3): the changes introduced in 0.49.0 and 0.50 (HMR, delta client) made it hard for the patch to apply
  • bumping metro in Rn to 0.50 with yarn dependency resolution and apply patch: does work ok on iOS, but on crash on android, due to change of delta client missing in java

I finally did this hack stuff:

diff --git a/node_modules/react-native/Libraries/Image/resolveAssetSource.js b/node_modules/react-native/Libraries/Image/resolveAssetSource.js
index 9c811e5..931953d 100644
--- a/node_modules/react-native/Libraries/Image/resolveAssetSource.js
+++ b/node_modules/react-native/Libraries/Image/resolveAssetSource.js
@@ -95,6 +95,11 @@ function resolveAssetSource(source: any): ?ResolvedAssetSource {
     return null;
   }
 
+  if (!asset.httpServerLocation.startsWith('/assets')) {
+    const parsedUrl = /\/[\w\-]+(\/.*)/.exec(asset.httpServerLocation);
+    asset.httpServerLocation = parsedUrl[1];
+  }
+
   const resolver = new AssetSourceResolver(
     getDevServerURL(),
     getScriptURL(),

using patch package.

We aim to remove this in one two month, when migrating to RN 0.58 !

@tychota
Copy link
Author

tychota commented Dec 5, 2018

Edit the above "hack does work in dev but not when assets are bundled.

I will move my assets from the centralised directory to the main directory for now, even if that means duplication.

@cmcewen
Copy link
Contributor

cmcewen commented Dec 7, 2018

Just wanted to chime in here and say I am also having this issue, and trying to apply the fix was unsuccessful

@Kudo
Copy link

Kudo commented Dec 10, 2018

In my case, to the watchFolder resides out of projectRoot.
Even the metro server accepts the request like.
http://localhost:8081/assets/../../../common/assets/images/image.png

The RN Android image system uses okhttp to send requests.
okhttp seems to prune the request as
http://localhost:8081/common/assets/images/image.png
Let's why 404 happens.

My folder structure is pretty much like @tychota.
Environment: RN 0.57.7, metro 0.48.3

Well, there is a dirty workaround for me.
To create a symbolic link in current project root to the shared folder.
ln -s ../../../common # In my project root
This seems make metro able to find files even without 0eaa741 patch.

jakex7 added a commit to software-mansion/react-native-svg that referenced this issue Oct 25, 2024
# Summary

After changing project structure, local images stopped displaying on
Android. This is due to the fact that React Native on Android uses
okhttp for requests, which modifies the endpoint "by cleaning up the
wrong part" from
`http://10.0.2.2:8081/../common/example/assets/image.jpg`
to
`http://10.0.2.2:8081/common/example/assets/image.jpg`

A quick fix for this is to create a symlink to the common directory,
allowing images to be resolved without the `../` segment.

Special thanks to Kudo for sharing the original workaround here:
facebook/metro#322 (comment).
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

4 participants