From 75d999190b856839d77a519dcd381b6e9b222d5f Mon Sep 17 00:00:00 2001 From: Rafael Oleza Date: Mon, 24 Sep 2018 10:06:24 -0700 Subject: [PATCH] Remove absolute path parameter from transformers Reviewed By: mjesun Differential Revision: D9195147 fbshipit-source-id: e95c5c29272eec7f1a48af7cede4c6d6b4a8ffb7 --- packages/metro/src/Bundler.js | 10 +- packages/metro/src/DeltaBundler/Worker.js | 9 +- packages/metro/src/DeltaBundler/WorkerFarm.js | 4 +- .../DeltaBundler/__tests__/WorkerFarm-test.js | 28 ++- packages/metro/src/JSTransformer/worker.js | 8 +- .../__snapshots__/worker-test.js.snap | 2 +- .../worker/__tests__/worker-test.js | 187 ++++++++---------- packages/metro/src/assetTransformer.js | 9 +- 8 files changed, 113 insertions(+), 144 deletions(-) diff --git a/packages/metro/src/Bundler.js b/packages/metro/src/Bundler.js index a4b1dc68ee..024a93f8c0 100644 --- a/packages/metro/src/Bundler.js +++ b/packages/metro/src/Bundler.js @@ -16,7 +16,7 @@ const WorkerFarm = require('./DeltaBundler/WorkerFarm'); const assert = require('assert'); const fs = require('fs'); const getTransformCacheKeyFn = require('./lib/getTransformCacheKeyFn'); -const toLocalPath = require('./node-haste/lib/toLocalPath'); +const path = require('path'); const {Cache, stableHash} = require('metro-cache'); @@ -128,14 +128,14 @@ class Bundler { } } - const localPath = toLocalPath(this._opts.watchFolders, filePath); + const filename = path.relative(this._opts.projectRoot, filePath); const partialKey = stableHash([ // This is the hash related to the global Bundler config. this._baseHash, // Path. - localPath, + filename, // We cannot include "transformCodeOptions" because of "projectRoot". assetPlugins, @@ -165,8 +165,8 @@ class Bundler { const data = result ? {result, sha1} : await this._transformer.transform( - filePath, - localPath, + filename, + _projectRoot, this._opts.transformerPath, workerOptions, ); diff --git a/packages/metro/src/DeltaBundler/Worker.js b/packages/metro/src/DeltaBundler/Worker.js index 860cbffc1b..f9ee0c5012 100644 --- a/packages/metro/src/DeltaBundler/Worker.js +++ b/packages/metro/src/DeltaBundler/Worker.js @@ -12,9 +12,9 @@ const crypto = require('crypto'); const fs = require('fs'); +const path = require('path'); import type {WorkerOptions as JsWorkerOptions} from '../JSTransformer/worker'; -import type {LocalPath} from '../node-haste/lib/toLocalPath'; import type {MixedOutput, TransformResultDependency} from './types.flow'; import type {LogEntry} from 'metro-core/src/Logger'; @@ -22,7 +22,6 @@ export type WorkerOptions = JsWorkerOptions; export type WorkerFn = typeof transform; export type TransformerFn = ( string, - LocalPath, Buffer, WorkerOptions, ) => Promise>; @@ -41,7 +40,7 @@ type Data = { async function transform( filename: string, - localPath: LocalPath, + projectRoot: string, transformerPath: string, transformerOptions: WorkerOptions, ): Promise> { @@ -53,7 +52,7 @@ async function transform( start_timestamp: process.hrtime(), }; - const data = fs.readFileSync(filename); + const data = fs.readFileSync(path.resolve(projectRoot, filename)); const sha1 = crypto .createHash('sha1') .update(data) @@ -65,7 +64,7 @@ async function transform( transform: TransformerFn, }); - const result = await transform(filename, localPath, data, transformerOptions); + const result = await transform(filename, data, transformerOptions); const transformFileEndLogEntry = getEndLogEntry( transformFileStartLogEntry, diff --git a/packages/metro/src/DeltaBundler/WorkerFarm.js b/packages/metro/src/DeltaBundler/WorkerFarm.js index e87eda5145..35cbbe2801 100644 --- a/packages/metro/src/DeltaBundler/WorkerFarm.js +++ b/packages/metro/src/DeltaBundler/WorkerFarm.js @@ -67,14 +67,14 @@ class WorkerFarm { async transform( filename: string, - localPath: string, + projectRoot: string, transformerPath: string, options: WorkerOptions, ): Promise { try { const data = await this._worker.transform( filename, - localPath, + projectRoot, transformerPath, options, ); diff --git a/packages/metro/src/DeltaBundler/__tests__/WorkerFarm-test.js b/packages/metro/src/DeltaBundler/__tests__/WorkerFarm-test.js index d0a1df259d..b46491a6de 100644 --- a/packages/metro/src/DeltaBundler/__tests__/WorkerFarm-test.js +++ b/packages/metro/src/DeltaBundler/__tests__/WorkerFarm-test.js @@ -16,8 +16,8 @@ const {Readable} = require('stream'); describe('Worker Farm', function() { let api; let WorkerFarm; - const fileName = '/an/arbitrary/file.js'; - const localPath = 'arbitrary/file.js'; + const fileName = 'arbitrary/file.js'; + const rootFolder = '/root'; const config = getDefaultConfig(); const opts = { @@ -70,14 +70,14 @@ describe('Worker Farm', function() { await new WorkerFarm(opts).transform( fileName, - localPath, + rootFolder, config.transformerPath, transformOptions, ); expect(api.transform).toBeCalledWith( fileName, - localPath, + rootFolder, config.transformerPath, transformOptions, ); @@ -88,26 +88,24 @@ describe('Worker Farm', function() { const message = 'message'; const snippet = 'snippet'; - api.transform.mockImplementation( - (filename, localPth, transformPath, opts) => { - const babelError = new SyntaxError(message); + api.transform.mockImplementation((filename, transformPath, opts) => { + const babelError = new SyntaxError(message); - babelError.type = 'SyntaxError'; - babelError.loc = {line: 2, column: 15}; - babelError.codeFrame = snippet; + babelError.type = 'SyntaxError'; + babelError.loc = {line: 2, column: 15}; + babelError.codeFrame = snippet; - return Promise.reject(babelError); - }, - ); + return Promise.reject(babelError); + }); expect.assertions(6); return workerFarm - .transform(fileName, localPath, '', true, {}) + .transform(fileName, rootFolder, '', {}) .catch(function(error) { expect(error.type).toEqual('TransformError'); expect(error.message).toBe( - 'SyntaxError in /an/arbitrary/file.js: ' + message, + 'SyntaxError in arbitrary/file.js: ' + message, ); expect(error.lineNumber).toBe(2); expect(error.column).toBe(15); diff --git a/packages/metro/src/JSTransformer/worker.js b/packages/metro/src/JSTransformer/worker.js index 2142bae44e..eb2eb32f3a 100644 --- a/packages/metro/src/JSTransformer/worker.js +++ b/packages/metro/src/JSTransformer/worker.js @@ -34,14 +34,12 @@ const { import type {TransformResultDependency} from 'metro/src/DeltaBundler'; import type {DynamicRequiresBehavior} from '../ModuleGraph/worker/collectDependencies'; -import type {LocalPath} from '../node-haste/lib/toLocalPath'; import type {Ast} from '@babel/core'; import type {Plugins as BabelPlugins} from 'babel-core'; import type {MetroSourceMapSegmentTuple} from 'metro-source-map'; export type TransformArgs = {| filename: string, - localPath: string, options: TransformOptions, plugins?: BabelPlugins, src: string, @@ -124,7 +122,6 @@ function getDynamicDepsBehavior( async function transform( filename: string, - localPath: LocalPath, data: Buffer, options: WorkerOptions, ): Promise { @@ -160,7 +157,6 @@ async function transform( const transformerArgs = { filename, - localPath, options: { ...options.transformOptions, // Inline requires are now performed at a secondary step. We cannot @@ -229,7 +225,7 @@ async function transform( configFile: false, comments: false, compact: false, - filename: localPath, + filename, plugins, sourceMaps: false, })); @@ -283,7 +279,7 @@ async function transform( { comments: false, compact: false, - filename: localPath, + filename, retainLines: false, sourceFileName: filename, sourceMaps: true, diff --git a/packages/metro/src/JSTransformer/worker/__tests__/__snapshots__/worker-test.js.snap b/packages/metro/src/JSTransformer/worker/__tests__/__snapshots__/worker-test.js.snap index e1bc9c4e66..acc7150c40 100644 --- a/packages/metro/src/JSTransformer/worker/__tests__/__snapshots__/worker-test.js.snap +++ b/packages/metro/src/JSTransformer/worker/__tests__/__snapshots__/worker-test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`code transformation worker: reports filename when encountering unsupported dynamic dependency 1`] = `"/root/local/file.js:Invalid call at line 3: require(a)"`; +exports[`code transformation worker: reports filename when encountering unsupported dynamic dependency 1`] = `"local/file.js:Invalid call at line 3: require(a)"`; exports[`code transformation worker: transforms a module with dependencies 1`] = ` Array [ diff --git a/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js b/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js index b2b227618c..d9111eba01 100644 --- a/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js +++ b/packages/metro/src/JSTransformer/worker/__tests__/worker-test.js @@ -25,6 +25,10 @@ const babelTransformerPath = require.resolve( ); const transformerContents = require('fs').readFileSync(babelTransformerPath); +const HEADER_DEV = + '__d(function (global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {'; +const HEADER_PROD = '__d(function (g, r, i, a, m, e, d) {'; + let fs; let mkdirp; let transform; @@ -47,7 +51,6 @@ describe('code transformation worker:', () => { it('transforms a simple script', async () => { const result = await transform( - '/root/local/file.js', 'local/file.js', 'someReallyArbitrary(code)', { @@ -77,31 +80,22 @@ describe('code transformation worker:', () => { }); it('transforms a simple module', async () => { - const result = await transform( - '/root/local/file.js', - 'local/file.js', - 'arbitrary(code)', - { - assetExts: [], - assetPlugins: [], - assetRegistryPath: '', - asyncRequireModulePath: 'asyncRequire', - type: 'module', - minifierPath: 'minifyModulePath', - babelTransformerPath, - transformOptions: {dev: true}, - dynamicDepsInPackages: 'reject', - optimizationSizeLimit: 100000, - }, - ); + const result = await transform('local/file.js', 'arbitrary(code)', { + assetExts: [], + assetPlugins: [], + assetRegistryPath: '', + asyncRequireModulePath: 'asyncRequire', + type: 'module', + minifierPath: 'minifyModulePath', + babelTransformerPath, + transformOptions: {dev: true}, + dynamicDepsInPackages: 'reject', + optimizationSizeLimit: 100000, + }); expect(result.output[0].type).toBe('js/module'); expect(result.output[0].data.code).toBe( - [ - '__d(function (global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {', - ' arbitrary(code);', - '});', - ].join('\n'), + [HEADER_DEV, ' arbitrary(code);', '});'].join('\n'), ); expect(result.output[0].data.map).toMatchSnapshot(); expect(result.dependencies).toEqual([]); @@ -116,28 +110,23 @@ describe('code transformation worker:', () => { 'import c from "./c";', ].join('\n'); - const result = await transform( - '/root/local/file.js', - 'local/file.js', - contents, - { - assetExts: [], - assetPlugins: [], - assetRegistryPath: '', - asyncRequireModulePath: 'asyncRequire', - isScript: false, - minifierPath: 'minifyModulePath', - babelTransformerPath, - transformOptions: {dev: true}, - dynamicDepsInPackages: 'reject', - optimizationSizeLimit: 100000, - }, - ); + const result = await transform('local/file.js', contents, { + assetExts: [], + assetPlugins: [], + assetRegistryPath: '', + asyncRequireModulePath: 'asyncRequire', + isScript: false, + minifierPath: 'minifyModulePath', + babelTransformerPath, + transformOptions: {dev: true}, + dynamicDepsInPackages: 'reject', + optimizationSizeLimit: 100000, + }); expect(result.output[0].type).toBe('js/module'); expect(result.output[0].data.code).toBe( [ - '__d(function (global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {', + HEADER_DEV, " 'use strict';", '', ' var _interopRequireDefault = _$$_REQUIRE(_dependencyMap[0], "@babel/runtime/helpers/interopRequireDefault");', @@ -166,7 +155,6 @@ describe('code transformation worker:', () => { it('transforms an es module with regenerator', async () => { const result = await transform( - '/root/local/file.js', 'local/file.js', 'export async function test() {}', { @@ -200,28 +188,23 @@ describe('code transformation worker:', () => { it('transforms import/export syntax when experimental flag is on', async () => { const contents = ['import c from "./c";'].join('\n'); - const result = await transform( - '/root/local/file.js', - 'local/file.js', - contents, - { - assetExts: [], - assetPlugins: [], - assetRegistryPath: '', - asyncRequireModulePath: 'asyncRequire', - isScript: false, - minifierPath: 'minifyModulePath', - babelTransformerPath, - transformOptions: {dev: true, experimentalImportSupport: true}, - dynamicDepsInPackages: 'reject', - optimizationSizeLimit: 100000, - }, - ); + const result = await transform('local/file.js', contents, { + assetExts: [], + assetPlugins: [], + assetRegistryPath: '', + asyncRequireModulePath: 'asyncRequire', + isScript: false, + minifierPath: 'minifyModulePath', + babelTransformerPath, + transformOptions: {dev: true, experimentalImportSupport: true}, + dynamicDepsInPackages: 'reject', + optimizationSizeLimit: 100000, + }); expect(result.output[0].type).toBe('js/module'); expect(result.output[0].data.code).toBe( [ - '__d(function (global, _$$_REQUIRE, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {', + HEADER_DEV, ' "use strict";', '', ' var c = _$$_IMPORT_DEFAULT(_dependencyMap[0], "./c");', @@ -247,7 +230,7 @@ describe('code transformation worker:', () => { ].join('\n'); try { - await transform('/root/local/file.js', 'local/file.js', contents, { + await transform('local/file.js', contents, { assetExts: [], assetPlugins: [], assetRegistryPath: '', @@ -266,11 +249,8 @@ describe('code transformation worker:', () => { }); it('supports dynamic dependencies from within `node_modules`', async () => { - await transform( - '/root/node_modules/foo/bar.js', - 'node_modules/foo/bar.js', - 'require(foo.bar);', - { + expect( + (await transform('node_modules/foo/bar.js', 'require(foo.bar);', { assetExts: [], assetPlugins: [], assetRegistryPath: '', @@ -280,56 +260,49 @@ describe('code transformation worker:', () => { babelTransformerPath, transformOptions: {dev: true}, dynamicDepsInPackages: 'throwAtRuntime', - optimizationSizeLimit: 100000, - }, + })).output[0].data.code, + ).toBe( + [ + HEADER_DEV, + ' (function (line) {', + " throw new Error('Dynamic require defined at line ' + line + '; not supported by Metro');", + ' })(1);', + '});', + ].join('\n'), ); }); it('minifies the code correctly', async () => { expect( - (await transform( - '/root/local/file.js', - 'local/file.js', - 'arbitrary(code);', - { - assetExts: [], - assetPlugins: [], - assetRegistryPath: '', - asyncRequireModulePath: 'asyncRequire', - isScript: false, - minifierPath: 'minifyModulePath', - babelTransformerPath, - transformOptions: {dev: true, minify: true}, - dynamicDepsInPackages: 'throwAtRuntime', - optimizationSizeLimit: 100000, - }, - )).output[0].data.code, - ).toBe( - ['__d(function (g, r, i, a, m, e, d) {', ' minified(code);', '});'].join( - '\n', - ), - ); + (await transform('local/file.js', 'arbitrary(code);', { + assetExts: [], + assetPlugins: [], + assetRegistryPath: '', + asyncRequireModulePath: 'asyncRequire', + isScript: false, + minifierPath: 'minifyModulePath', + babelTransformerPath, + transformOptions: {dev: true, minify: true}, + dynamicDepsInPackages: 'throwAtRuntime', + optimizationSizeLimit: 100000, + })).output[0].data.code, + ).toBe([HEADER_PROD, ' minified(code);', '});'].join('\n')); }); it('minifies a JSON file', async () => { expect( - (await transform( - '/root/local/file.json', - 'local/file.json', - 'arbitrary(code);', - { - assetExts: [], - assetPlugins: [], - assetRegistryPath: '', - asyncRequireModulePath: 'asyncRequire', - isScript: false, - minifierPath: 'minifyModulePath', - babelTransformerPath, - transformOptions: {dev: true, minify: true}, - dynamicDepsInPackages: 'throwAtRuntime', - optimizationsizelimit: 100000, - }, - )).output[0].data.code, + (await transform('local/file.json', 'arbitrary(code);', { + assetExts: [], + assetPlugins: [], + assetRegistryPath: '', + asyncRequireModulePath: 'asyncRequire', + isScript: false, + minifierPath: 'minifyModulePath', + babelTransformerPath, + transformOptions: {dev: true, minify: true}, + dynamicDepsInPackages: 'throwAtRuntime', + optimizationsizelimit: 100000, + })).output[0].data.code, ).toBe( [ '__d(function(global, require, _aUnused, _bUnused, module, exports, _cUnused) {', diff --git a/packages/metro/src/assetTransformer.js b/packages/metro/src/assetTransformer.js index 58661df2a0..888128f44e 100644 --- a/packages/metro/src/assetTransformer.js +++ b/packages/metro/src/assetTransformer.js @@ -9,6 +9,8 @@ */ 'use strict'; +const path = require('path'); + const {getAssetData} = require('./Assets'); const {generateAssetCodeFileAst} = require('./Bundler/util'); @@ -16,14 +18,13 @@ import type {TransformOptions} from './JSTransformer/worker'; import type {Ast} from '@babel/core'; type Params = { - localPath: string, filename: string, options: TransformOptions, src: string, }; async function transform( - {filename, localPath, options, src}: Params, + {filename, options, src}: Params, assetRegistryPath: string, assetDataPlugins: $ReadOnlyArray, ): Promise<{ast: Ast}> { @@ -34,9 +35,11 @@ async function transform( minify: false, }; + const absolutePath = path.resolve(options.projectRoot, filename); + const data = await getAssetData( + absolutePath, filename, - localPath, assetDataPlugins, options.platform, );