Skip to content

Commit

Permalink
metro: allow dynamic dependencies from within node_modules
Browse files Browse the repository at this point in the history
Summary: Tries to adress #65. We need a reasonnable workaround to support modules like `moment.js` that do dynamic requires but only in some cases. By replacing the call by a function that throws, we move the exception at runtime instead of happening at compile time. We don't want to do that for non-node_modules file because they are fixable directly, while `node_modules` are not fixable by people and they get completely blocked by the error at compile time.

Reviewed By: rafeca

Differential Revision: D6736989

fbshipit-source-id: a6e1fd9b56fa83907400884efd8f8594018b7c37
  • Loading branch information
Jean Lauliac authored and facebook-github-bot committed Jan 18, 2018
1 parent 40497ee commit 46545d4
Show file tree
Hide file tree
Showing 17 changed files with 185 additions and 50 deletions.
16 changes: 10 additions & 6 deletions packages/metro/src/Bundler/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const {

import type {PostProcessModules} from '../DeltaBundler';
import type {Options as JSTransformerOptions} from '../JSTransformer/worker';
import type {DynamicRequiresBehavior} from '../ModuleGraph/worker/collectDependencies';
import type {GlobalTransformCache} from '../lib/GlobalTransformCache';
import type {TransformCache} from '../lib/TransformCaching';
import type {Reporter} from '../lib/reporting';
Expand Down Expand Up @@ -82,6 +83,7 @@ export type Options = {|
+assetRegistryPath: string,
+blacklistRE?: RegExp,
+cacheVersion: string,
+dynamicDepsInPackages: DynamicRequiresBehavior,
+enableBabelRCLookup: boolean,
+extraNodeModules: {},
+getPolyfills: ({platform: ?string}) => $ReadOnlyArray<string>,
Expand Down Expand Up @@ -117,17 +119,18 @@ class Bundler {

opts.projectRoots.forEach(verifyRootExists);

this._transformer = new Transformer(
opts.transformModulePath,
opts.maxWorkers,
{
this._transformer = new Transformer({
maxWorkers: opts.maxWorkers,
reporters: {
stdoutChunk: chunk =>
opts.reporter.update({type: 'worker_stdout_chunk', chunk}),
stderrChunk: chunk =>
opts.reporter.update({type: 'worker_stderr_chunk', chunk}),
},
opts.workerPath || undefined,
);
transformModulePath: opts.transformModulePath,
dynamicDepsInPackages: opts.dynamicDepsInPackages,
workerPath: opts.workerPath || undefined,
});

this._depGraphPromise = DependencyGraph.load({
assetExts: opts.assetExts,
Expand All @@ -137,6 +140,7 @@ class Bundler {
getPolyfills: opts.getPolyfills,
getTransformCacheKey: getTransformCacheKeyFn({
cacheVersion: opts.cacheVersion,
dynamicDepsInPackages: opts.dynamicDepsInPackages,
projectRoots: opts.projectRoots,
transformModulePath: opts.transformModulePath,
}),
Expand Down
5 changes: 5 additions & 0 deletions packages/metro/src/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type {
import type {PostProcessModules} from './DeltaBundler';
import type {PostProcessModules as PostProcessModulesForBuck} from './ModuleGraph/types.flow.js';
import type {TransformVariants} from './ModuleGraph/types.flow';
import type {DynamicRequiresBehavior} from './ModuleGraph/worker/collectDependencies';
import type {HasteImpl} from './node-haste/Module';
import type {IncomingMessage, ServerResponse} from 'http';

Expand All @@ -39,6 +40,9 @@ export type ConfigT = {
enhanceMiddleware: Middleware => Middleware,

extraNodeModules: {[id: string]: string},

+dynamicDepsInPackages: DynamicRequiresBehavior,

/**
* Specify any additional asset file extensions to be used by the packager.
* For example, if you want to include a .ttf file, you would return ['ttf']
Expand Down Expand Up @@ -160,6 +164,7 @@ const DEFAULT = ({
enhanceMiddleware: middleware => middleware,
extraNodeModules: {},
assetTransforms: false,
dynamicDepsInPackages: 'throwAtRuntime',
getAssetExts: () => [],
getBlacklistRE: () => blacklist(),
getEnableBabelRCLookup: () => false,
Expand Down
13 changes: 11 additions & 2 deletions packages/metro/src/JSTransformer/__tests__/Transformer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ describe('Transformer', function() {
const localPath = 'arbitrary/file.js';
const transformModulePath = __filename;

const opts = {
maxWorkers: 4,
reporters: {},
transformModulePath,
dynamicDepsInPackages: 'reject',
workerPath: null,
};

beforeEach(function() {
Cache = jest.fn();
Cache.prototype.get = jest.fn((a, b, c) => c());
Expand Down Expand Up @@ -55,7 +63,7 @@ describe('Transformer', function() {
const transformOptions = {arbitrary: 'options'};
const code = 'arbitrary(code)';

new Transformer(transformModulePath, 4).transformFile(
new Transformer(opts).transformFile(
fileName,
localPath,
code,
Expand All @@ -74,11 +82,12 @@ describe('Transformer', function() {
transformOptions,
[],
'',
'reject',
);
});

it('should add file info to parse errors', () => {
const transformer = new Transformer(transformModulePath, 4);
const transformer = new Transformer(opts);
const message = 'message';
const snippet = 'snippet';

Expand Down
32 changes: 20 additions & 12 deletions packages/metro/src/JSTransformer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import type {BabelSourceMap} from 'babel-core';
import type {Options, TransformedCode} from './worker';
import type {LocalPath} from '../node-haste/lib/toLocalPath';
import type {ResultWithMap} from './worker/minify';
import type {DynamicRequiresBehavior} from '../ModuleGraph/worker/collectDependencies';

import typeof {minify as Minify, transform as Transform} from './worker';

Expand All @@ -37,31 +38,37 @@ type Reporters = {
module.exports = class Transformer {
_worker: WorkerInterface;
_transformModulePath: string;

constructor(
transformModulePath: string,
maxWorkers: number,
reporters: Reporters,
workerPath: string = require.resolve('./worker'),
) {
this._transformModulePath = transformModulePath;

if (maxWorkers > 1) {
_dynamicDepsInPackages: DynamicRequiresBehavior;

constructor(options: {|
+maxWorkers: number,
+reporters: Reporters,
+transformModulePath: string,
+dynamicDepsInPackages: DynamicRequiresBehavior,
+workerPath: ?string,
|}) {
this._transformModulePath = options.transformModulePath;
this._dynamicDepsInPackages = options.dynamicDepsInPackages;
const {workerPath = require.resolve('./worker')} = options;

if (options.maxWorkers > 1) {
this._worker = this._makeFarm(
workerPath,
this._computeWorkerKey,
['minify', 'transform'],
maxWorkers,
options.maxWorkers,
);

const {reporters} = options;
this._worker.getStdout().on('data', chunk => {
reporters.stdoutChunk(chunk.toString('utf8'));
});

this._worker.getStderr().on('data', chunk => {
reporters.stderrChunk(chunk.toString('utf8'));
});
} else {
// eslint-disable-next-line flow-no-fixme
// $FlowFixMe: Flow doesn't support dynamic requires
this._worker = require(workerPath);
}
}
Expand Down Expand Up @@ -101,6 +108,7 @@ module.exports = class Transformer {
options,
assetExts,
assetRegistryPath,
this._dynamicDepsInPackages,
);

debug('Done transforming file', filename);
Expand Down
25 changes: 24 additions & 1 deletion packages/metro/src/JSTransformer/worker/__tests__/worker-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('code transformation worker:', () => {
},
[],
'',
'reject',
);

expect(result.code).toBe(
Expand All @@ -60,6 +61,7 @@ describe('code transformation worker:', () => {
},
[],
'',
'reject',
);

expect(result.code).toBe(
Expand Down Expand Up @@ -91,6 +93,7 @@ describe('code transformation worker:', () => {
},
[],
'',
'reject',
);

expect(result.code).toBe(
Expand Down Expand Up @@ -130,11 +133,31 @@ describe('code transformation worker:', () => {
},
[],
'',
'reject',
);
throw new Error('should not reach this');
} catch (error) {
expect(error).toBeInstanceOf(InvalidRequireCallError);
if (!(error instanceof InvalidRequireCallError)) {
throw error;
}
expect(error.message).toMatchSnapshot();
}
});

it('supports dynamic dependencies from within `node_modules`', async () => {
await transformCode(
'/root/node_modules/bar/file.js',
`node_modules/bar/file.js`,
'require(global.something);\n',
path.join(__dirname, '../../../transformer.js'),
false,
{
dev: true,
transform: {},
},
[],
'',
'throwAtRuntime',
);
});
});
30 changes: 29 additions & 1 deletion packages/metro/src/JSTransformer/worker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import type {MetroSourceMapSegmentTuple} from 'metro-source-map';
import type {LocalPath} from '../../node-haste/lib/toLocalPath';
import type {ResultWithMap} from './minify';
import type {Ast, Plugins as BabelPlugins} from 'babel-core';
import type {DynamicRequiresBehavior} from '../../ModuleGraph/worker/collectDependencies';

export type TransformedCode = {
code: string,
Expand Down Expand Up @@ -97,6 +98,7 @@ function postTransform(
isScript: boolean,
options: Options,
transformFileStartLogEntry: LogEntry,
dynamicDepsInPackages: DynamicRequiresBehavior,
receivedAst: ?Ast,
): Data {
// Transformers can ouptut null ASTs (if they ignore the file). In that case
Expand Down Expand Up @@ -125,7 +127,13 @@ function postTransform(
} else {
let dependencyMapName;
try {
({dependencies, dependencyMapName} = collectDependencies(ast));
const opts = {
dynamicRequires: getDynamicDepsBehavior(
dynamicDepsInPackages,
filename,
),
};
({dependencies, dependencyMapName} = collectDependencies(ast, opts));
} catch (error) {
if (error instanceof collectDependencies.InvalidRequireCallError) {
throw new InvalidRequireCallError(error, filename);
Expand Down Expand Up @@ -162,6 +170,24 @@ function postTransform(
};
}

function getDynamicDepsBehavior(
inPackages: DynamicRequiresBehavior,
filename: string,
): DynamicRequiresBehavior {
switch (inPackages) {
case 'reject':
return 'reject';
case 'throwAtRuntime':
const isPackage = /(?:^|[/\\])node_modules[/\\]/.test(filename);
return isPackage ? inPackages : 'reject';
default:
(inPackages: empty);
throw new Error(
`invalid value for dynamic deps behavior: \`${inPackages}\``,
);
}
}

function transformCode(
filename: string,
localPath: LocalPath,
Expand All @@ -171,6 +197,7 @@ function transformCode(
options: Options,
assetExts: $ReadOnlyArray<string>,
assetRegistryPath: string,
dynamicDepsInPackages: DynamicRequiresBehavior,
): Data | Promise<Data> {
const isJson = filename.endsWith('.json');

Expand Down Expand Up @@ -216,6 +243,7 @@ function transformCode(
isScript,
options,
transformFileStartLogEntry,
dynamicDepsInPackages,
];

return transformResult instanceof Promise
Expand Down
Loading

0 comments on commit 46545d4

Please sign in to comment.