Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add require.context support #822

Closed
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
d37ca65
feat: add `resolveContext` method for matching files
EvanBacon May 28, 2022
faf5df7
Apply suggestions from code review
EvanBacon May 28, 2022
87bf0af
Update HasteFS.js
EvanBacon May 28, 2022
0d59e7f
Update HasteFS.js
EvanBacon Jun 22, 2022
5852acb
Add require context changes
EvanBacon Jun 22, 2022
12fcb4c
Merge branch 'main' into @evanbacon/require-context/resolve-file-paths
EvanBacon Jun 22, 2022
360ee19
Add all require.context changes
EvanBacon Jun 22, 2022
f9a84b0
Merge branch 'main' into @evanbacon/require-context/resolve-file-paths
EvanBacon Jul 19, 2022
7b78806
pr feedback
EvanBacon Jul 19, 2022
1b807f3
Move buffer sha upstream
EvanBacon Jul 19, 2022
797a92c
fix types
EvanBacon Jul 19, 2022
6f795ac
fix lint
EvanBacon Jul 19, 2022
0cd6a1f
normalize matching file patterns
EvanBacon Jul 19, 2022
b8336fd
fix lint
EvanBacon Jul 19, 2022
645443b
revert changes
EvanBacon Jul 19, 2022
6e25851
fix tests
EvanBacon Jul 19, 2022
ec0ac71
move sha1 back up
EvanBacon Jul 22, 2022
2b59675
rename function
EvanBacon Jul 22, 2022
182b4e8
Update Transformer-test.js
EvanBacon Jul 25, 2022
2baf25d
restructure to use privateState
EvanBacon Jul 25, 2022
5f110e6
added tests
EvanBacon Jul 27, 2022
063e99a
Update traverseDependencies-test.js
EvanBacon Jul 27, 2022
4b965fb
resolvedContexts
EvanBacon Aug 9, 2022
56c6a66
Update graphOperations.js
EvanBacon Aug 9, 2022
9d469a5
Overhaul traverseDependencies tests, some API/behaviour changes
motiz88 Aug 9, 2022
b88384a
Merge branch '@evanbacon/require-context/resolve-file-paths' of https…
motiz88 Aug 9, 2022
c66d112
Fix getTransformFn after absolute path change
motiz88 Aug 9, 2022
7e4308e
Add require.context integration test
motiz88 Aug 9, 2022
0795f6c
Make order of keys in context module deterministic
motiz88 Aug 9, 2022
ec46ce0
Add integration test for require and context with the same first arg
motiz88 Aug 9, 2022
2550dc8
Remove contextParams from Module type and processModule args
motiz88 Aug 9, 2022
d96a3bc
Tighten incremental edge cases + add more tests
motiz88 Aug 9, 2022
e05634f
Fix order sensitive test snapshot
motiz88 Aug 9, 2022
a7dd372
Add some line breaks to context module template
motiz88 Aug 9, 2022
7600f9e
Merge remote-tracking branch 'upstream/main' into @evanbacon/require-…
motiz88 Aug 10, 2022
6bf2b5a
Fix up types post Flow upgrade
motiz88 Aug 10, 2022
4aae435
id --> debugId, add tests
motiz88 Aug 10, 2022
2172d42
Remove debugId, getContextModuleId
motiz88 Aug 10, 2022
612562c
Rewrite DeltaCalculator context tests and fix bugs
motiz88 Aug 10, 2022
6404eb0
Light cleanup + file header fixes
motiz88 Aug 10, 2022
6833b44
Update HasteFS.js
EvanBacon Aug 15, 2022
49b97a3
PR Feedback
EvanBacon Aug 15, 2022
fbaf8bb
Merge branch 'main' into @evanbacon/require-context/resolve-file-paths
motiz88 Aug 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 56 additions & 2 deletions packages/metro-file-map/src/HasteFS.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
* @flow strict-local
*/

import type {FileMetaData} from './flow-types';
import type {FileData, Path} from './flow-types';
import type {FileData, FileMetaData, Path} from './flow-types';

import H from './constants';
import * as fastPath from './lib/fast_path';
import * as path from 'path';
// $FlowFixMe[untyped-import] - jest-util
import {globsToMatcher, replacePathSepForGlob} from 'jest-util';

Expand Down Expand Up @@ -85,6 +85,53 @@ export default class HasteFS {
return files;
}

/**
* Given a search context, return a list of file paths matching the query.
* The query matches against normalized paths which start with `./`,
* for example: `a/b.js` -> `./a/b.js`
*/
matchFilesWithContext(
root: Path,
context: $ReadOnly<{
/* Should search for files recursively. */
recursive: boolean,
/* Filter relative paths against a pattern. */
filter: RegExp,
}>,
): Array<Path> {
const files = [];
const prefix = './';

for (const file of this.getAbsoluteFileIterator()) {
const filePath = fastPath.relative(root, file);

const isRelative =
EvanBacon marked this conversation as resolved.
Show resolved Hide resolved
filePath && !filePath.startsWith('..') && !path.isAbsolute(filePath);
Copy link
Contributor

@robhogan robhogan Aug 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment:

filePath.startsWith('..') assumes normalisation, which seems to be safe in practice but I can't see it explicitly mentioned in the Node JS docs for relative (as it is for join, for example).

I wondered about the necessity of the isAbsolute check too, given we've obtained this path using relative(), but I guess that's for Windows, eg for two files on different drives?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isAbsolute check appears to be extraneous now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure normalization is required for the the .. check to work as this paradigm is cross-platform.

Copy link
Contributor

@robhogan robhogan Aug 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure normalization is required for the the .. check to work as this paradigm is cross-platform.

If it's not normalised it might not appear at the start of the path - eg foo/../../bar is valid but normalises to ../bar

Copy link
Contributor

@robhogan robhogan Aug 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isAbsolute check appears to be extraneous now.

I think it's still necessary for Windows tbh, looks like we get absolute paths back when there's no valid relative path between locations:

node -e "console.log(path.win32.relative('C:/foo', 'D:/bar'))";
D:\bar

// Ignore everything outside of the provided `root`.
if (!isRelative) {
continue;
}

// Prevent searching in child directories during a non-recursive search.
if (!context.recursive && filePath.includes(path.sep)) {
continue;
}

if (
context.filter.test(
// NOTE(EvanBacon): Ensure files start with `./` for matching purposes
EvanBacon marked this conversation as resolved.
Show resolved Hide resolved
// this ensures packages work across Metro and Webpack (ex: Storybook for React DOM / React Native).
// `a/b.js` -> `./a/b.js`
prefix + normalizeSlashes(filePath),
)
) {
files.push(file);
}
}

return files;
}

matchFilesWithGlob(globs: $ReadOnlyArray<Glob>, root: ?Path): Set<Path> {
const files = new Set<string>();
const matcher = globsToMatcher(globs);
Expand All @@ -103,3 +150,10 @@ export default class HasteFS {
return this._files.get(relativePath);
}
}

function normalizeSlashes(path: string): string {
if (/^\\\\\?\\/.test(path) || /[^\u0000-\u0080]+/.test(path)) {
EvanBacon marked this conversation as resolved.
Show resolved Hide resolved
return path;
}
return path.replace(/\\/g, '/');
}
63 changes: 63 additions & 0 deletions packages/metro-file-map/src/__tests__/HasteFS-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict-local
* @format
*/

import HasteFS from '../HasteFS';

jest.mock('../lib/fast_path', () => ({
resolve: (a, b) => b,
relative: jest.requireActual('path').relative,
}));

describe('matchFilesWithContext', () => {
it(`matches files against context`, () => {
const hfs = new HasteFS({
rootDir: '/',
files: new Map([
[
'/foo/another.js',
// $FlowFixMe: mocking files
{},
],
[
'/bar.js',
// $FlowFixMe: mocking files
{},
],
]),
});

// Test non-recursive skipping deep paths
expect(
hfs.matchFilesWithContext('/', {
filter: new RegExp(
// Test starting with `./` since this is mandatory for parity with Webpack.
/^\.\/.*/,
),
recursive: false,
}),
).toEqual(['/bar.js']);

// Test inner directory
expect(
hfs.matchFilesWithContext('/foo', {
filter: new RegExp(/.*/),
recursive: true,
}),
).toEqual(['/foo/another.js']);

// Test recursive
expect(
hfs.matchFilesWithContext('/', {
filter: new RegExp(/.*/),
recursive: true,
}),
).toEqual(['/foo/another.js', '/bar.js']);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,13 @@ describe('require', () => {
expect(fn.mock.calls.length).toBe(1);
});

it('throws when using require.context directly', () => {
createModuleSystem(moduleSystem, false, '');
expect(() => moduleSystem.__r.context('foobar')).toThrow(
'The experimental Metro feature `require.context` is not enabled in your project.',
);
});

it('throws an error when trying to require an unknown module', () => {
createModuleSystem(moduleSystem, false, '');

Expand Down
14 changes: 14 additions & 0 deletions packages/metro-runtime/src/polyfills/require.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,20 @@ function metroImportAll(moduleId: ModuleID | VerboseModuleNameForDev | number) {
}
metroRequire.importAll = metroImportAll;

// The `require.context()` syntax is never executed in the runtime because it is converted
// to `require()` in `metro/src/ModuleGraph/worker/collectDependencies.js` after collecting
// dependencies. If the feature flag is not enabled then the conversion never takes place and this error is thrown (development only).
metroRequire.context = function fallbackRequireContext() {
if (__DEV__) {
throw new Error(
'The experimental Metro feature `require.context` is not enabled in your project.\nThis can be enabled by setting the `transformer.unstable_allowRequireContext` property to `true` in your Metro configuration.',
);
}
throw new Error(
'The experimental Metro feature `require.context` is not enabled in your project.',
);
};

let inGuard = false;
function guardedLoadModule(
moduleId: ModuleID,
Expand Down
8 changes: 7 additions & 1 deletion packages/metro/src/Bundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,18 @@ class Bundler {
async transformFile(
filePath: string,
transformOptions: TransformOptions,
/** Optionally provide the file contents, this can be used to provide virtual contents for a file. */
fileBuffer?: Buffer,
EvanBacon marked this conversation as resolved.
Show resolved Hide resolved
): Promise<TransformResultWithSource<>> {
// We need to be sure that the DependencyGraph has been initialized.
// TODO: Remove this ugly hack!
await this._depGraph.ready();

return this._transformer.transformFile(filePath, transformOptions);
return this._transformer.transformFile(
filePath,
transformOptions,
fileBuffer,
);
}

// Waits for the bundler to become ready.
Expand Down
69 changes: 61 additions & 8 deletions packages/metro/src/DeltaBundler/DeltaCalculator.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@

'use strict';

import type {DeltaResult, Graph, Options} from './types.flow';

const {
import {markModifiedContextModules} from './graphOperations';
import {
createGraph,
initialTraverseDependencies,
reorderGraph,
EvanBacon marked this conversation as resolved.
Show resolved Hide resolved
traverseDependencies,
} = require('./graphOperations');
} from './graphOperations';
import type {DeltaResult, Graph, Options} from './types.flow';

const {EventEmitter} = require('events');

/**
Expand All @@ -33,6 +34,7 @@ class DeltaCalculator<T> extends EventEmitter {
_currentBuildPromise: ?Promise<DeltaResult<T>>;
_deletedFiles: Set<string> = new Set();
_modifiedFiles: Set<string> = new Set();
_addedFiles: Set<string> = new Set();

_graph: Graph<T>;

Expand Down Expand Up @@ -72,6 +74,7 @@ class DeltaCalculator<T> extends EventEmitter {
});
this._modifiedFiles = new Set();
this._deletedFiles = new Set();
this._addedFiles = new Set();
}

/**
Expand Down Expand Up @@ -99,13 +102,16 @@ class DeltaCalculator<T> extends EventEmitter {
this._modifiedFiles = new Set();
const deletedFiles = this._deletedFiles;
this._deletedFiles = new Set();
const addedFiles = this._addedFiles;
this._addedFiles = new Set();

// Concurrent requests should reuse the same bundling process. To do so,
// this method stores the promise as an instance variable, and then it's
// removed after it gets resolved.
this._currentBuildPromise = this._getChangedDependencies(
modifiedFiles,
deletedFiles,
addedFiles,
);

let result;
Expand All @@ -121,6 +127,7 @@ class DeltaCalculator<T> extends EventEmitter {
// which is not correct.
modifiedFiles.forEach((file: string) => this._modifiedFiles.add(file));
deletedFiles.forEach((file: string) => this._deletedFiles.add(file));
addedFiles.forEach((file: string) => this._addedFiles.add(file));

// If after an error the number of modules has changed, we could be in
// a weird state. As a safe net we clean the dependency modules to force
Expand Down Expand Up @@ -177,12 +184,45 @@ class DeltaCalculator<T> extends EventEmitter {
filePath: string,
...
}): mixed => {
let state: void | 'deleted' | 'modified' | 'added';
if (this._deletedFiles.has(filePath)) {
state = 'deleted';
} else if (this._modifiedFiles.has(filePath)) {
state = 'modified';
} else if (this._addedFiles.has(filePath)) {
state = 'added';
}

let nextState: 'deleted' | 'modified' | 'added';
if (type === 'delete') {
this._deletedFiles.add(filePath);
this._modifiedFiles.delete(filePath);
nextState = 'deleted';
} else if (type === 'add') {
EvanBacon marked this conversation as resolved.
Show resolved Hide resolved
motiz88 marked this conversation as resolved.
Show resolved Hide resolved
// A deleted+added file is modified
nextState = state === 'deleted' ? 'modified' : 'added';
} else {
this._deletedFiles.delete(filePath);
this._modifiedFiles.add(filePath);
// type === 'change'
// An added+modified file is added
nextState = state === 'added' ? 'added' : 'modified';
}

switch (nextState) {
case 'deleted':
this._deletedFiles.add(filePath);
this._modifiedFiles.delete(filePath);
this._addedFiles.delete(filePath);
break;
case 'added':
this._addedFiles.add(filePath);
this._deletedFiles.delete(filePath);
this._modifiedFiles.delete(filePath);
break;
case 'modified':
this._modifiedFiles.add(filePath);
this._deletedFiles.delete(filePath);
this._addedFiles.delete(filePath);
break;
default:
(nextState: empty);
}

// Notify users that there is a change in some of the bundle files. This
Expand All @@ -193,6 +233,7 @@ class DeltaCalculator<T> extends EventEmitter {
async _getChangedDependencies(
modifiedFiles: Set<string>,
deletedFiles: Set<string>,
addedFiles: Set<string>,
): Promise<DeltaResult<T>> {
if (!this._graph.dependencies.size) {
const {added} = await initialTraverseDependencies(
Expand Down Expand Up @@ -224,6 +265,18 @@ class DeltaCalculator<T> extends EventEmitter {
}
});

// NOTE(EvanBacon): This check adds extra complexity so we feature gate it
// to enable users to opt out.
if (this._options.unstable_allowRequireContext) {
// Check if any added or removed files are matched in a context module.
// We only need to do this for added files because (1) deleted files will have a context
// module as an inverse dependency, (2) modified files don't invalidate the contents
// of the context module.
addedFiles.forEach(filePath => {
markModifiedContextModules(this._graph, filePath, modifiedFiles);
});
}

// We only want to process files that are in the bundle.
const modifiedDependencies = Array.from(modifiedFiles).filter(
(filePath: string) => this._graph.dependencies.has(filePath),
Expand Down
20 changes: 18 additions & 2 deletions packages/metro/src/DeltaBundler/Transformer.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import type {TransformResult, TransformResultWithSource} from '../DeltaBundler';
import type {TransformerConfig, TransformOptions} from './Worker';
import type {ConfigT} from 'metro-config/src/configTypes.flow';
import crypto from 'crypto';

const getTransformCacheKey = require('./getTransformCacheKey');
const WorkerFarm = require('./WorkerFarm');
Expand Down Expand Up @@ -66,6 +67,7 @@ class Transformer {
async transformFile(
filePath: string,
transformerOptions: TransformOptions,
fileBuffer?: Buffer,
): Promise<TransformResultWithSource<>> {
const cache = this._cache;

Expand Down Expand Up @@ -119,15 +121,26 @@ class Transformer {
unstable_transformProfile,
]);

const sha1 = this._getSha1(filePath);
let sha1: string;
if (fileBuffer) {
// Shortcut for virtual modules which provide the contents with the filename.
sha1 = crypto.createHash('sha1').update(fileBuffer).digest('hex');
} else {
sha1 = this._getSha1(filePath);
}

let fullKey = Buffer.concat([partialKey, Buffer.from(sha1, 'hex')]);
const result = await cache.get(fullKey);

// A valid result from the cache is used directly; otherwise we call into
// the transformer to computed the corresponding result.
const data = result
? {result, sha1}
: await this._workerFarm.transform(localPath, transformerOptions);
: await this._workerFarm.transform(
localPath,
transformerOptions,
fileBuffer,
);

// Only re-compute the full key if the SHA-1 changed. This is because
// references are used by the cache implementation in a weak map to keep
Expand All @@ -141,6 +154,9 @@ class Transformer {
return {
...data.result,
getSource(): Buffer {
if (fileBuffer) {
return fileBuffer;
}
return fs.readFileSync(filePath);
},
};
Expand Down