facebook / metro

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

[0.80.11] breaks aliases with babel-plugin-module-resolver on imports (?) #1345

Closed efstathiosntonas closed 2 months ago

efstathiosntonas commented 2 months ago

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

bug, using react-native 0.75.2, Paper, watchman@2024.08.26.00

Tried: 1.rm -rf node_modules 2.--reset-cache

  1. no luck

downgrading to 0.80.10 works as a charm.

What is the current behavior?

throws:

error: Error: Unable to resolve module ./ from /Users/stathis/IdeaProjects/xxxxxx/src/xxxxxx/posts/components/comments/PostComment/components/ContentCommentLines.tsx:

None of these files exist:
  * src/flows/posts/components/comments/PostComment/components/index(.android.js|.native.js|.js|.android.jsx|.native.jsx|.jsx|.android.json|.native.json|.json|.android.ts|.native.ts|.ts|.android.tsx|.native.tsx|.tsx|.android.svg|.native.svg|.svg)
  3 | import isEqual from 'react-fast-compare';
  4 |
> 5 | import {
    | ^
  6 |   ContentLeftBorderInnerView,
  7 |   ContentLeftBorderOuterView
  8 | } from '@flows/posts/components/comments/PostComment/components';
    at ModuleResolver.resolveDependency (/Users/stathis/IdeaProjects/xxxxxx/node_modules/metro/src/node-haste/DependencyGraph/ModuleResolution.js:112:15)
Click for simulator screenshot ![Screenshot 2024-09-04 at 16 03 01](https://github.com/user-attachments/assets/cbcac80b-5304-4580-88b3-7e4292cfe743)
Click for babel config ```js const moduleResolver = [ [ 'module-resolver', { root: ['./src'], extensions: [ '.ios.ts', '.android.ts', '.ts', '.ios.tsx', '.android.tsx', '.tsx', '.jsx', '.js', '.json' ], alias: { '@assets': './src/assets', '@components': './src/components', '@contexts': './src/contexts', '@flows': './src/flows', '@functions': './src/functions', '@generated': './src/generated', '@hooks': './src/hooks', '@store': './src/store', '@styles': './src/styles', '@themes': './src/themes', '@typings': './src/types', '@utils': './src/utils' } } ] ]; module.exports = { presets: [ 'module:@react-native/babel-preset' ], plugins: process.env.NODE_ENV === 'production' ? [ 'transform-remove-console', ...moduleResolver, 'macros', 'react-native-reanimated/plugin' ] : [...moduleResolver, 'macros', 'react-native-reanimated/plugin'] }; ```
Click for tsconfig ```json { "compilerOptions": { "emitDecoratorMetadata": true, "allowJs": true, "allowSyntheticDefaultImports": true, "baseUrl": ".", "allowUnreachableCode": false, "experimentalDecorators": true, "allowUnusedLabels": false, "noStrictGenericChecks": false, "esModuleInterop": true, "importHelpers": true, "isolatedModules": true, "jsx": "react-native", "lib": ["esnext"], "moduleResolution": "node", "noEmit": true, "module": "esnext", "paths": { "*": ["src/*"], "@assets/*": ["src/assets/*"], "@components/*": ["src/components/*", "components/*"], "@contexts/*": ["src/contexts/*"], "@flows/*": ["src/flows/*"], "@functions/*": ["src/functions/*"], "@generated/*": ["src/generated/*"], "@hooks/*": ["src/hooks/*"], "@store/*": ["src/store/*"], "@styles/*": ["src/styles/*"], "@themes/*": ["src/themes/*"], "@typings/*": ["src/types/*"], "@utils/*": ["src/utils/*"] }, "resolveJsonModule": true, "skipLibCheck": true, "strict": true, "target": "esnext", "typeRoots": ["./typings"] }, "exclude": [ "node_modules", "./node_modules/", "**/node_modules", "ios", "android", "babel.config.js", "metro.config.js" ], "include": ["src", "__tests__", "typings"] } ```

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.

What is the expected behavior?

expect to import packages with aliases

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

metro config:

const { getDefaultConfig, mergeConfig } = require('@react-native/metro-config');

const defaultConfig = getDefaultConfig(__dirname);
const { assetExts, sourceExts } = defaultConfig.resolver;

metroConfig = (() => {
  return {
    transformer: {
      babelTransformerPath: require.resolve('react-native-svg-transformer/react-native')
    },
    resolver: {
      assetExts: [...assetExts.filter((ext) => ext !== 'svg'), 'bin', 'lottie'],
      sourceExts: [...sourceExts, 'svg']
    }
  };
})();

module.exports = mergeConfig(defaultConfig, metroConfig);
robhogan commented 2 months ago

Thanks for the quick report @efstathiosntonas. Could you share how @flows/posts is set up for you? In particular, is that a bare package.json or does it define exports? (I'm guessing there's a package.json at src/flows/posts or src/flows?

Also, what file should this be resolving to? Is it included within the files Metro is claiming don't exist?

efstathiosntonas commented 2 months ago

@robhogan no, there's no package in there. There's only one plain package.json in the root with no exports. I'm not in a monorepo if it helps.

efstathiosntonas commented 2 months ago

To answer the second question, the file resides into src/flows/posts/components/comments/PostComment/components/.

I believe the answer to the issue is the beginning of the error: Unable to resolve module ./, it looks for ./ instead of the full path and can't find it?

robhogan commented 2 months ago

Oh I see.. Metro's error message isn't great here - the error is actually resolving a dependency of posts/components/comments/PostComment/components/ContentCommentLines.tsx.

So you have an import x from './' in components/ContentCommentLines.tsx that's meant to resolve to components/index.tsx??

(I'm still a bit puzzled about what @flows is if you don't have nested package.jsons and this isn't a workspace project. What do you mean by "aliases"? But that might not be related to the ./ issue...)

efstathiosntonas commented 2 months ago

I'm using this for aliases: https://dev.to/larswaechter/path-aliases-with-typescript-in-nodejs-4353

flows is under src/flows

robhogan commented 2 months ago

Ah, I missed that you were using a Babel module resolver.

We don't support that, but we didn't intend to break it. Let me try to repro this...

robhogan commented 2 months ago

Could you share the imports of ContentCommentLines.tsx?

efstathiosntonas commented 2 months ago
import React, { memo } from 'react';

import isEqual from 'react-fast-compare';

import {
  ContentLeftBorderInnerView,
  ContentLeftBorderOuterView
} from '@flows/posts/components/comments/PostComment/components';
robhogan commented 2 months ago

Ok, I have a repro. I think this might be the edge case mentioned in https://github.com/facebook/metro/commit/1e1dfe173bd793f4d718cd08fa6af15659b0516c that that commit was intended to fix.

When you write from '@flows/posts/components/comments/PostComment/components';, what do you expect that to resolve to? It looks like it could reasonably be:

The problem is that Babel module-resolver transforms:

import {
  ContentLeftBorderInnerView,
  ContentLeftBorderOuterView
} from '@flows/posts/components/comments/PostComment/components';

into

import {
  ContentLeftBorderInnerView,
  ContentLeftBorderOuterView
} from './';

But ./ can't be resolved because there's no index (or package.json) inside the src/posts/components/comments/PostComment/components directory.

In Metro 0.80.10, we would've resolved ./ to ../components.tsx, which is pretty strange, but it looks to me like that's the behaviour you were relying on.

If you changed your import to from '@flows/posts/components/comments/PostComment/components.tsx', presumably it works?

efstathiosntonas commented 2 months ago

Thanks for the repro Rob.

I would expect to resolve to

*src/flows/posts/components/comments/PostComment/components/anyComponent.tsx

there’s no components.tsx

robhogan commented 2 months ago

I see a src/flows/posts/components/comments/PostComment/components.tsx in your screenshot?

efstathiosntonas commented 2 months ago

hey Rob, sorry for the late reply, yeah you’re right, need to get rid of this bad file naming 😂, it’s confusing.

will try to fix it tomorrow and update the issue.

robhogan commented 2 months ago

If my theory is right here this is really a babel-plugin-module-resolver issue that would equally affect Node or Webpack. It transforms your (correct) specifier to ./, which precludes extension matching to ${__dirname}.tsx due to the trailing slash.

It only affects the edge case where you have a directory and adjacent files with the same name + extension, but if it transformed your alias to . instead of ./ it'd fine I think - presumably that's what the TypeScript resolver is doing.

efstathiosntonas commented 2 months ago

Hi Rob, indeed, the moment I renamed the components.tsx to comment-components.tsx it worked fine, thankfully enough it was the only "bad" filename. Life got back to normal 😅

Thank you for taking the time looking into this, at least we learned something new 😄

please feel free to close this issue

efstathiosntonas commented 2 months ago

totally off topic, but do you have any idea why metro is jumping to 100% while it's still obviously loading files? It happens on iOS only, simulator rn info bar shows "good" percentages not 100% , on the video the "normal" percentage counting is for Android. This is happening for at least 7-8 metro versions now.

https://github.com/user-attachments/assets/c4090a74-30a5-48ef-975f-39a1e8f902be

ACHP commented 2 months ago

Same problem here I have a file app/utils/foo.ts that import a function from app/utils.ts, the import get transformed by the babel plugin to "./", but there is no app/utils/index.ts … Exactly the same problem.

Renaming the file or just moving app/utils.ts to app/utils/index.ts would work as long as I own the code, but can it happens with library code as well (I mean inside node_modules package).

Should we consider fixing babel-plugin-module-resolver by checking that an index.[suffixes] file exist before transforming to "./" ?

robhogan commented 2 months ago

Renaming the file or just moving app/utils.ts to app/utils/index.ts would work as long as I own the code, but can it happens with library code as well (I mean inside node_modules package).

Third-party npm packages are (I hope 😬🤞) not relying on Babel to transform their imports. If they ship an import * from './' expecting it to resolve to something in the parent directory, I believe that’d already be broken under Node and Webpack. Metro is aligning with others here - please correct me if I’m wrong.

Should we consider fixing babel-plugin-module-resolver by checking that an index.[suffixes] file exist before transforming to "./" ?

A Babel plugin shouldn’t be performing any IO - that makes transform caching impossible - but I think if it transformed to . without the slash that would be fine. But yep fixing this in the plugin is the right way forward here.

Metro resolving “./“ to “../dirname.js” was always a bug. We didn’t anticipate it affecting babel-plugin-module-resolver like this, but I don’t think we should revert the fix.

robhogan commented 2 months ago

totally off topic, but do you have any idea why metro is jumping to 100% while it's still obviously loading files? It happens on iOS only, simulator rn info bar shows "good" percentages not 100% , on the video the "normal" percentage counting is for Android. This is happening for at least 7-8 metro versions now.

Screen.Recording.2024-09-05.at.09.33.13.mov

Thanks for the report - the progress bar is indicative at best (it never really knows how much of the graph is undiscovered) but moving to 100% immediately is a bug - seems like it’s racing such that instead of going from 0/1 to 1/n, it’s going to 1/1 after the first transform, and then by design it never goes backwards.

robhogan commented 2 months ago

please feel free to close this issue

Yeah, I'm going to close this as a "won't fix" - it's a babel-plugin-module-resolver bug.

Thanks for the reports though folks - it's good to know about this.