From c1c6d9c09fa92aa2ba71d803fdfa480e4dbb6a51 Mon Sep 17 00:00:00 2001 From: Alex Hunt Date: Thu, 28 Jul 2022 04:39:59 -0700 Subject: [PATCH] Add watcher.additionalExts option, enable inclusion of .cjs and .mjs files by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: These changes bring spec-compliant support for `.cjs` and `.mjs` module file extensions to Metro. Resolves https://github.com/facebook/metro/issues/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 --- docs/Configuration.md | 19 ++++- .../__snapshots__/loadConfig-test.js.snap | 16 +++++ packages/metro-config/src/configTypes.flow.js | 3 +- .../metro-config/src/defaults/defaults.js | 2 + packages/metro-config/src/defaults/index.js | 2 + .../__snapshots__/resolver-test.js.snap | 28 +++++++- .../DeltaBundler/__tests__/resolver-test.js | 69 ++++++++++++++++++- .../src/ModuleGraph/node-haste/node-haste.js | 34 +++++++-- .../DependencyGraph/createHasteMap.js | 8 ++- 9 files changed, 169 insertions(+), 12 deletions(-) diff --git a/docs/Configuration.md b/docs/Configuration.md index 887db1ce6d..d452173670 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -130,7 +130,11 @@ An array of asset extensions to include in the bundle. For example, if you would Type: `Array` -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` @@ -401,6 +405,19 @@ Dot notation in this section indicates a nested configuration object, e.g. `watc ::: +#### `additionalExts` + +Type: `Array` + +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` diff --git a/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap b/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap index 64a5607f48..d7aa4eea67 100644 --- a/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap +++ b/packages/metro-config/src/__tests__/__snapshots__/loadConfig-test.js.snap @@ -148,6 +148,10 @@ Object { "/", ], "watcher": Object { + "additionalExts": Array [ + "cjs", + "mjs", + ], "watchman": Object { "deferStates": Array [ "hg.update", @@ -305,6 +309,10 @@ Object { "/", ], "watcher": Object { + "additionalExts": Array [ + "cjs", + "mjs", + ], "watchman": Object { "deferStates": Array [ "hg.update", @@ -462,6 +470,10 @@ Object { "/", ], "watcher": Object { + "additionalExts": Array [ + "cjs", + "mjs", + ], "watchman": Object { "deferStates": Array [ "hg.update", @@ -619,6 +631,10 @@ Object { "/", ], "watcher": Object { + "additionalExts": Array [ + "cjs", + "mjs", + ], "watchman": Object { "deferStates": Array [ "hg.update", diff --git a/packages/metro-config/src/configTypes.flow.js b/packages/metro-config/src/configTypes.flow.js index e0bf41152b..d34985d37c 100644 --- a/packages/metro-config/src/configTypes.flow.js +++ b/packages/metro-config/src/configTypes.flow.js @@ -168,8 +168,9 @@ type SymbolicatorConfigT = { }; type WatcherConfigT = { + additionalExts: $ReadOnlyArray, watchman: { - deferStates?: $ReadOnlyArray, + deferStates: $ReadOnlyArray, }, }; diff --git a/packages/metro-config/src/defaults/defaults.js b/packages/metro-config/src/defaults/defaults.js index fccd5e4e60..895d011d6f 100644 --- a/packages/metro-config/src/defaults/defaults.js +++ b/packages/metro-config/src/defaults/defaults.js @@ -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); diff --git a/packages/metro-config/src/defaults/index.js b/packages/metro-config/src/defaults/index.js index 3fe095f229..68e1bddc41 100644 --- a/packages/metro-config/src/defaults/index.js +++ b/packages/metro-config/src/defaults/index.js @@ -16,6 +16,7 @@ const { DEFAULT_METRO_MINIFIER_PATH, assetExts, assetResolutions, + additionalExts, defaultCreateModuleIdFactory, platforms, sourceExts, @@ -129,6 +130,7 @@ const getDefaultValues = (projectRoot: ?string): ConfigT => ({ unstable_compactOutput: false, }, watcher: { + additionalExts, watchman: { deferStates: ['hg.update'], }, diff --git a/packages/metro/src/DeltaBundler/__tests__/__snapshots__/resolver-test.js.snap b/packages/metro/src/DeltaBundler/__tests__/__snapshots__/resolver-test.js.snap index cbb1df6209..7af33fb754 100644 --- a/packages/metro/src/DeltaBundler/__tests__/__snapshots__/resolver-test.js.snap +++ b/packages/metro/src/DeltaBundler/__tests__/__snapshots__/resolver-test.js.snap @@ -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: @@ -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: @@ -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: @@ -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';" +`; diff --git a/packages/metro/src/DeltaBundler/__tests__/resolver-test.js b/packages/metro/src/DeltaBundler/__tests__/resolver-test.js index e7feaec838..c2910947f2 100644 --- a/packages/metro/src/DeltaBundler/__tests__/resolver-test.js +++ b/packages/metro/src/DeltaBundler/__tests__/resolver-test.js @@ -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, @@ -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': '', @@ -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', () => { @@ -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': '', diff --git a/packages/metro/src/ModuleGraph/node-haste/node-haste.js b/packages/metro/src/ModuleGraph/node-haste/node-haste.js index 566c6ddfe4..a16b260f42 100644 --- a/packages/metro/src/ModuleGraph/node-haste/node-haste.js +++ b/packages/metro/src/ModuleGraph/node-haste/node-haste.js @@ -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'; @@ -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, + + /** + * The additional extensions to include in the file map as source files that + * can be explicitly imported. + */ + additionalExts: $ReadOnlyArray, + + assetExts: $ReadOnlyArray, assetResolutions: $ReadOnlyArray, +disableHierarchicalLookup: boolean, +emptyModulePath: string, @@ -37,7 +49,6 @@ type ResolveOptions = { +platform: string, platforms?: $ReadOnlyArray, resolveRequest?: ?CustomResolver, - +sourceExts: Extensions, transformedFiles: {[path: Path]: TransformedCodeFile, ...}, }; @@ -63,12 +74,14 @@ const createModuleMap = ({ files, moduleCache, sourceExts, + additionalExts, platforms, }: { files: Array, moduleCache: ModuleCache, + sourceExts: $ReadOnlySet, + additionalExts: $ReadOnlySet, platforms: void | $ReadOnlyArray, - sourceExts: Extensions, }): ModuleMapData => { const platformSet = new Set( (platforms ?? defaults.platforms).concat([NATIVE_PLATFORM]), @@ -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; } @@ -128,6 +143,7 @@ exports.createResolveFn = function (options: ResolveOptions): ResolveFn { extraNodeModules, transformedFiles, sourceExts, + additionalExts, platform, platforms, } = options; @@ -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: '', }), diff --git a/packages/metro/src/node-haste/DependencyGraph/createHasteMap.js b/packages/metro/src/node-haste/DependencyGraph/createHasteMap.js index 6e6614115b..e2d871eb95 100644 --- a/packages/metro/src/node-haste/DependencyGraph/createHasteMap.js +++ b/packages/metro/src/node-haste/DependencyGraph/createHasteMap.js @@ -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),