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

[metro] Metro config option requireCycleIgnorePatterns is not respected #26613

Open
hdwatts opened this issue Jan 23, 2024 · 7 comments · May be fixed by #27039
Open

[metro] Metro config option requireCycleIgnorePatterns is not respected #26613

hdwatts opened this issue Jan 23, 2024 · 7 comments · May be fixed by #27039
Labels
Issue accepted Platform: web Using Expo in the browser Router expo-router

Comments

@hdwatts
Copy link
Contributor

hdwatts commented Jan 23, 2024

Minimal reproducible example

https://github.com/hdwatts/expo-recreation-playground/tree/require-cycle-example

Summary

I use a library that has a cyclical import, which leads to constant log pollution alerting me to refactor the imports.

Metro has support for a requireCycleIgnorePatterns option, which is node_modules by default. Unfortunately, it appears as if that configuration option gets swallowed up by expo somewhere.

In the example repo, I have created a cyclical import and access it on both an Expo Router API route and on the front end. In both cases, my very lenient metro config option is ignored:

config.resolver = {
  ...config.resolver,
  requireCycleIgnorePatterns: [
    /(^|\/|\\)node_modules($|\/|\\)/,
    /.*circular.*/,
  ],
};

Require cycle: components/circular/backend/circle1.ts -> components/circular/backend/circle3.ts -> components/circular/backend/circle2.ts -> components/circular/backend/circle1.ts

Require cycle: components/circular/frontend/circle1.ts -> components/circular/frontend/circle3.ts -> components/circular/frontend/circle2.ts -> components/circular/frontend/circle1.ts

This appears to be web only, and does not occur on iOS.

Environment

expo-env-info 1.2.0 environment info:
System:
OS: macOS 14.2.1
Shell: 5.9 - /bin/zsh
Binaries:
Node: 20.8.1 - ~/Library/Caches/fnm_multishells/4396_1705978127572/bin/node
npm: 10.1.0 - ~/Library/Caches/fnm_multishells/4396_1705978127572/bin/npm
SDKs:
iOS SDK:
Platforms: DriverKit 23.2, iOS 17.2, macOS 14.2, tvOS 17.2, visionOS 1.0, watchOS 10.2
IDEs:
Android Studio: 2022.1 AI-221.6008.13.2211.9619390
Xcode: 15.2/15C500b - /usr/bin/xcodebuild
npmPackages:
expo: ~50.0.2 => 50.0.2
expo-router: ~3.4.3 => 3.4.4
react: 18.2.0 => 18.2.0
react-dom: 18.2.0 => 18.2.0
react-native: 0.73.2 => 0.73.2
react-native-web: ~0.19.6 => 0.19.10
npmGlobalPackages:
eas-cli: 5.7.0
Expo Workflow: managed

@hdwatts hdwatts added the needs validation Issue needs to be validated label Jan 23, 2024
@expo-bot expo-bot added needs review Issue is ready to be reviewed by a maintainer and removed needs validation Issue needs to be validated labels Jan 23, 2024
@Kudo
Copy link
Contributor

Kudo commented Jan 23, 2024

hi there! i tried your repro but it doesn't show the require cycle error for me.

$ git clone https://github.com/hdwatts/expo-recreation-playground.git
$ cd expo-recreation-playground
$ git checkout require-cycle-example
$ bun i --frozen-lockfile
$ npx expo start -i

after removing the line, the error starts to appear. do i miss anything from the repro?

@hdwatts
Copy link
Contributor Author

hdwatts commented Jan 23, 2024

Do i miss anything from the repro?

Good call out - my repro was using web not iOS. I've updated my issue to reflect that.

I can confirm that on iOS the logs do not appear, but on web they do.

image

@Kudo
Copy link
Contributor

Kudo commented Jan 23, 2024

i can confirm the issue and accept the issue. also, let us keep it as lower priority. the error seems to be coming from the api-routes server renderer that we keep working.

@Kudo Kudo added Platform: web Using Expo in the browser Issue accepted Router expo-router and removed needs review Issue is ready to be reviewed by a maintainer labels Jan 23, 2024
@expo-bot
Copy link
Collaborator

Thank you for filing this issue!
This comment acknowledges we believe this may be a bug and there’s enough information to investigate it.
However, we can’t promise any sort of timeline for resolution. We prioritize issues based on severity, breadth of impact, and alignment with our roadmap. If you’d like to help move it more quickly, you can continue to investigate it more deeply and/or you can open a pull request that fixes the cause.

@javascripter
Copy link

I'm using the below pathc-package patch for now.

diff --git a/node_modules/metro-runtime/src/polyfills/require.js b/node_modules/metro-runtime/src/polyfills/require.js
index ce67cb4..18edd64 100644
--- a/node_modules/metro-runtime/src/polyfills/require.js
+++ b/node_modules/metro-runtime/src/polyfills/require.js
@@ -131,7 +131,9 @@ function metroRequire(moduleId) {
 // `requireCycleIgnorePatterns` configuration.
 function shouldPrintRequireCycle(modules) {
   const regExps =
-    global[__METRO_GLOBAL_PREFIX__ + "__requireCycleIgnorePatterns"];
+    global[__METRO_GLOBAL_PREFIX__ + "__requireCycleIgnorePatterns"] ||
+    // HACK: Expo SSR mode doesn't have this global. Fall back to the default.
+    [/(^|\/|\\)node_modules($|\/|\\)/];
   if (!Array.isArray(regExps)) {
     return true;
   }

In this function,

function shouldPrintRequireCycle(modules) {
  const regExps =
    global[__METRO_GLOBAL_PREFIX__ + "__requireCycleIgnorePatterns"]

the value does not exist in the global object or globalThis object. If you eval it like this there, the value exists, so I guess prelude code is injected in a wrong place/environment or something.

eval(__METRO_GLOBAL_PREFIX__ + "__requireCycleIgnorePatterns"])

@bryanltobing
Copy link

can confirm that this is still the issue when using api-routes. solving with patch for now

@SleeplessByte
Copy link

This is an issue when not using api-routes, but regular expo-router on web.

Workaround by #26613 (comment) works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue accepted Platform: web Using Expo in the browser Router expo-router
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants