Skip to content

Commit

Permalink
Add configuration option to suppress require cycle warnings (#707)
Browse files Browse the repository at this point in the history 
Summary:
In many projects there are require cycles in the `node_modules` dependencies that are outside the developer's control, or cycles in first-party code that are known not to be problematic.

This adds an option to selectively suppress require cycle warnings, and defaults to suppressing any cycle within `node_modules`.

Closes #28

Pull Request resolved: #707

Test Plan:
See D37157514

**Static Docs Preview: metro**
|[Full Site](https://our.intern.facebook.com/intern/staticdocs/eph/1655421690/metro/)|

|**Modified Pages**|
|[docs/configuration](https://our.intern.facebook.com/intern/staticdocs/eph/1655421690/metro/docs/configuration/)||[docs/configuration](https://our.intern.facebook.com/intern/staticdocs/eph/D36778288/V10/metro/docs/configuration/)||[docs/configuration](https://our.intern.facebook.com/intern/staticdocs/eph/1655419713/metro/docs/configuration/)|

Reviewed By: huntie

Differential Revision: D36778288

Pulled By: robhogan

fbshipit-source-id: b2588c1f3f74175af3ccb05ed7b0efe36f0eebb5
  • Loading branch information
@hsource @facebook-github-bot
hsource authored and facebook-github-bot committed Jun 17, 2022
1 parent 7007e48 commit 100ca6f
Show file tree
Hide file tree
Showing 9 changed files with 179 additions and 16 deletions.
10 changes: 10 additions & 0 deletions docs/Configuration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,16 @@ Type: `Array<string>`

Additional platforms to look out for, For example, if you want to add a "custom" platform, and use modules ending in .custom.js, you would return ['custom'] here.

#### `requireCycleIgnorePatterns`

Type: `Array<RegExp>`

In development mode, suppress require cycle warnings for any cycle involving a module that matches any of these expressions. This is useful for third-party code and first-party expected cycles.

Note that if you specify your own value for this config option it will replace (not concatenate with) Metro's default.

Defaults to `[/(^|\/|\\)node_modules($|\/|\\)/]`.

---
### Transformer Options

Expand Down
12 changes: 12 additions & 0 deletions packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ Object {
"windows",
"web",
],
"requireCycleIgnorePatterns": Array [
/\\(\\^\\|\\\\/\\|\\\\\\\\\\)node_modules\\(\\$\\|\\\\/\\|\\\\\\\\\\)/,
],
"resolveRequest": null,
"resolverMainFields": Array [
"browser",
Expand Down Expand Up @@ -211,6 +214,9 @@ Object {
"windows",
"web",
],
"requireCycleIgnorePatterns": Array [
/\\(\\^\\|\\\\/\\|\\\\\\\\\\)node_modules\\(\\$\\|\\\\/\\|\\\\\\\\\\)/,
],
"resolveRequest": null,
"resolverMainFields": Array [
"browser",
Expand Down Expand Up @@ -364,6 +370,9 @@ Object {
"windows",
"web",
],
"requireCycleIgnorePatterns": Array [
/\\(\\^\\|\\\\/\\|\\\\\\\\\\)node_modules\\(\\$\\|\\\\/\\|\\\\\\\\\\)/,
],
"resolveRequest": null,
"resolverMainFields": Array [
"browser",
Expand Down Expand Up @@ -517,6 +526,9 @@ Object {
"windows",
"web",
],
"requireCycleIgnorePatterns": Array [
/\\(\\^\\|\\\\/\\|\\\\\\\\\\)node_modules\\(\\$\\|\\\\/\\|\\\\\\\\\\)/,
],
"resolveRequest": null,
"resolverMainFields": Array [
"browser",
Expand Down
1 change: 1 addition & 0 deletions packages/metro-config/src/configTypes.flow.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ type ResolverConfigT = {
resolverMainFields: $ReadOnlyArray<string>,
sourceExts: $ReadOnlyArray<string>,
useWatchman: boolean,
requireCycleIgnorePatterns: $ReadOnlyArray<RegExp>,
};

type SerializerConfigT = {
Expand Down
1 change: 1 addition & 0 deletions packages/metro-config/src/defaults/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const getDefaultValues = (projectRoot: ?string): ConfigT => ({
resolveRequest: null,
resolverMainFields: ['browser', 'main'],
useWatchman: true,
requireCycleIgnorePatterns: [/(^|\/|\\)node_modules($|\/|\\)/],
},

serializer: {
Expand Down
53 changes: 52 additions & 1 deletion packages/metro-runtime/src/polyfills/__tests__/require-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,14 @@ function createModule(
verboseName,
factory,
dependencyMap = [],
globalPrefix = '',
) {
moduleSystem.__d(factory, moduleId, dependencyMap, verboseName);
moduleSystem[globalPrefix + '__d'](
factory,
moduleId,
dependencyMap,
verboseName,
);
}

describe('require', () => {
Expand Down Expand Up @@ -665,6 +671,51 @@ describe('require', () => {
console.warn = warn;
});

it('does not log warning for cyclic dependency in ignore list', () => {
moduleSystem.__customPrefix__requireCycleIgnorePatterns = [/foo/];
createModuleSystem(moduleSystem, true, '__customPrefix');

createModule(
moduleSystem,
0,
'foo.js',
(global, require) => {
require(1);
},
[],
'__customPrefix',
);

createModule(
moduleSystem,
1,
'bar.js',
(global, require) => {
require(2);
},
[],
'__customPrefix',
);

createModule(
moduleSystem,
2,
'baz.js',
(global, require) => {
require(0);
},
[],
'__customPrefix',
);

const warn = console.warn;
console.warn = jest.fn();

moduleSystem.__r(0);
expect(console.warn).toHaveBeenCalledTimes(0);
console.warn = warn;
});

it('sets the exports value to their current value', () => {
createModuleSystem(moduleSystem, false, '');

Expand Down
32 changes: 25 additions & 7 deletions packages/metro-runtime/src/polyfills/require.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,15 @@ function metroRequire(moduleId: ModuleID | VerboseModuleNameForDev): Exports {
.map((id: number) =>
modules[id] ? modules[id].verboseName : '[unknown]',
);
// We want to show A -> B -> A:
cycle.push(cycle[0]);
console.warn(
`Require cycle: ${cycle.join(' -> ')}\n\n` +
'Require cycles are allowed, but can result in uninitialized values. ' +
'Consider refactoring to remove the need for a cycle.',
);

if (shouldPrintRequireCycle(cycle)) {
cycle.push(cycle[0]); // We want to print A -> B -> A:
console.warn(
`Require cycle: ${cycle.join(' -> ')}\n\n` +
'Require cycles are allowed, but can result in uninitialized values. ' +
'Consider refactoring to remove the need for a cycle.',
);
}
}
}

Expand All @@ -194,6 +196,22 @@ function metroRequire(moduleId: ModuleID | VerboseModuleNameForDev): Exports {
: guardedLoadModule(moduleIdReallyIsNumber, module);
}

// We print require cycles unless they match a pattern in the
// `requireCycleIgnorePatterns` configuration.
function shouldPrintRequireCycle(modules: $ReadOnlyArray<?string>): boolean {
const regExps =
global[__METRO_GLOBAL_PREFIX__ + '__requireCycleIgnorePatterns'];
if (!Array.isArray(regExps)) {
return true;
}

const isIgnored = module =>
module != null && regExps.some(regExp => regExp.test(module));

// Print the cycle unless any part of it is ignored
return modules.every(module => !isIgnored(module));
}

function metroImportDefault(moduleId: ModuleID | VerboseModuleNameForDev) {
if (__DEV__ && typeof moduleId === 'string') {
const verboseName = moduleId;
Expand Down
62 changes: 55 additions & 7 deletions packages/metro/src/lib/__tests__/getPreludeCode-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@ const vm = require('vm');
['development', 'production'].forEach((mode: string) => {
describe(`${mode} mode`, () => {
const isDev = mode === 'development';
const globalPrefix = '';
const globalPrefix = '__metro';
const requireCycleIgnorePatterns = [];

it('sets up `process.env.NODE_ENV` and `__DEV__`', () => {
const sandbox: $FlowFixMe = {};
vm.createContext(sandbox);
vm.runInContext(getPreludeCode({isDev, globalPrefix}), sandbox);
vm.runInContext(
getPreludeCode({isDev, globalPrefix, requireCycleIgnorePatterns}),
sandbox,
);
expect(sandbox.process.env.NODE_ENV).toEqual(mode);
expect(sandbox.__DEV__).toEqual(isDev);
});
Expand All @@ -31,17 +35,51 @@ const vm = require('vm');
const sandbox: $FlowFixMe = {};
vm.createContext(sandbox);
vm.runInContext(
getPreludeCode({isDev, globalPrefix: '__metro'}),
getPreludeCode({
isDev,
globalPrefix: '__customPrefix',
requireCycleIgnorePatterns,
}),
sandbox,
);
expect(sandbox.__METRO_GLOBAL_PREFIX__).toBe('__customPrefix');
});

it('sets up `${globalPrefix}__requireCycleIgnorePatterns` in development', () => {
const sandbox: $FlowFixMe = {};
vm.createContext(sandbox);
vm.runInContext(
getPreludeCode({
isDev,
globalPrefix,
requireCycleIgnorePatterns: [
/blah/,
/(^|\/|\\)node_modules($|\/|\\)/,
],
}),
sandbox,
);
expect(sandbox.__METRO_GLOBAL_PREFIX__).toBe('__metro');

if (isDev) {
expect(sandbox[`${globalPrefix}__requireCycleIgnorePatterns`]).toEqual([
/blah/,
/(^|\/|\\)node_modules($|\/|\\)/,
]);
} else {
expect(
sandbox[`${globalPrefix}__requireCycleIgnorePatterns`],
).not.toBeDefined();
}
});

it('does not override an existing `process.env`', () => {
const nextTick = () => {};
const sandbox: $FlowFixMe = {process: {nextTick, env: {FOOBAR: 123}}};
vm.createContext(sandbox);
vm.runInContext(getPreludeCode({isDev, globalPrefix}), sandbox);
vm.runInContext(
getPreludeCode({isDev, globalPrefix, requireCycleIgnorePatterns}),
sandbox,
);
expect(sandbox.process.env.NODE_ENV).toEqual(mode);
expect(sandbox.process.env.FOOBAR).toEqual(123);
expect(sandbox.process.nextTick).toEqual(nextTick);
Expand All @@ -53,7 +91,12 @@ const vm = require('vm');
const BAR = 2;
vm.createContext(sandbox);
vm.runInContext(
getPreludeCode({isDev, globalPrefix, extraVars: {FOO, BAR}}),
getPreludeCode({
isDev,
globalPrefix,
requireCycleIgnorePatterns,
extraVars: {FOO, BAR},
}),
sandbox,
);
expect(sandbox.FOO).toBe(FOO);
Expand All @@ -64,7 +107,12 @@ const vm = require('vm');
const sandbox: $FlowFixMe = {};
vm.createContext(sandbox);
vm.runInContext(
getPreludeCode({isDev, globalPrefix, extraVars: {__DEV__: 123}}),
getPreludeCode({
isDev,
globalPrefix,
requireCycleIgnorePatterns,
extraVars: {__DEV__: 123},
}),
sandbox,
);
expect(sandbox.__DEV__).toBe(isDev);
Expand Down
15 changes: 15 additions & 0 deletions packages/metro/src/lib/getPreludeCode.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,33 @@ function getPreludeCode({
extraVars,
isDev,
globalPrefix,
requireCycleIgnorePatterns,
}: {
+extraVars?: {[string]: mixed, ...},
+isDev: boolean,
+globalPrefix: string,
+requireCycleIgnorePatterns: $ReadOnlyArray<RegExp>,
}): string {
const vars = [
// Ensure these variable names match the ones referenced in metro-runtime
// require.js
'__BUNDLE_START_TIME__=this.nativePerformanceNow?nativePerformanceNow():Date.now()',
`__DEV__=${String(isDev)}`,
...formatExtraVars(extraVars),
'process=this.process||{}',
`__METRO_GLOBAL_PREFIX__='${globalPrefix}'`,
];

if (isDev) {
// Ensure these variable names match the ones referenced in metro-runtime
// require.js
vars.push(
`${globalPrefix}__requireCycleIgnorePatterns=[${requireCycleIgnorePatterns
.map(regex => regex.toString())
.join(',')}]`,
);
}

return `var ${vars.join(',')};${processEnv(
isDev ? 'development' : 'production',
)}`;
Expand Down
9 changes: 8 additions & 1 deletion packages/metro/src/lib/getPrependedScripts.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ async function getPrependedScripts(
_getPrelude({
dev: options.dev,
globalPrefix: config.transformer.globalPrefix,
requireCycleIgnorePatterns: config.resolver.requireCycleIgnorePatterns,
}),
...dependencies.values(),
];
Expand All @@ -79,12 +80,18 @@ async function getPrependedScripts(
function _getPrelude({
dev,
globalPrefix,
requireCycleIgnorePatterns,
}: {
dev: boolean,
globalPrefix: string,
requireCycleIgnorePatterns: $ReadOnlyArray<RegExp>,
...
}): Module<> {
const code = getPreludeCode({isDev: dev, globalPrefix});
const code = getPreludeCode({
isDev: dev,
globalPrefix,
requireCycleIgnorePatterns,
});
const name = '__prelude__';

return {
Expand Down

0 comments on commit 100ca6f

Please sign in to comment.