Fix issue with assets living outside of the project root

Summary:
When we removed the magic logic to find assets across `watchFolders` in a711d73 (which was unsound), an issue was introduced that caused assets outside of the projectRoot to not be found by the development server.

This diff adds a temporary fix to the issue, the final solution will be done once we force all `watchFolders` to be relative to the `projectRoot`.

This fixes #322

Reviewed By: mjesun

Differential Revision: D13277398

fbshipit-source-id: 27a1adf2bd2191a7506b97691773796ba00d48f2

packages/metro/src/Assets.js

@@ -179,7 +179,12 @@ async function getAssetData(
   platform: ?string = null,
   publicPath: string,
 ): Promise<AssetData> {
-  let assetUrlPath = path.join(publicPath, path.dirname(localPath));
+  // 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.
   if (path.sep === '\\') {
@@ -247,6 +252,7 @@ async function getAssetFiles(
 async function getAsset(
   relativePath: string,
   projectRoot: string,
+  watchFolders: $ReadOnlyArray<string>,
   platform: ?string = null,
 ): Promise<Buffer> {
   const assetData = AssetPaths.parse(
@@ -256,9 +262,9 @@ async function getAsset(
 
   const absolutePath = path.resolve(projectRoot, relativePath);
 
-  if (!absolutePath.startsWith(path.resolve(projectRoot))) {
+  if (!pathBelongsToRoots(absolutePath, [projectRoot, ...watchFolders])) {
     throw new Error(
-      `'${relativePath}' could not be found, because it cannot be found in the project root: ${projectRoot})`,
+      `'${relativePath}' could not be found, because it cannot be found in the project root or any watch folder`,
     );
   }
 
@@ -273,6 +279,19 @@ async function getAsset(
   return readFile(record.files[record.files.length - 1]);
 }
 
+function pathBelongsToRoots(
+  pathToCheck: string,
+  roots: $ReadOnlyArray<string>,
+): boolean {
+  for (const rootFolder of roots) {
+    if (pathToCheck.startsWith(path.resolve(rootFolder))) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 module.exports = {
   getAsset,
   getAssetData,

packages/metro/src/Server.js

@@ -348,6 +348,7 @@ class Server {
       const data = await getAsset(
         assetPath[1],
         this._config.projectRoot,
+        this._config.watchFolders,
         /* $FlowFixMe: query may be empty for invalid URLs */
         urlObj.query.platform,
       );

packages/metro/src/Server/__tests__/Server-test.js

@@ -625,7 +625,12 @@ describe('processRequest', () => {
 
       server.processRequest(req, res);
       res.end.mockImplementation(value => {
-        expect(getAsset).toBeCalledWith('imgs/a.png', '/root', 'ios');
+        expect(getAsset).toBeCalledWith(
+          'imgs/a.png',
+          '/root',
+          ['/root'],
+          'ios',
+        );
         expect(value).toBe('i am image');
         done();
       });
@@ -643,7 +648,12 @@ describe('processRequest', () => {
 
       server.processRequest(req, res);
       res.end.mockImplementation(value => {
-        expect(getAsset).toBeCalledWith('imgs/a.png', '/root', 'ios');
+        expect(getAsset).toBeCalledWith(
+          'imgs/a.png',
+          '/root',
+          ['/root'],
+          'ios',
+        );
         expect(value).toBe(mockData.slice(0, 4));
         done();
       });
@@ -662,6 +672,7 @@ describe('processRequest', () => {
         expect(getAsset).toBeCalledWith(
           'imgs/\u{4E3B}\u{9875}/logo.png',
           '/root',
+          ['/root'],
           undefined,
         );
         expect(value).toBe('i am image');

packages/metro/src/__tests__/Assets-test.js

@@ -37,8 +37,8 @@ describe('getAsset', () => {
     writeImages({'b.png': 'b image', '[email protected]': 'b2 image'});
 
     return Promise.all([
-      getAssetStr('imgs/b.png', '/root'),
-      getAssetStr('imgs/[email protected]', '/root'),
+      getAssetStr('imgs/b.png', '/root', []),
+      getAssetStr('imgs/[email protected]', '/root', []),
     ]).then(resp => resp.forEach(data => expect(data).toBe('b image')));
   });
 
@@ -52,11 +52,11 @@ describe('getAsset', () => {
 
     expect(
       await Promise.all([
-        getAssetStr('imgs/b.png', '/root', 'ios'),
-        getAssetStr('imgs/b.png', '/root', 'android'),
-        getAssetStr('imgs/c.png', '/root', 'android'),
-        getAssetStr('imgs/c.png', '/root', 'ios'),
-        getAssetStr('imgs/c.png', '/root'),
+        getAssetStr('imgs/b.png', '/root', [], 'ios'),
+        getAssetStr('imgs/b.png', '/root', [], 'android'),
+        getAssetStr('imgs/c.png', '/root', [], 'android'),
+        getAssetStr('imgs/c.png', '/root', [], 'ios'),
+        getAssetStr('imgs/c.png', '/root', []),
       ]),
     ).toEqual([
       'b ios image',
@@ -74,8 +74,8 @@ describe('getAsset', () => {
     });
 
     return Promise.all([
-      getAssetStr('imgs/b.jpg', '/root'),
-      getAssetStr('imgs/b.png', '/root'),
+      getAssetStr('imgs/b.jpg', '/root', []),
+      getAssetStr('imgs/b.png', '/root', []),
     ]).then(data => expect(data).toEqual(['jpeg image', 'png image']));
   });
 
@@ -87,7 +87,7 @@ describe('getAsset', () => {
       '[email protected]': 'b4.5 image',
     });
 
-    expect(await getAssetStr('imgs/[email protected]', '/root')).toBe('b4 image');
+    expect(await getAssetStr('imgs/[email protected]', '/root', [])).toBe('b4 image');
   });
 
   it('should pick the bigger one with platform ext', async () => {
@@ -104,11 +104,35 @@ describe('getAsset', () => {
 
     expect(
       await Promise.all([
-        getAssetStr('imgs/[email protected]', '/root'),
-        getAssetStr('imgs/[email protected]', '/root', 'ios'),
+        getAssetStr('imgs/[email protected]', '/root', []),
+        getAssetStr('imgs/[email protected]', '/root', [], 'ios'),
       ]),
     ).toEqual(['b4 image', 'b4 ios image']);
   });
+
+  it('should find an image located on a watchFolder', async () => {
+    mkdirp.sync('/anotherfolder');
+
+    writeImages({
+      '../../anotherfolder/b.png': 'b image',
+    });
+
+    expect(
+      await getAssetStr('../anotherfolder/b.png', '/root', ['/anotherfolder']),
+    ).toBe('b image');
+  });
+
+  it('should throw an error if an image is not located on any watchFolder', async () => {
+    mkdirp.sync('/anotherfolder');
+
+    writeImages({
+      '../../anotherfolder/b.png': 'b image',
+    });
+
+    await expect(
+      getAssetStr('../anotherfolder/b.png', '/root', []),
+    ).rejects.toBeInstanceOf(Error);
+  });
 });
 
 describe('getAssetData', () => {