facebook / metro

🚇 The JavaScript bundler for React Native
https://metrobundler.dev
MIT License
5.17k stars 614 forks source link

ESM TypeScript support in the metro-resolver #886

Open MaijaHeiskanen opened 1 year ago

MaijaHeiskanen commented 1 year ago

Do you want to request a feature or report a bug?

Feature

What is the current behavior?

metro-resolver cannot resolve TypeScript imports that are in ESM format, e.g. have .js file extension in the import. It only adds extensions listed in sourceExts at the path, but does not consider situations where the original file extensions needs to be replaced.

What is the expected behavior?

metro-resolver should try to resolve TypeScript imports that are in ESM format. metro-resolver should try out the different file-extensions as it currently does, but in addition try out the file extensions also after removing the original file extension of the import. Example:

import {thing} from './something.js

Currently metro-resolver by default tries to resolve this as ./something.js, ./something.js.native, ./something.js.android.js, ./something.js.ts, ./something.js.android.ts, ... etc.

It should also try to resolve it as following: Currently metro-resolver by default tries to resolve this as ./something, ./something.native, ./something.android.js, ./something.ts, ./something.android.ts, ... etc. For my use case it should try to replace .js with .ts and .tsx, but there might be other use cases too.

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

Metro version: 0.67.0 Metro-resolver version: 0.67.0 Node version: 16.18.0 Yarn version: 1.22.4 Windows 11 and MacOS

// metro.config.js
const path = require('path');
const {getDefaultConfig} = require('metro-config');

module.exports = (async () => {
    const {
        resolver: { sourceExts, assetExts }
    } = await getDefaultConfig();

    return {
        projectRoot: path.resolve(__dirname, '.'),
        // watchFolders: [path.resolve(__dirname, './node_modules')],
        watchFolders: [
            path.resolve(__dirname, '../../node_modules'),
            path.resolve(__dirname, '../../packages/package1'),
            path.resolve(__dirname, '../../packages/package2'),
            path.resolve(__dirname, '../../packages/package3')
        ],
        resolver: {
            assetExts: assetExts.filter((ext) => ext !== 'svg'),
            // https://github.com/facebook/metro/issues/1#issuecomment-453450709
            extraNodeModules: new Proxy(
                {},
                {
                    get: (target, name) => path.join(process.cwd(), `node_modules/${name}`)
                }
            ),
            sourceExts: [...sourceExts, 'svg']
        },
        transformer: {
            babelTransformerPath: require.resolve('react-native-svg-transformer'),
            // eslint-disable-next-line require-await
            getTransformOptions: async () => ({
                transform: {
                    experimentalImportSupport: false,
                    inlineRequires: false
                }
            })
        }
    };
})();
huntie commented 1 year ago

Thanks for bringing this to our attention!

If my understanding is right, you're trying to enable this new TypeScript ES module compiler behaviour in your app's source code.

Put shortly, the behaviour you suggest above is functionality we don't yet support and would be independent from Metro's current concepts (so yes, new feature 🙂 — adding detail here to provide a clear spec).

Current module extension concepts

Replacing explicitly-provided extensions (e.g. ./something.js -> ./something.ts) has until now not been a necessary feature:

Under the above, file.js -> file.ios.js is not intended behaviour.

TypeScript ES module imports

TypeScript appears to be handling ES module extensions differently — requiring metro-resolver to resolve a source file which isn't present on the filesystem (e.g. .cjs import referencing a .cts file).

// @filename: helper.cts
export function helper() {
    console.log("hello world!");
}

// @filename: index.mts
import foo from "./helper.cjs"; // <- replaced with .cts

So, we'll need some new config concept or builtin behaviour in metro-resolver to handle this. Perhaps, .cts and .mts as "extra" extensions that are swappable for specific fully-specified extensions. 📥 This will go in our task backlog!

Recommendations

huntie commented 1 year ago

Some more digging in the TypeScript proposal on why this design was decided:

Because TypeScript never rewrites module specifiers in its JavaScript emit, the only possible answer is that it should mirror whatever resolution behavior the code’s intended runtime module resolver has\ — https://github.com/microsoft/TypeScript/issues/50152

There is also feedback/concern about this: Concerns with TypeScript 4.5's Node 12+ ESM Support. And this reply implies the TS team is committed to this design, and CommonJS also remains the default recommended setup from their point of view (intentional move of publishing no migration guide).

From the React Native side:

mo22 commented 1 year ago

can be kind-of fixed with the following code in metro.config.js:

const path = require('path');
const fs = require('fs');

/** @type {import('metro-config').ConfigT} */
module.exports = {
  // [...]
  resolver: {
    resolveRequest: (context, moduleName, platform) => {
      if ((moduleName.startsWith('.') || moduleName.startsWith('/')) && moduleName.endsWith('.js')) {
        const possibleResolvedTsFile = path.resolve(context.originModulePath, '..', moduleName).replace(/\.js$/, '.ts');
        if (fs.existsSync(possibleResolvedTsFile)) {
          return {
            filePath: possibleResolvedTsFile,
            type: 'sourceFile',
          };
        }
      }
      return context.resolveRequest(context, moduleName, platform);
    },
  }
  // [...]
};
ramijarrar commented 1 month ago

@mo22 I made some modifications to your suggested (kind-of) fix in order to avoid breaking things like platform-specific imports, .tsx files, etc.

I just stripped the .js or .jsx extensions and let the built-in resolver handle the rest:

resolver: {
  resolveRequest: (context, rawModuleName, platform) => {
    let moduleName = rawModuleName;

    // Resolve fully specified TS imports by stripping extension
    const isPackage =
      !moduleName.startsWith(".") && !moduleName.startsWith("/");
    const isJsOrJsxFile =
      !isPackage &&
      (moduleName.endsWith(".js") || moduleName.endsWith(".jsx"));
    if (isJsOrJsxFile) moduleName = moduleName.replace(/\.[^/.]+$/, "");

    return context.resolveRequest(context, moduleName, platform);
  },
},