Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0.80.4] Metro bundle duplicated code when use unstable_enablePackageExports and unstable_enableSymlinks #1197

Closed
orochi97 opened this issue Jan 20, 2024 · 3 comments
Assignees
Labels

Comments

@orochi97
Copy link

orochi97 commented Jan 20, 2024

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

bug

What is the current behavior?

My project uses pnpm + monorepo + react-native, and other self-developed libraries.

In my example project, I have three sub-projects, of which tool-demo is the react-native project, tool-runtime and tool-utils are other function libraries.

In the metro.config.js, unstable_enablePackageExports and unstable_enableSymlinks are true.

The dependencies are like this:

  1. tool-demo use tool-utils
  2. tool-utils uses tool-runtime/index and tool-runtime/setget.
  3. tool-runtime/index also uses tool-runtime/setget

When I bundle the react-native project,the code of tool-runtime/setget has been bundled twice.

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.

Here is my example project: https://github.com/orochi97/monorepo/tree/main

steps:

  1. pnpm install
  2. cd packages/tool-demo and pnpm run build:ios, then will output the index.bundle.ios.js in the tool-demo dir.
  3. search the keyword console.log('setRuntimeHelper'); in the index.bundle.ios.js, will find that the code is duplicated.

This will cause files with different paths to reference tool-runtime/setget will get different code values, like the let number in tool-runtime/setget.

What is the expected behavior?

No duplicate bundled code.

Now I use resolveRequest to solve this question. For details, please view the commented code below in the metro.config.js

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

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

const config = {
  resolver: {
    unstable_enablePackageExports: true,
    unstable_enableSymlinks: true,
    resolveRequest: (context, moduleName, platform) => {
      // if (moduleName.includes('/setget')) {
      //   const { type, filePath } = context.resolveRequest(context, moduleName, platform);
      //   return {
      //     type,
      //     filePath: realpathSync(filePath)
      //   }
      // }
      return context.resolveRequest(context, moduleName, platform);
    }
  },
  watchFolders: [
    resolve(__dirname, '../../node_modules'),
    resolve(__dirname, '../../packages/tool-utils'),
    resolve(__dirname, '../../packages/tool-runtime'),
  ],
};
module.exports = mergeConfig(getDefaultConfig(__dirname), config);

npx react-native info

System:
  OS: macOS 13.3.1
  CPU: (10) arm64 Apple M2 Pro
  Memory: 317.52 MB / 16.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 20.10.0
    path: ~/.nvm/versions/node/v20.10.0/bin/node
  Yarn: Not Found
  npm:
    version: 10.2.3
    path: ~/.nvm/versions/node/v20.10.0/bin/npm
  Watchman:
    version: 2023.10.23.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.14.2
    path: /Users/cchealthier/.rbenv/shims/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 22.4
      - iOS 16.4
      - macOS 13.3
      - tvOS 16.4
      - watchOS 9.4
  Android SDK: Not Found
IDEs:
  Android Studio: 2023.1 AI-231.9392.1.2311.11255304
  Xcode:
    version: 14.3/14E222b
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.9
    path: /usr/bin/javac
  Ruby:
    version: 2.7.6
    path: /Users/cchealthier/.rbenv/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.73.2
    wanted: ^0.73.2
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: false

Looking forward to reply, thank you :)

@orochi97 orochi97 changed the title [0.80.4] Metro bundle duplicate code when use unstable_enablePackageExports and unstable_enableSymlinks [0.80.4] Metro bundle duplicated code when use unstable_enablePackageExports and unstable_enableSymlinks Jan 20, 2024
@robhogan robhogan self-assigned this Jan 22, 2024
@robhogan robhogan added the bug label Jan 22, 2024
@robhogan
Copy link
Contributor

Huge thanks @orochi97 for the detailed report and repro - confirmed this is a previously-unknown bug due to a non-real absolute path ending up in the dependency graph, which shouldn't happen.

I'm looking into this now - in the mean time your commented out realPathSync custom resolver looks like a good workaround.

robhogan added a commit to robhogan/metro that referenced this issue Jan 22, 2024
Summary:
Fix: facebook#1197

`unstable_enablePackageExports` and `unstable_enableSymlinks` in combination may result in non-real module paths in the graph, because `PackageExportsResolve` only uses `context.doesFileExist` and does not necessarily return real paths.

Often, especially with pnpm, this results in a module appearing multiple times under different paths in the output bundle, as if duplicated on the file system, which causes incorrect behaviour for stateful modules and increases bundle size.

The fix mirrors the implementation of `resolveSourceFileForExt` in the non-exports resolver.

Changelog:
```
 - **[Experimental]:** Fix module duplication due to non-real resolved paths when combining `unstable_enablePackageExports` and `unstable_enableSymlinks`.
```

Differential Revision: D52964393
@robhogan
Copy link
Contributor

This turned out to be an issue with the package exports resolver, and the fix also ended up reducing bundle size by ~5% in the case of your demo. Thanks again for the report.

#1198

@orochi97
Copy link
Author

@robhogan Thank you for your reply and fix :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants