Skip to content

Commit

Permalink
pr feedback
Browse files Browse the repository at this point in the history
revert broken feedback

Updated comment

Added tests for HasteFS

Update HasteFS-test.js

Add contextModuleTemplates tests

fixup types

Update index.js

Drop getTransformFn

drop unused

Update types.flow.js

Added more tests

inputPath -> from

Test require.context

fixup
  • Loading branch information
EvanBacon committed Jul 19, 2022
1 parent f9a84b0 commit 7b78806
Show file tree
Hide file tree
Showing 21 changed files with 625 additions and 298 deletions.
8 changes: 6 additions & 2 deletions packages/metro-file-map/src/HasteFS.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,11 @@ export default class HasteFS {
return files;
}

/** Given a search context, return a list of file paths matching the query. */
/**
* 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<{
Expand All @@ -108,7 +112,7 @@ export default class HasteFS {
}

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

Expand Down
52 changes: 52 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,52 @@
/**
* 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
* @format
*/

import HasteFS from '../HasteFS';

describe('matchFilesWithContext', () => {
it(`matches files against context`, () => {
const hfs = new HasteFS({
rootDir: '/',
files: new Map([]),
});

// $FlowFixMe: mocking files
hfs.getAbsoluteFileIterator = function () {
return ['/foo/another.js', '/bar.js'];
};

// 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
19 changes: 11 additions & 8 deletions packages/metro-runtime/src/polyfills/require.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,16 +278,19 @@ function metroImportAll(moduleId: ModuleID | VerboseModuleNameForDev | number) {
}
metroRequire.importAll = metroImportAll;

if (__DEV__) {
// 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 = () => {
// 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 the project \`metro.config.js\`.`,
`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(
Expand Down
25 changes: 10 additions & 15 deletions packages/metro/src/DeltaBundler/DeltaCalculator.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@
import * as path from 'path';
import type {DeltaResult, Graph, Options} from './types.flow';

import {
removeContextQueryParam,
fileMatchesContext,
} from '../lib/contextModule';
import {ensureRequireContext, fileMatchesContext} from '../lib/contextModule';

const {
createGraph,
Expand Down Expand Up @@ -253,27 +250,25 @@ class DeltaCalculator<T> extends EventEmitter {
// to enable users to opt out.
if (this._options.unstable_allowRequireContext) {
const checkModifiedContextDependencies = (filePath: string) => {
this._graph.dependencies.forEach(value => {
this._graph.dependencies.forEach(dependency => {
const {contextParams} = dependency;
if (
value.contextParams &&
!modifiedDependencies.includes(value.path) &&
fileMatchesContext(
removeContextQueryParam(value.path),
filePath,
value.contextParams,
)
contextParams &&
contextParams.from != null &&
!modifiedDependencies.includes(dependency.path) &&
fileMatchesContext(filePath, ensureRequireContext(contextParams))
) {
modifiedDependencies.push(value.path);
modifiedDependencies.push(dependency.path);
}
});
};

// Check if any added or removed files are matched in a context module.
Array.from(addedFiles).forEach(filePath =>
addedFiles.forEach(filePath =>
checkModifiedContextDependencies(filePath),
);

Array.from(deletedFiles).forEach(filePath =>
deletedFiles.forEach(filePath =>
checkModifiedContextDependencies(filePath),
);
}
Expand Down
10 changes: 0 additions & 10 deletions packages/metro/src/DeltaBundler/Transformer.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

import type {TransformResult, TransformResultWithSource} from '../DeltaBundler';
import type {TransformerConfig, TransformOptions} from './Worker';
import {toRequireContext} from '../lib/contextModule';
import crypto from 'crypto';
import type {ConfigT} from 'metro-config/src/configTypes.flow';

Expand Down Expand Up @@ -142,15 +141,6 @@ class Transformer {
fileBuffer,
);

data.result.dependencies.forEach(dependency => {
if (dependency.data.contextParams) {
// Convert JSON regular expression into RegExp.
dependency.data.contextParams = toRequireContext(
dependency.data.contextParams,
);
}
});

// 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
// track of the cache that returned the result.
Expand Down
22 changes: 20 additions & 2 deletions packages/metro/src/DeltaBundler/Worker.flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,23 @@ type Data = $ReadOnly<{
transformFileEndLogEntry: LogEntry,
}>;

/**
* When the `Buffer` is sent over the worker thread it gets serialized into a JSON object.
* This helper method will deserialize it if needed.
*
* @returns `Buffer` representation of the JSON object.
* @returns `null` if the given object is nullish or not a serialized `Buffer` object.
*/
function asDeserializedBuffer(value: any): Buffer | null {
if (Buffer.isBuffer(value)) {
return value;
}
if (value && value.type === 'Buffer') {
return Buffer.from(value.data);
}
return null;
}

async function transform(
filename: string,
transformOptions: JsTransformOptions,
Expand All @@ -60,8 +77,9 @@ async function transform(
): Promise<Data> {
let data;

if (fileBuffer && fileBuffer.type === 'Buffer') {
data = Buffer.from(fileBuffer.data);
const fileBufferObject = asDeserializedBuffer(fileBuffer);
if (fileBufferObject) {
data = fileBufferObject;
} else {
data = fs.readFileSync(path.resolve(projectRoot, filename));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('DeltaCalculator', () => {
let fileWatcher;

const options = {
unstable_allowRequireContext: true,
experimentalImportBundleSupport: false,
onProgress: null,
resolve: (from: string, to: string) => {
Expand Down
43 changes: 19 additions & 24 deletions packages/metro/src/DeltaBundler/graphOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

'use strict';

import type {RequireContextParams} from '../ModuleGraph/worker/collectDependencies';
import type {
Dependency,
Graph,
Expand All @@ -43,7 +44,7 @@ import type {
import CountingSet from '../lib/CountingSet';
import {
appendContextQueryParam,
removeContextQueryParam,
ensureRequireContext,
} from '../lib/contextModule';

import * as path from 'path';
Expand Down Expand Up @@ -121,13 +122,11 @@ type InternalOptions<T> = $ReadOnly<{
onDependencyAdded: () => mixed,
resolve: Options<T>['resolve'],
transform: Options<T>['transform'],
transformContext: Options<T>['transformContext'],
shallow: boolean,
}>;

function getInternalOptions<T>({
transform,
transformContext,
resolve,
onProgress,
experimentalImportBundleSupport,
Expand All @@ -139,7 +138,6 @@ function getInternalOptions<T>({
return {
experimentalImportBundleSupport,
transform,
transformContext,
resolve,
onDependencyAdd: () => onProgress && onProgress(numProcessed, ++total),
onDependencyAdded: () => onProgress && onProgress(++numProcessed, total),
Expand Down Expand Up @@ -264,13 +262,7 @@ async function traverseDependenciesForSingleFile<T>(
): Promise<void> {
options.onDependencyAdd();

await processModule(
path,
graph,
delta,
options,
graph.dependencies.get(path)?.contextParams,
);
await processModule(path, graph, delta, options);

options.onDependencyAdded();
}
Expand All @@ -280,17 +272,20 @@ async function processModule<T>(
graph: Graph<T>,
delta: Delta,
options: InternalOptions<T>,
contextParams?: RequireContext,
// This fallback is used when a new dependency is added after the initial bundle has been created
// the invocation comes from `traverseDependenciesForSingleFile`.
contextParams:
| ?RequireContext
| RequireContextParams = graph.dependencies.get(path)?.contextParams,
): Promise<Module<T>> {
const resolvedContextParams =
contextParams || graph.dependencies.get(path)?.contextParams;

// Transform the file via the given option.
// TODO: Unbind the transform method from options
let result;
if (resolvedContextParams) {
const modulePath = removeContextQueryParam(path);
result = await options.transformContext(modulePath, resolvedContextParams);
if (contextParams != null) {
result = await options.transform(
nullthrows(contextParams.from),
ensureRequireContext(contextParams),
);
} else {
result = await options.transform(path);
}
Expand All @@ -313,7 +308,7 @@ async function processModule<T>(
// Update the module information.
const module = {
...previousModule,
contextParams: resolvedContextParams,
contextParams: contextParams,
dependencies: new Map(previousDependencies),
getSource: result.getSource,
output: result.output,
Expand Down Expand Up @@ -489,14 +484,14 @@ function resolveDependencies<T>(
let resolvedDep;

// `require.context`
if (dep.data.contextParams) {
let absolutePath = path.join(parentPath, '..', dep.name);
const {contextParams} = dep.data;
if (contextParams) {
contextParams.from = path.join(parentPath, '..', dep.name);

// Ensure the filepath has uniqueness applied to ensure multiple `require.context`
// statements can be used to target the same file with different properties.
absolutePath = appendContextQueryParam(
absolutePath,
dep.data.contextParams,
const absolutePath = appendContextQueryParam(
ensureRequireContext(contextParams),
);

resolvedDep = {
Expand Down
13 changes: 4 additions & 9 deletions packages/metro/src/DeltaBundler/types.flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import type {JsTransformOptions} from 'metro-transform-worker';
import CountingSet from '../lib/CountingSet';

export type RequireContext = {
/** Absolute file path pointing to the root directory of the context. */
from?: string,
/* Should search for files recursively. Optional, default `true` when `require.context` is used */
recursive: boolean,
/* Filename filter pattern for use in `require.context`. Optional, default `.*` (any file) when `require.context` is used */
Expand Down Expand Up @@ -120,15 +122,10 @@ export type TransformResultWithSource<T = MixedOutput> = $ReadOnly<{
getSource: () => Buffer,
}>;

/** Transformer for generating `require.context` virtual module. */
export type TransformContextFn<T = MixedOutput> = (
export type TransformFn<T = MixedOutput> = (
string,
RequireContext,
?RequireContext,
) => Promise<TransformResultWithSource<T>>;

export type TransformFn<T = MixedOutput> = string => Promise<
TransformResultWithSource<T>,
>;
export type AllowOptionalDependenciesWithOptions = {
+exclude: Array<string>,
};
Expand All @@ -139,8 +136,6 @@ export type AllowOptionalDependencies =
export type Options<T = MixedOutput> = {
+resolve: (from: string, to: string, context?: ?RequireContext) => string,
+transform: TransformFn<T>,
/** Given a path and require context, return a virtual context module. */
+transformContext: TransformContextFn<T>,
+transformOptions: TransformInputOptions,
+onProgress: ?(numProcessed: number, total: number) => mixed,
+experimentalImportBundleSupport: boolean,
Expand Down

0 comments on commit 7b78806

Please sign in to comment.