Skip to content

Commit

Permalink
Add watcher.additionalExts option, enable inclusion of .cjs and .mjs …
Browse files Browse the repository at this point in the history
…files by default

Summary:
These changes bring spec-compliant support for `.cjs` and `.mjs` module file extensions to Metro. Resolves #535 and replaces D34168944.

- Adds a new `watcher.additionalExts` config option to pass additional extensions to the file watcher. This enables Metro to include these in the file map separately from `resolver.sourceExts`, with the effect being to enable new module types that require an explicit file extension when `require`/`import`ed.
- Sets the default value for  `watcher.additionalExts` to enable explicit resolution of `.cjs` and `.mjs` files by default.

Changelog:

**‌[Breaking]** Add `watcher.additionalExts` option, enable inclusion of `.cjs` and `.mjs` files by default

(In edge cases, e.g. when both `foo.cjs` and `foo.cjs.js` exist, this changes ordering.)

Reviewed By: motiz88

Differential Revision: D37927392

fbshipit-source-id: f71a704dafb720ce38387e222b808ebd9d56b3f4
  • Loading branch information
huntie authored and facebook-github-bot committed Jul 28, 2022
1 parent a3dc30a commit c1c6d9c
Show file tree
Hide file tree
Showing 9 changed files with 169 additions and 12 deletions.
19 changes: 18 additions & 1 deletion docs/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ An array of asset extensions to include in the bundle. For example, if you would

Type: `Array<string>`

An array of source extensions to include in the bundle. For example, if you would give `['ts']` you would be able to include `.ts` files in the bundle.
The list of source file extensions to include in the bundle. For example, including `'ts'` allows Metro to include `.ts` files in the bundle.

The order of these extensions defines the order to match files on disk. For more info, see [Module Resolution](https://facebook.github.io/metro/docs/resolution).

Defaults to `['js', 'jsx', 'json', 'ts', 'tsx']`.

#### `resolverMainFields`

Expand Down Expand Up @@ -401,6 +405,19 @@ Dot notation in this section indicates a nested configuration object, e.g. `watc

:::

#### `additionalExts`

Type: `Array<string>`

The extensions which Metro should watch in addition to `sourceExts`, but which will not be automatically tried by the resolver.

Therefore, the two behaviour differences from `resolver.sourceExts` when importing a module are:

- Modules can only be required when fully specified (e.g. `import moduleA from 'moduleA.mjs'`).
- No platform-specific resolution is performed.

Defaults to `['cjs', 'mjs']`.

#### `watchman.deferStates`

Type: `Array<string>`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ Object {
"/",
],
"watcher": Object {
"additionalExts": Array [
"cjs",
"mjs",
],
"watchman": Object {
"deferStates": Array [
"hg.update",
Expand Down Expand Up @@ -305,6 +309,10 @@ Object {
"/",
],
"watcher": Object {
"additionalExts": Array [
"cjs",
"mjs",
],
"watchman": Object {
"deferStates": Array [
"hg.update",
Expand Down Expand Up @@ -462,6 +470,10 @@ Object {
"/",
],
"watcher": Object {
"additionalExts": Array [
"cjs",
"mjs",
],
"watchman": Object {
"deferStates": Array [
"hg.update",
Expand Down Expand Up @@ -619,6 +631,10 @@ Object {
"/",
],
"watcher": Object {
"additionalExts": Array [
"cjs",
"mjs",
],
"watchman": Object {
"deferStates": Array [
"hg.update",
Expand Down
3 changes: 2 additions & 1 deletion packages/metro-config/src/configTypes.flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,9 @@ type SymbolicatorConfigT = {
};

type WatcherConfigT = {
additionalExts: $ReadOnlyArray<string>,
watchman: {
deferStates?: $ReadOnlyArray<string>,
deferStates: $ReadOnlyArray<string>,
},
};

Expand Down
2 changes: 2 additions & 0 deletions packages/metro-config/src/defaults/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ exports.assetResolutions = ['1', '1.5', '2', '3', '4'];

exports.sourceExts = ['js', 'jsx', 'json', 'ts', 'tsx'];

exports.additionalExts = ['cjs', 'mjs'];

exports.moduleSystem = (require.resolve(
'metro-runtime/src/polyfills/require.js',
): string);
Expand Down
2 changes: 2 additions & 0 deletions packages/metro-config/src/defaults/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const {
DEFAULT_METRO_MINIFIER_PATH,
assetExts,
assetResolutions,
additionalExts,
defaultCreateModuleIdFactory,
platforms,
sourceExts,
Expand Down Expand Up @@ -129,6 +130,7 @@ const getDefaultValues = (projectRoot: ?string): ConfigT => ({
unstable_compactOutput: false,
},
watcher: {
additionalExts,
watchman: {
deferStates: ['hg.update'],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ None of these files exist:
3 | import bar from 'foo';"
`;

exports[`linux relative paths fails when trying to require a non supported extension 1`] = `
exports[`linux relative paths fails when trying to implicitly require an extension not listed in sourceExts 1`] = `
"Unable to resolve module ./a.another from /root/index.js:
None of these files exist:
Expand All @@ -165,6 +165,18 @@ None of these files exist:
3 | import bar from 'foo';"
`;

exports[`linux relative paths with additional files included in the file map (watcher.additionalExts) fails when implicitly requiring a file outside sourceExts 1`] = `
"Unable to resolve module ./a from /root/index.js:
None of these files exist:
* a(.native|..js|.native.js|.js|..json|.native.json|.json)
* a/index(.native|..js|.native.js|.js|..json|.native.json|.json)
1 | import foo from 'bar';
> 2 | import a from './a';
| ^
3 | import bar from 'foo';"
`;

exports[`win32 assets checks asset extensions case insensitively 1`] = `
"Unable to resolve module ./asset.PNG from C:\\\\root\\\\index.js:
Expand Down Expand Up @@ -318,7 +330,7 @@ None of these files exist:
3 | import bar from 'foo';"
`;

exports[`win32 relative paths fails when trying to require a non supported extension 1`] = `
exports[`win32 relative paths fails when trying to implicitly require an extension not listed in sourceExts 1`] = `
"Unable to resolve module ./a.another from C:\\\\root\\\\index.js:
None of these files exist:
Expand All @@ -329,3 +341,15 @@ None of these files exist:
| ^
3 | import bar from 'foo';"
`;

exports[`win32 relative paths with additional files included in the file map (watcher.additionalExts) fails when implicitly requiring a file outside sourceExts 1`] = `
"Unable to resolve module ./a from C:\\\\root\\\\index.js:
None of these files exist:
* a(.native|..js|.native.js|.js|..json|.native.json|.json)
* a\\\\index(.native|..js|.native.js|.js|..json|.native.json|.json)
1 | import foo from 'bar';
> 2 | import a from './a';
| ^
3 | import bar from 'foo';"
`;
69 changes: 68 additions & 1 deletion packages/metro/src/DeltaBundler/__tests__/resolver-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ let resolver;
sourceExts: ['js', 'json'],
useWatchman: false,
},
watcher: {
additionalExts: ['cjs', 'mjs'],
},
maxWorkers: 1,
projectRoot: p('/root'),
reporter: require('../../lib/reporting').nullReporter,
Expand Down Expand Up @@ -275,7 +278,7 @@ let resolver;
);
});

it('fails when trying to require a non supported extension', async () => {
it('fails when trying to implicitly require an extension not listed in sourceExts', async () => {
setMockFileSystem({
'index.js': mockFileImport("import root from './a.another';"),
'a.another': '',
Expand Down Expand Up @@ -325,6 +328,46 @@ let resolver;
p('/root/folder.js'),
);
});

describe('with additional files included in the file map (watcher.additionalExts)', () => {
it('resolves modules outside sourceExts when required explicitly', async () => {
setMockFileSystem({
'index.js': mockFileImport("import a from './a.cjs';"),
'a.cjs': '',
});

resolver = await createResolver({
resolver: {
sourceExts: ['js', 'json'],
},
watcher: {
additionalExts: ['cjs'],
},
});
expect(resolver.resolve(p('/root/index.js'), './a.cjs')).toBe(
p('/root/a.cjs'),
);
});

it('fails when implicitly requiring a file outside sourceExts', async () => {
setMockFileSystem({
'index.js': mockFileImport("import a from './a';"),
'a.cjs': '',
});

resolver = await createResolver({
resolver: {
sourceExts: ['js', 'json'],
},
watcher: {
additionalExts: ['cjs'],
},
});
expect(() =>
resolver.resolve(p('/root/index.js'), './a'),
).toThrowErrorMatchingSnapshot();
});
});
});

describe('absolute paths', () => {
Expand Down Expand Up @@ -607,6 +650,30 @@ let resolver;
);
});

