From b7d76074a06b9742387543df266fc5302a939eba Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Fri, 4 Sep 2015 15:21:58 -0700 Subject: [PATCH] [react-packager] Support platform extensions in image requires Summary: We don't currently support platform extensions in asset modules. This adds supports for it: ``` require('./a.png'); ``` Will require 'a.ios.png' if it exists and 'a.png' if it doesn't. --- .../AssetServer/__tests__/AssetServer-test.js | 69 ++++++++++ react-packager/src/AssetServer/index.js | 39 ++++-- .../__tests__/DependencyGraph-test.js | 85 ++++++++++++ .../DependencyGraph/index.js | 26 ++-- .../src/Server/__tests__/Server-test.js | 10 +- react-packager/src/Server/index.js | 5 +- .../__tests__/extractAssetResolution-test.js | 52 -------- .../__tests__/getAssetDataFromName-test.js | 123 ++++++++++++++++++ .../__tests__/getPotentialPlatformExt-test.js | 24 ++++ .../src/lib/getAssetDataFromName.js | 38 +++++- .../src/lib/getPlatformExtension.js | 28 ++++ 11 files changed, 413 insertions(+), 86 deletions(-) delete mode 100644 react-packager/src/lib/__tests__/extractAssetResolution-test.js create mode 100644 react-packager/src/lib/__tests__/getAssetDataFromName-test.js create mode 100644 react-packager/src/lib/__tests__/getPotentialPlatformExt-test.js create mode 100644 react-packager/src/lib/getPlatformExtension.js diff --git a/react-packager/src/AssetServer/__tests__/AssetServer-test.js b/react-packager/src/AssetServer/__tests__/AssetServer-test.js index 54ef7012..d3688914 100644 --- a/react-packager/src/AssetServer/__tests__/AssetServer-test.js +++ b/react-packager/src/AssetServer/__tests__/AssetServer-test.js @@ -1,6 +1,7 @@ 'use strict'; jest + .dontMock('../../lib/getPlatformExtension') .dontMock('../../lib/getAssetDataFromName') .dontMock('../'); @@ -47,6 +48,43 @@ describe('AssetServer', () => { ); }); + pit('should work for the simple case with platform ext', () => { + const server = new AssetServer({ + projectRoots: ['/root'], + assetExts: ['png'], + }); + + fs.__setMockFilesystem({ + 'root': { + imgs: { + 'b.ios.png': 'b ios image', + 'b.android.png': 'b android image', + 'c.png': 'c general image', + 'c.android.png': 'c android image', + } + } + }); + + return Promise.all([ + server.get('imgs/b.png', 'ios').then( + data => expect(data).toBe('b ios image') + ), + server.get('imgs/b.png', 'android').then( + data => expect(data).toBe('b android image') + ), + server.get('imgs/c.png', 'android').then( + data => expect(data).toBe('c android image') + ), + server.get('imgs/c.png', 'ios').then( + data => expect(data).toBe('c general image') + ), + server.get('imgs/c.png').then( + data => expect(data).toBe('c general image') + ), + ]); + }); + + pit('should work for the simple case with jpg', () => { const server = new AssetServer({ projectRoots: ['/root'], @@ -95,6 +133,37 @@ describe('AssetServer', () => { ); }); + pit('should pick the bigger one with platform ext', () => { + const server = new AssetServer({ + projectRoots: ['/root'], + assetExts: ['png'], + }); + + fs.__setMockFilesystem({ + 'root': { + imgs: { + 'b@1x.png': 'b1 image', + 'b@2x.png': 'b2 image', + 'b@4x.png': 'b4 image', + 'b@4.5x.png': 'b4.5 image', + 'b@1x.ios.png': 'b1 ios image', + 'b@2x.ios.png': 'b2 ios image', + 'b@4x.ios.png': 'b4 ios image', + 'b@4.5x.ios.png': 'b4.5 ios image', + } + } + }); + + return Promise.all([ + server.get('imgs/b@3x.png').then(data => + expect(data).toBe('b4 image') + ), + server.get('imgs/b@3x.png', 'ios').then(data => + expect(data).toBe('b4 ios image') + ), + ]); + }); + pit('should support multiple project roots', () => { const server = new AssetServer({ projectRoots: ['/root', '/root2'], diff --git a/react-packager/src/AssetServer/index.js b/react-packager/src/AssetServer/index.js index f442f6b8..5e4c1127 100644 --- a/react-packager/src/AssetServer/index.js +++ b/react-packager/src/AssetServer/index.js @@ -20,7 +20,6 @@ const stat = Promise.denodeify(fs.stat); const readDir = Promise.denodeify(fs.readdir); const readFile = Promise.denodeify(fs.readFile); - const validateOpts = declareOpts({ projectRoots: { type: 'array', @@ -39,9 +38,9 @@ class AssetServer { this._assetExts = opts.assetExts; } - get(assetPath) { + get(assetPath, platform = null) { const assetData = getAssetDataFromName(assetPath); - return this._getAssetRecord(assetPath).then(record => { + return this._getAssetRecord(assetPath, platform).then(record => { for (let i = 0; i < record.scales.length; i++) { if (record.scales[i] >= assetData.resolution) { return readFile(record.files[i]); @@ -85,9 +84,10 @@ class AssetServer { * 1. We first parse the directory of the asset * 2. We check to find a matching directory in one of the project roots * 3. We then build a map of all assets and their scales in this directory - * 4. Then pick the closest resolution (rounding up) to the requested one + * 4. Then try to pick platform-specific asset records + * 5. Then pick the closest resolution (rounding up) to the requested one */ - _getAssetRecord(assetPath) { + _getAssetRecord(assetPath, platform = null) { const filename = path.basename(assetPath); return ( @@ -104,11 +104,20 @@ class AssetServer { const files = res[1]; const assetData = getAssetDataFromName(filename); - const map = this._buildAssetMap(dir, files); - const record = map[assetData.assetName]; + const map = this._buildAssetMap(dir, files, platform); + + let record; + if (platform != null){ + record = map[getAssetKey(assetData.assetName, platform)] || + map[assetData.assetName]; + } else { + record = map[assetData.assetName]; + } if (!record) { - throw new Error('Asset not found'); + throw new Error( + `Asset not found: ${assetPath} for platform: ${platform}` + ); } return record; @@ -141,9 +150,10 @@ class AssetServer { const map = Object.create(null); assets.forEach(function(asset, i) { const file = files[i]; - let record = map[asset.assetName]; + const assetKey = getAssetKey(asset.assetName, asset.platform); + let record = map[assetKey]; if (!record) { - record = map[asset.assetName] = { + record = map[assetKey] = { scales: [], files: [], }; @@ -151,6 +161,7 @@ class AssetServer { let insertIndex; const length = record.scales.length; + for (insertIndex = 0; insertIndex < length; insertIndex++) { if (asset.resolution < record.scales[insertIndex]) { break; @@ -164,4 +175,12 @@ class AssetServer { } } +function getAssetKey(assetName, platform) { + if (platform != null) { + return `${assetName} : ${platform}`; + } else { + return assetName; + } +} + module.exports = AssetServer; diff --git a/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js b/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js index bf4f6596..6dc2bc7e 100644 --- a/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js +++ b/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js @@ -16,6 +16,7 @@ jest .dontMock('../../crawlers') .dontMock('../../crawlers/node') .dontMock('../../replacePatterns') + .dontMock('../../../lib/getPlatformExtension') .dontMock('../../../lib/getAssetDataFromName') .dontMock('../../fastfs') .dontMock('../../AssetModule_DEPRECATED') @@ -423,6 +424,90 @@ describe('DependencyGraph', function() { }); }); + pit('should respect platform extension in assets', function() { + var root = '/root'; + fs.__setMockFilesystem({ + 'root': { + 'index.js': [ + '/**', + ' * @providesModule index', + ' */', + 'require("./imgs/a.png");', + 'require("./imgs/b.png");', + 'require("./imgs/c.png");', + ].join('\n'), + 'imgs': { + 'a@1.5x.ios.png': '', + 'b@.7x.ios.png': '', + 'c.ios.png': '', + 'c@2x.ios.png': '', + }, + 'package.json': JSON.stringify({ + name: 'rootPackage' + }), + } + }); + + var dgraph = new DependencyGraph({ + roots: [root], + fileWatcher: fileWatcher, + assetExts: ['png', 'jpg'], + cache: cache, + }); + + dgraph.setup({ platform: 'ios' }); + + return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) { + expect(deps) + .toEqual([ + { + id: 'index', + path: '/root/index.js', + dependencies: [ + './imgs/a.png', + './imgs/b.png', + './imgs/c.png', + ], + isAsset: false, + isAsset_DEPRECATED: false, + isJSON: false, + isPolyfill: false, + resolution: undefined, + }, + { + id: 'rootPackage/imgs/a.png', + path: '/root/imgs/a@1.5x.ios.png', + resolution: 1.5, + dependencies: [], + isAsset: true, + isAsset_DEPRECATED: false, + isJSON: false, + isPolyfill: false, + }, + { + id: 'rootPackage/imgs/b.png', + path: '/root/imgs/b@.7x.ios.png', + resolution: 0.7, + dependencies: [], + isAsset: true, + isAsset_DEPRECATED: false, + isJSON: false, + isPolyfill: false, + }, + { + id: 'rootPackage/imgs/c.png', + path: '/root/imgs/c.ios.png', + resolution: 1, + dependencies: [], + isAsset: true, + isAsset_DEPRECATED: false, + isJSON: false, + isPolyfill: false, + }, + ]); + }); + }); + pit('Deprecated and relative assets can live together', function() { var root = '/root'; fs.__setMockFilesystem({ diff --git a/react-packager/src/DependencyResolver/DependencyGraph/index.js b/react-packager/src/DependencyResolver/DependencyGraph/index.js index f2b77f4d..0820c14a 100644 --- a/react-packager/src/DependencyResolver/DependencyGraph/index.js +++ b/react-packager/src/DependencyResolver/DependencyGraph/index.js @@ -1,4 +1,4 @@ -/** + /** * Copyright (c) 2015-present, Facebook, Inc. * All rights reserved. * @@ -17,6 +17,7 @@ const crawl = require('../crawlers'); const debug = require('debug')('DependencyGraph'); const declareOpts = require('../../lib/declareOpts'); const getAssetDataFromName = require('../../lib/getAssetDataFromName'); +const getPontentialPlatformExt = require('../../lib/getPlatformExtension'); const isAbsolutePath = require('absolute-path'); const path = require('path'); const util = require('util'); @@ -274,7 +275,7 @@ class DependencyGraph { // `platformExt` could be set in the `setup` method. if (!this._platformExt) { - const platformExt = getPlatformExt(entryPath); + const platformExt = getPontentialPlatformExt(entryPath); if (platformExt && this._opts.platforms.indexOf(platformExt) > -1) { this._platformExt = platformExt; } else { @@ -390,12 +391,18 @@ class DependencyGraph { return Promise.resolve().then(() => { if (this._isAssetFile(potentialModulePath)) { const {name, type} = getAssetDataFromName(potentialModulePath); - const pattern = new RegExp('^' + name + '(@[\\d\\.]+x)?\\.' + type); + + let pattern = '^' + name + '(@[\\d\\.]+x)?'; + if (this._platformExt != null) { + pattern += '(\\.' + this._platformExt + ')?'; + } + pattern += '\\.' + type; + // We arbitrarly grab the first one, because scale selection // will happen somewhere const [assetFile] = this._fastfs.matches( path.dirname(potentialModulePath), - pattern + new RegExp(pattern) ); if (assetFile) { @@ -496,7 +503,7 @@ class DependencyGraph { const modules = this._hasteMap[name]; if (this._platformExt != null) { for (let i = 0; i < modules.length; i++) { - if (getPlatformExt(modules[i].path) === this._platformExt) { + if (getPontentialPlatformExt(modules[i].path) === this._platformExt) { return modules[i]; } } @@ -662,15 +669,6 @@ function normalizePath(modulePath) { return modulePath.replace(/\/$/, ''); } -// Extract platform extension: index.ios.js -> ios -function getPlatformExt(file) { - const parts = path.basename(file).split('.'); - if (parts.length < 3) { - return null; - } - return parts[parts.length - 2]; -} - util.inherits(NotFoundError, Error); module.exports = DependencyGraph; diff --git a/react-packager/src/Server/__tests__/Server-test.js b/react-packager/src/Server/__tests__/Server-test.js index 5152054e..64af474a 100644 --- a/react-packager/src/Server/__tests__/Server-test.js +++ b/react-packager/src/Server/__tests__/Server-test.js @@ -244,8 +244,16 @@ describe('processRequest', () => { expect(res.end).toBeCalledWith('i am image'); }); - it('should return 404', () => { + it('should parse the platform option', () => { + const req = {url: '/assets/imgs/a.png?platform=ios'}; + const res = {end: jest.genMockFn()}; + AssetServer.prototype.get.mockImpl(() => Promise.resolve('i am image')); + + server.processRequest(req, res); + jest.runAllTimers(); + expect(AssetServer.prototype.get).toBeCalledWith('imgs/a.png', 'ios'); + expect(res.end).toBeCalledWith('i am image'); }); }); diff --git a/react-packager/src/Server/index.js b/react-packager/src/Server/index.js index 7d65d0a2..e889a357 100644 --- a/react-packager/src/Server/index.js +++ b/react-packager/src/Server/index.js @@ -278,7 +278,8 @@ class Server { _processAssetsRequest(req, res) { const urlObj = url.parse(req.url, true); const assetPath = urlObj.pathname.match(/^\/assets\/(.+)$/); - this._assetServer.get(assetPath[1]) + const assetEvent = Activity.startEvent(`processing asset request ${assetPath[1]}`); + this._assetServer.get(assetPath[1], urlObj.query.platform) .then( data => res.end(data), error => { @@ -286,7 +287,7 @@ class Server { res.writeHead('404'); res.end('Asset not found'); } - ).done(); + ).done(() => Activity.endEvent(assetEvent)); } _processProfile(req, res) { diff --git a/react-packager/src/lib/__tests__/extractAssetResolution-test.js b/react-packager/src/lib/__tests__/extractAssetResolution-test.js deleted file mode 100644 index d0309ca6..00000000 --- a/react-packager/src/lib/__tests__/extractAssetResolution-test.js +++ /dev/null @@ -1,52 +0,0 @@ -'use strict'; - -jest.autoMockOff(); -var getAssetDataFromName = require('../getAssetDataFromName'); - -describe('getAssetDataFromName', function() { - it('should extract resolution simple case', function() { - var data = getAssetDataFromName('test@2x.png'); - expect(data).toEqual({ - assetName: 'test.png', - resolution: 2, - type: 'png', - name: 'test', - }); - }); - - it('should default resolution to 1', function() { - var data = getAssetDataFromName('test.png'); - expect(data).toEqual({ - assetName: 'test.png', - resolution: 1, - type: 'png', - name: 'test', - }); - }); - - it('should support float', function() { - var data = getAssetDataFromName('test@1.1x.png'); - expect(data).toEqual({ - assetName: 'test.png', - resolution: 1.1, - type: 'png', - name: 'test', - }); - - data = getAssetDataFromName('test@.1x.png'); - expect(data).toEqual({ - assetName: 'test.png', - resolution: 0.1, - type: 'png', - name: 'test', - }); - - data = getAssetDataFromName('test@0.2x.png'); - expect(data).toEqual({ - assetName: 'test.png', - resolution: 0.2, - type: 'png', - name: 'test', - }); - }); -}); diff --git a/react-packager/src/lib/__tests__/getAssetDataFromName-test.js b/react-packager/src/lib/__tests__/getAssetDataFromName-test.js new file mode 100644 index 00000000..d6711001 --- /dev/null +++ b/react-packager/src/lib/__tests__/getAssetDataFromName-test.js @@ -0,0 +1,123 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * 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. + */ +'use strict'; + +jest.dontMock('../getPlatformExtension') + .dontMock('../getAssetDataFromName'); + +describe('getAssetDataFromName', () => { + let getAssetDataFromName; + + beforeEach(() => { + getAssetDataFromName = require('../getAssetDataFromName'); + }); + + it('should get data from name', () => { + expect(getAssetDataFromName('a/b/c.png')).toEqual({ + resolution: 1, + assetName: 'a/b/c.png', + type: 'png', + name: 'c', + platform: null, + }); + + expect(getAssetDataFromName('a/b/c@1x.png')).toEqual({ + resolution: 1, + assetName: 'a/b/c.png', + type: 'png', + name: 'c', + platform: null, + }); + + expect(getAssetDataFromName('a/b/c@2.5x.png')).toEqual({ + resolution: 2.5, + assetName: 'a/b/c.png', + type: 'png', + name: 'c', + platform: null, + }); + + expect(getAssetDataFromName('a/b/c.ios.png')).toEqual({ + resolution: 1, + assetName: 'a/b/c.png', + type: 'png', + name: 'c', + platform: 'ios', + }); + + expect(getAssetDataFromName('a/b/c@1x.ios.png')).toEqual({ + resolution: 1, + assetName: 'a/b/c.png', + type: 'png', + name: 'c', + platform: 'ios', + }); + + expect(getAssetDataFromName('a/b/c@2.5x.ios.png')).toEqual({ + resolution: 2.5, + assetName: 'a/b/c.png', + type: 'png', + name: 'c', + platform: 'ios', + }); + }); + + describe('resolution extraction', () => { + it('should extract resolution simple case', () => { + var data = getAssetDataFromName('test@2x.png'); + expect(data).toEqual({ + assetName: 'test.png', + resolution: 2, + type: 'png', + name: 'test', + platform: null, + }); + }); + + it('should default resolution to 1', () => { + var data = getAssetDataFromName('test.png'); + expect(data).toEqual({ + assetName: 'test.png', + resolution: 1, + type: 'png', + name: 'test', + platform: null, + }); + }); + + it('should support float', () => { + var data = getAssetDataFromName('test@1.1x.png'); + expect(data).toEqual({ + assetName: 'test.png', + resolution: 1.1, + type: 'png', + name: 'test', + platform: null, + }); + + data = getAssetDataFromName('test@.1x.png'); + expect(data).toEqual({ + assetName: 'test.png', + resolution: 0.1, + type: 'png', + name: 'test', + platform: null, + }); + + data = getAssetDataFromName('test@0.2x.png'); + expect(data).toEqual({ + assetName: 'test.png', + resolution: 0.2, + type: 'png', + name: 'test', + platform: null, + }); + }); + }); +}); diff --git a/react-packager/src/lib/__tests__/getPotentialPlatformExt-test.js b/react-packager/src/lib/__tests__/getPotentialPlatformExt-test.js new file mode 100644 index 00000000..5f734880 --- /dev/null +++ b/react-packager/src/lib/__tests__/getPotentialPlatformExt-test.js @@ -0,0 +1,24 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * 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. + */ +'use strict'; + +jest.dontMock('../getPlatformExtension'); + +describe('getPlatformExtension', function() { + it('should get platform ext', function() { + var getPlatformExtension = require('../getPlatformExtension'); + expect(getPlatformExtension('a.ios.js')).toBe('ios'); + expect(getPlatformExtension('a.android.js')).toBe('android'); + expect(getPlatformExtension('/b/c/a.ios.js')).toBe('ios'); + expect(getPlatformExtension('/b/c.android/a.ios.js')).toBe('ios'); + expect(getPlatformExtension('/b/c/a@1.5x.ios.png')).toBe('ios'); + expect(getPlatformExtension('/b/c/a@1.5x.lol.png')).toBe(null); + expect(getPlatformExtension('/b/c/a.lol.png')).toBe(null); + }); +}); diff --git a/react-packager/src/lib/getAssetDataFromName.js b/react-packager/src/lib/getAssetDataFromName.js index c4848fd1..33fa13ce 100644 --- a/react-packager/src/lib/getAssetDataFromName.js +++ b/react-packager/src/lib/getAssetDataFromName.js @@ -1,14 +1,29 @@ + /** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * 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. + */ 'use strict'; -var path = require('path'); +const path = require('path'); +const getPlatformExtension = require('./getPlatformExtension'); function getAssetDataFromName(filename) { - var ext = path.extname(filename); + const ext = path.extname(filename); + const platformExt = getPlatformExtension(filename); - var re = new RegExp('@([\\d\\.]+)x\\' + ext + '$'); + let pattern = '@([\\d\\.]+)x'; + if (platformExt != null) { + pattern += '(\\.' + platformExt + ')?'; + } + pattern += '\\' + ext + '$'; + const re = new RegExp(pattern); - var match = filename.match(re); - var resolution; + const match = filename.match(re); + let resolution; if (!(match && match[1])) { resolution = 1; @@ -19,12 +34,21 @@ function getAssetDataFromName(filename) { } } - var assetName = match ? filename.replace(re, ext) : filename; + let assetName; + if (match) { + assetName = filename.replace(re, ext); + } else if (platformExt != null) { + assetName = filename.replace(new RegExp(`\\.${platformExt}\\${ext}`), ext); + } else { + assetName = filename; + } + return { resolution: resolution, assetName: assetName, type: ext.slice(1), - name: path.basename(assetName, ext) + name: path.basename(assetName, ext), + platform: platformExt, }; } diff --git a/react-packager/src/lib/getPlatformExtension.js b/react-packager/src/lib/getPlatformExtension.js new file mode 100644 index 00000000..3f34da42 --- /dev/null +++ b/react-packager/src/lib/getPlatformExtension.js @@ -0,0 +1,28 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * 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. + */ +'use strict'; + +const path = require('path'); + +const SUPPORTED_PLATFORM_EXTS = ['android', 'ios']; + +const re = new RegExp( + '[^\\.]+\\.(' + SUPPORTED_PLATFORM_EXTS.join('|') + ')\\.\\w+$' +); + +// Extract platform extension: index.ios.js -> ios +function getPlatformExtension(file) { + const match = file.match(re); + if (match && match[1]) { + return match[1]; + } + return null; +} + +module.exports = getPlatformExtension;