Move the getAssetData() logic out of the Bundler

Reviewed By: davidaurelio

Differential Revision: D6183424

fbshipit-source-id: 47a658de25ac0943f4b82e7eb90c1018e9d0357d

packages/metro-bundler/src/AssetServer/__tests__/AssetServer-test.js

@@ -6,17 +6,24 @@
  * LICENSE file in the root directory of this source tree. An additional grant
  * of patent rights can be found in the PATENTS file in the same directory.
  *
+ * @emails oncall+javascript_foundation
  * @format
  */
 
 'use strict';
 
 jest.mock('fs');
+jest.mock('image-size');
 
 const AssetServer = require('../');
 const crypto = require('crypto');
 const fs = require('fs');
 
+require('image-size').mockReturnValue({
+  width: 300,
+  height: 200,
+});
+
 const {objectContaining} = jasmine;
 
 describe('AssetServer', () => {
@@ -195,12 +202,15 @@ describe('AssetServer', () => {
         },
       });
 
-      return server.getAssetData('imgs/b.png').then(data => {
+      return server.getAssetData('/root/imgs/b.png').then(data => {
         expect(data).toEqual(
           objectContaining({
+            __packager_asset: true,
             type: 'png',
             name: 'b',
             scales: [1, 2, 4, 4.5],
+            fileSystemLocation: '/root/imgs',
+            httpServerLocation: '/assets/imgs',
             files: [
               '/root/imgs/b@1x.png',
               '/root/imgs/b@2x.png',
@@ -229,12 +239,15 @@ describe('AssetServer', () => {
         },
       });
 
-      return server.getAssetData('imgs/b.jpg').then(data => {
+      return server.getAssetData('/root/imgs/b.jpg').then(data => {
         expect(data).toEqual(
           objectContaining({
+            __packager_asset: true,
             type: 'jpg',
             name: 'b',
             scales: [1, 2, 4, 4.5],
+            fileSystemLocation: '/root/imgs',
+            httpServerLocation: '/assets/imgs',
             files: [
               '/root/imgs/b@1x.jpg',
               '/root/imgs/b@2x.jpg',
@@ -275,18 +288,18 @@ describe('AssetServer', () => {
         }
 
         return server
-          .getAssetData('imgs/b.jpg')
+          .getAssetData('/root/imgs/b.jpg')
           .then(data =>
             expect(data).toEqual(objectContaining({hash: hash.digest('hex')})),
           );
       });
 
       it('changes the hash when the passed-in file watcher emits an `all` event', () => {
-        return server.getAssetData('imgs/b.jpg').then(initialData => {
+        return server.getAssetData('/root/imgs/b.jpg').then(initialData => {
           mockFS.root.imgs['b@4x.jpg'] = 'updated data';
           server.onFileChange('all', '/root/imgs/b@4x.jpg');
           return server
-            .getAssetData('imgs/b.jpg')
+            .getAssetData('/root/imgs/b.jpg')
             .then(data => expect(data.hash).not.toEqual(initialData.hash));
         });
       });

packages/metro-bundler/src/AssetServer/index.js

@@ -17,9 +17,34 @@ const AssetPaths = require('../node-haste/lib/AssetPaths');
 const crypto = require('crypto');
 const denodeify = require('denodeify');
 const fs = require('fs');
+const imageSize = require('image-size');
 const path = require('path');
+const toLocalPath = require('../node-haste/lib/toLocalPath');
 
-import type {AssetData} from '../node-haste/lib/AssetPaths';
+const {isAssetTypeAnImage} = require('../Bundler/util');
+
+import type {AssetPath} from '../node-haste/lib/AssetPaths';
+
+type AssetInfo = {|
+  files: Array<string>,
+  hash: string,
+  name: string,
+  scales: Array<number>,
+  type: string,
+|};
+
+export type AssetData = {|
+  __packager_asset: boolean,
+  fileSystemLocation: string,
+  httpServerLocation: string,
+  width: ?number,
+  height: ?number,
+  scales: Array<number>,
+  files: Array<string>,
+  hash: string,
+  name: string,
+  type: string,
+|};
 
 const stat = denodeify(fs.stat);
 const readDir = denodeify(fs.readdir);
@@ -57,16 +82,10 @@ class AssetServer {
     });
   }
 
-  getAssetData(
+  _getAssetInfo(
     assetPath: string,
     platform: ?string = null,
-  ): Promise<{|
+  ): Promise<AssetInfo> {
-    files: Array<string>,
-    hash: string,
-    name: string,
-    scales: Array<number>,
-    type: string,
-  |}> {
     const nameData = AssetPaths.parse(
       assetPath,
       new Set(platform != null ? [platform] : []),
@@ -97,6 +116,37 @@ class AssetServer {
     });
   }
 
+  async getAssetData(
+    assetPath: string,
+    platform: ?string = null,
+  ): Promise<AssetData> {
+    const localPath = toLocalPath(this._roots, assetPath);
+    var assetUrlPath = path.join('/assets', path.dirname(localPath));
+
+    // On Windows, change backslashes to slashes to get proper URL path from file path.
+    if (path.sep === '\\') {
+      assetUrlPath = assetUrlPath.replace(/\\/g, '/');
+    }
+
+    const isImage = isAssetTypeAnImage(path.extname(assetPath).slice(1));
+    const assetData = await this._getAssetInfo(localPath, platform);
+    const dimensions = isImage ? imageSize(assetData.files[0]) : null;
+    const scale = assetData.scales[0];
+
+    return {
+      __packager_asset: true,
+      fileSystemLocation: path.dirname(assetPath),
+      httpServerLocation: assetUrlPath,
+      width: dimensions ? dimensions.width / scale : undefined,
+      height: dimensions ? dimensions.height / scale : undefined,
+      scales: assetData.scales,
+      files: assetData.files,
+      hash: assetData.hash,
+      name: assetData.name,
+      type: assetData.type,
+    };
+  }
+
   onFileChange(type: string, filePath: string) {
     this._hashes.delete(this._files.get(filePath));
   }
@@ -164,6 +214,7 @@ class AssetServer {
         // important: we want to resolve root + dir
         // to ensure the requested path doesn't traverse beyond root
         const absPath = path.resolve(root, dir);
+
         return stat(absPath).then(
           fstat => {
             // keep asset requests from traversing files
@@ -237,7 +288,7 @@ class AssetServer {
     return map;
   }
 
-  _getAssetDataFromName(platforms: Set<string>, file: string): ?AssetData {
+  _getAssetDataFromName(platforms: Set<string>, file: string): ?AssetPath {
     return AssetPaths.tryParse(file, platforms);
   }
 }

packages/metro-bundler/src/Bundler/__tests__/Bundler-test.js

@@ -228,6 +228,8 @@ describe('Bundler', function() {
 
   describe('.bundle', () => {
     const mockAsset = {
+      __packager_asset: true,
+      fileSystemLocation: '/root/img',
       scales: [1, 2, 3],
       files: [
         '/root/img/img.png',
@@ -235,8 +237,11 @@ describe('Bundler', function() {
         '/root/img/img@3x.png',
       ],
       hash: 'i am a hash',
+      height: 100,
+      httpServerLocation: '/assets/img',
       name: 'img',
       type: 'png',
+      width: 50,
     };
 
     beforeEach(() => {

packages/metro-bundler/src/Bundler/index.js

@@ -22,21 +22,14 @@ const Resolver = require('../Resolver');
 const Bundle = require('./Bundle');
 const HMRBundle = require('./HMRBundle');
 const ModuleTransport = require('../lib/ModuleTransport');
-const imageSize = require('image-size');
 const path = require('path');
-const denodeify = require('denodeify');
 const defaults = require('../defaults');
 const toLocalPath = require('../node-haste/lib/toLocalPath');
 const createModuleIdFactory = require('../lib/createModuleIdFactory');
 
-const {generateAssetTransformResult, isAssetTypeAnImage} = require('./util');
+const {generateAssetTransformResult} = require('./util');
 
-const {
+const {sep: pathSeparator} = require('path');
-  sep: pathSeparator,
-  join: joinPath,
-  dirname: pathDirname,
-  extname,
-} = require('path');
 
 const VERSION = require('../../package.json').version;
 
@@ -89,8 +82,6 @@ export type ExtendedAssetDescriptor = AssetDescriptor & {
   +files: Array<string>,
 };
 
-const sizeOf = denodeify(imageSize);
-
 const {
   createActionStartEntry,
   createActionEndEntry,
@@ -791,41 +782,9 @@ class Bundler {
     assetPlugins: Array<string>,
     platform: ?string = null,
   ) {
-    const localPath = toLocalPath(this._projectRoots, module.path);
-    var assetUrlPath = joinPath('/assets', pathDirname(localPath));
-
-    // On Windows, change backslashes to slashes to get proper URL path from file path.
-    if (pathSeparator === '\\') {
-      assetUrlPath = assetUrlPath.replace(/\\/g, '/');
-    }
-
-    const isImage = isAssetTypeAnImage(extname(module.path).slice(1));
-
     return this._assetServer
-      .getAssetData(localPath, platform)
+      .getAssetData(module.path, platform)
-      .then(assetData => {
+      .then(asset => {
-        return Promise.all([
-          isImage ? sizeOf(assetData.files[0]) : null,
-          assetData,
-        ]);
-      })
-      .then(res => {
-        const dimensions = res[0];
-        const assetData = res[1];
-        const scale = assetData.scales[0];
-        const asset = {
-          __packager_asset: true,
-          fileSystemLocation: pathDirname(module.path),
-          httpServerLocation: assetUrlPath,
-          width: dimensions ? dimensions.width / scale : undefined,
-          height: dimensions ? dimensions.height / scale : undefined,
-          scales: assetData.scales,
-          files: assetData.files,
-          hash: assetData.hash,
-          name: assetData.name,
-          type: assetData.type,
-        };
-
         return this._applyAssetPlugins(assetPlugins, asset);
       })
       .then(asset => {

packages/metro-bundler/src/Server/index.js

@@ -438,7 +438,6 @@ class Server {
       return await getOrderedDependencyPaths(
         this._deltaBundler,
         this._assetServer,
-        this._projectRoots,
         bundleOptions,
       );
     }

packages/metro-bundler/src/lib/__tests__/getOrderedDependencyPaths-test.js

@@ -63,11 +63,11 @@ describe('getOrderedDependencyPaths', () => {
       await getOrderedDependencyPaths(deltaBundler, assetsServer, ['/tmp'], {}),
     ).toEqual([
       '/tmp/1.js',
-      '2.png@2x',
+      '/tmp/2.png@2x',
-      '2.png@3x',
+      '/tmp/2.png@3x',
       '/tmp/3.js',
-      '4.png@2x',
+      '/tmp/4.png@2x',
-      '4.png@3x',
+      '/tmp/4.png@3x',
       '/tmp/5.js',
     ]);
   });

packages/metro-bundler/src/lib/getOrderedDependencyPaths.js

@@ -14,8 +14,6 @@
 
 const Serializers = require('../DeltaBundler/Serializers');
 
-const toLocalPath = require('../node-haste/lib/toLocalPath');
-
 import type AssetsServer from '../AssetServer';
 import type {Options} from '../DeltaBundler/Serializers';
 import type DeltaBundler from '../DeltaBundler';
@@ -23,7 +21,6 @@ import type DeltaBundler from '../DeltaBundler';
 async function getOrderedDependencyPaths(
   deltaBundler: DeltaBundler,
   assetsServer: AssetsServer,
-  projectRoots: $ReadOnlyArray<string>,
   options: Options,
 ): Promise<Array<string>> {
   const modules = await Serializers.getAllModules(deltaBundler, options);
@@ -34,7 +31,7 @@ async function getOrderedDependencyPaths(
         return [module.path];
       } else {
         const assetData = await assetsServer.getAssetData(
-          toLocalPath(projectRoots, module.path),
+          module.path,
           options.platform,
         );
 

packages/metro-bundler/src/node-haste/AssetResolutionCache.js

@@ -15,7 +15,7 @@
 const AssetPaths = require('./lib/AssetPaths');
 const MapWithDefaults = require('./lib/MapWithDefaults');
 
-import type {AssetData} from './lib/AssetPaths';
+import type {AssetPath} from './lib/AssetPaths';
 
 type Options = {|
   /**
@@ -116,7 +116,7 @@ class AssetResolutionCache {
     return results;
   };
 
-  _isValidAsset(assetData: AssetData): boolean {
+  _isValidAsset(assetData: AssetPath): boolean {
     return this._opts.assetExtensions.has(assetData.type);
   }
 }

packages/metro-bundler/src/node-haste/lib/AssetPaths.js

@@ -15,8 +15,7 @@
 const parsePlatformFilePath = require('./parsePlatformFilePath');
 const path = require('path');
 
-export type AssetData = {|
+export type AssetPath = {|
-  // TODO: rename to "assetPath", what it actually is.
   assetName: string,
   name: string,
   platform: ?string,
@@ -47,7 +46,7 @@ function parseBaseName(
  * Return `null` if the `filePath` doesn't have a valid extension, required
  * to describe the type of an asset.
  */
-function tryParse(filePath: string, platforms: Set<string>): ?AssetData {
+function tryParse(filePath: string, platforms: Set<string>): ?AssetPath {
   const result = parsePlatformFilePath(filePath, platforms);
   const {dirPath, baseName, platform, extension} = result;
   if (extension == null) {
@@ -63,7 +62,7 @@ function tryParse(filePath: string, platforms: Set<string>): ?AssetData {
   };
 }
 
-function parse(filePath: string, platforms: Set<string>): AssetData {
+function parse(filePath: string, platforms: Set<string>): AssetPath {
   const result = tryParse(filePath, platforms);
   if (result == null) {
     throw new Error('invalid asset file path: `${filePath}');