it('resolves main field correctly for a fully specified module included by watcher.additionalExts', async () => {
setMockFileSystem({
'index.js': '',
node_modules: {
foo: {
'package.json': JSON.stringify({
name: 'foo',
main: './main.cjs',
}),
'main.cjs': '',
},
},
});

resolver = await createResolver({
watcher: {
additionalExts: ['cjs'],
},
});
expect(resolver.resolve(p('/root/index.js'), 'foo')).toBe(
p('/root/node_modules/foo/main.cjs'),
);
});

it('allows package names with dots', async () => {
setMockFileSystem({
'index.js': '',
Expand Down
34 changes: 28 additions & 6 deletions packages/metro/src/ModuleGraph/node-haste/node-haste.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import type {Moduleish} from '../../node-haste/DependencyGraph/ModuleResolution';
import type {ResolveFn, TransformedCodeFile} from '../types.flow';
import type {Extensions, Path} from './node-haste.flow';
import type {Path} from './node-haste.flow';
import type {ModuleMapData, ModuleMapItem} from 'metro-file-map';
import type {CustomResolver} from 'metro-resolver';

Expand All @@ -27,7 +27,19 @@ const defaults = require('metro-config/src/defaults/defaults');
const path = require('path');

type ResolveOptions = {
assetExts: Extensions,
/**
* (Used by the resolver) The extensions tried (in order) to implicitly
* locate a source file.
*/
sourceExts: $ReadOnlyArray<string>,

/**
* The additional extensions to include in the file map as source files that
* can be explicitly imported.
*/
additionalExts: $ReadOnlyArray<string>,

assetExts: $ReadOnlyArray<string>,
assetResolutions: $ReadOnlyArray<string>,
+disableHierarchicalLookup: boolean,
+emptyModulePath: string,
Expand All @@ -37,7 +49,6 @@ type ResolveOptions = {
+platform: string,
platforms?: $ReadOnlyArray<string>,
resolveRequest?: ?CustomResolver,
+sourceExts: Extensions,
transformedFiles: {[path: Path]: TransformedCodeFile, ...},
};

Expand All @@ -63,12 +74,14 @@ const createModuleMap = ({
files,
moduleCache,
sourceExts,
additionalExts,
platforms,
}: {
files: Array<string>,
moduleCache: ModuleCache,
sourceExts: $ReadOnlySet<string>,
additionalExts: $ReadOnlySet<string>,
platforms: void | $ReadOnlyArray<string>,
sourceExts: Extensions,
}): ModuleMapData => {
const platformSet = new Set(
(platforms ?? defaults.platforms).concat([NATIVE_PLATFORM]),
Expand All @@ -82,10 +95,12 @@ const createModuleMap = ({
}
let id;
let module;
const fileExt = path.extname(filePath).substr(1);

if (filePath.endsWith(PACKAGE_JSON)) {
module = moduleCache.getPackage(filePath);
id = module.data.name;
} else if (sourceExts.indexOf(path.extname(filePath).substr(1)) !== -1) {
} else if (sourceExts.has(fileExt) || additionalExts.has(fileExt)) {
module = moduleCache.getModule(filePath);
id = module.name;
}
Expand Down Expand Up @@ -128,6 +143,7 @@ exports.createResolveFn = function (options: ResolveOptions): ResolveFn {
extraNodeModules,
transformedFiles,
sourceExts,
additionalExts,
platform,
platforms,
} = options;
Expand Down Expand Up @@ -161,7 +177,13 @@ exports.createResolveFn = function (options: ResolveOptions): ResolveFn {
moduleCache,
moduleMap: new ModuleMap({
duplicates: new Map(),
map: createModuleMap({files, moduleCache, sourceExts, platforms}),
map: createModuleMap({
files,
moduleCache,
sourceExts: new Set(sourceExts),
additionalExts: new Set(additionalExts),
platforms,
}),
mocks: new Map(),
rootDir: '',
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,13 @@ function createHasteMap(
computeDependencies,
computeSha1: true,
dependencyExtractor: config.resolver.dependencyExtractor,
extensions: config.resolver.sourceExts.concat(config.resolver.assetExts),
extensions: Array.from(
new Set([
...config.resolver.sourceExts,
...config.resolver.assetExts,
...config.watcher.additionalExts,
]),
),
forceNodeFilesystemAPI: !config.resolver.useWatchman,
hasteImplModulePath: config.resolver.hasteImplModulePath,
ignorePattern: getIgnorePattern(config),
Expand Down

0 comments on commit c1c6d9c

Please sign in to comment.