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

Custom mapping isn't integrated when done through metro.config.js #1765

Closed
Bertram25 opened this issue Jul 13, 2023 · 10 comments
Closed

Custom mapping isn't integrated when done through metro.config.js #1765

Bertram25 opened this issue Jul 13, 2023 · 10 comments

Comments

@Bertram25
Copy link

Bertram25 commented Jul 13, 2023

🐛 Bug Report

After the upgrade of React Native to 72.3, Metro to 0.76.7, and UI Kitten to 2.2.0, the following metro.config.js content doesn't integrate the custom mapping anymore.

// Reference: https://akveo.github.io/react-native-ui-kitten/docs/guides/improving-performance#improving-performance
const { getDefaultConfig } = require("@react-native/metro-config");

const evaMetroConfig = require("@ui-kitten/metro-config");
const evaConfig = {
  evaPackage: "@eva-design/eva",
  customMappingPath: "./app/theme/customMapping.json",
};

module.exports = evaMetroConfig.create(evaConfig, getDefaultConfig(__dirname));

The following was previously working with React Native 70.6, UI Kitten 2.1.1 and Metro 0.72.3

The bundle message: success Successfully bootstrapped @eva-design/eva is not displayed anymore as well.

As a workaround, the command: yarn ui-kitten bootstrap @eva-design/eva ./app/theme/customMapping.json still works

When looking deeper and debugging locally, it appeared that the metro function injected into reporter: { update: <method> } is never run, and thus never runs the eva bootstrapservice.run() method which create the generate.json file used for theming.

To Reproduce

Steps to reproduce the behavior:

  • Using a react native project with the above configuration
  • using a custom mapped them value in the project.
  • The value is not taken in account

Link to runnable example or repository (highly encouraged)

Minimal app with the bug reproduced: https://github.com/Bertram25/minimal-ui-kitten-app

UI Kitten and Eva version

Package Version
@eva-design/eva 2.2.0
@ui-kitten/components 5.3.1

Environment information

npx envinfo --preset react-native

  System:
    OS: Windows 10 10.0.19045
    CPU: (8) x64 Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz
  Binaries:
    Node: 16.14.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.5 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 8.5.0 - C:\Program Files\nodejs\npm.CMD
  npmPackages:
    react: 18.2.0 => 18.2.0
    react-native: 0.72.3 => 0.72.3
@greenfrvr
Copy link
Member

@Bertram25 I experienced similar behavior, but with expo. When running project locally everything is working fine, but when building project reporter update is also never run. For now we don't have neither a fix nor an understanding why that happen, but we'll try to figure out.

Is this metro issue related only to cases when running metro locally or it is also appears when building app binaries?

@zhenkaGo
Copy link

this issue was resolved in this PR. It will be released with a 0.73 version

@Bertram25
Copy link
Author

Hello again,

So we upgraded our project to react native 0.73 and checked that the community-cli is installed in the right version. (It seems your PR was applied on the 12.0 version and that react native 0.73 is provided with the version 12.3.0)
As assessed in our yarn.lock file:

"@react-native-community/[email protected]":
  version "12.3.0"
  resolved "https://registry.yarnpkg.com/@react-native-community/cli/-/cli-12.3.0.tgz#c89aacc3973943bf24002255d7d0859b511d88a1"
  integrity sha512-XeQohi2E+S2+MMSz97QcEZ/bWpi8sfKiQg35XuYeJkc32Til2g0b97jRpn0/+fV0BInHoG1CQYWwHA7opMsrHg==
  dependencies:
    "@react-native-community/cli-clean" "12.3.0"
    "@react-native-community/cli-config" "12.3.0"
    "@react-native-community/cli-debugger-ui" "12.3.0"
    "@react-native-community/cli-doctor" "12.3.0"
    "@react-native-community/cli-hermes" "12.3.0"
    "@react-native-community/cli-plugin-metro" "12.3.0"
    "@react-native-community/cli-server-api" "12.3.0"
    "@react-native-community/cli-tools" "12.3.0"
    "@react-native-community/cli-types" "12.3.0"
...

With the same adapted metro config, we still didn't have the message:
success Successfully bootstrapped @eva-design/eva

So we added the following command on our (yarn) start script (in package.json):
"start": "ui-kitten bootstrap @eva-design/eva ./app/theme/customMapping.json && react-native start --experimental-debugger"

This is actually triggering the message (Successfully bootstrapped...) the first time, but then no more. We checked that and it seems to be ok, since our custom mapping hasn't changed between 2 calls, and that you are using a checksum to avoid generating the node_modules/@eva-design/eva/generated.json several times. (Still it would be great if you could add a message to tell the file is there).

But then once starting the app on e.g.: Android using (a - run on android on Metro after running react-native start), the generated mapping are not found by the app with the given message for each custom mappings not found:
e.g.:

Running "ourApp" with {"rootTag":11}
 WARN  Button: unsupported configuration.
Check one of the following prop values: {
  "appearance": "filled",
  "variants": [
    "giant"
  ],
  "states": []
}
📖 Documentation: https://akveo.github.io/react-native-ui-kitten/docs/components/button/api
    at Wrapper
...

We tried to investigate where the generated.json file was used from (in the styled.js code and theme service) but couldn't conclude on anything specific.

Do you know what we could be doing wrong?

@Bertram25
Copy link
Author

Bertram25 commented Jan 11, 2024

@zhenkaGo Maybe a more specific question would help: Can you give us some pointers on how best we could debug the fact that the generated.json file is packaged/used by the style and/or the them service/provider/consumer?

Namely how and why generated.json file is valid (with our custom mapping values) once the bootstrap service has run, but seems not taken in account with the custom mapping in the final build.

@zhenkaGo
Copy link

Hi @Bertram25. sorry for taking so long to answer
Try to add this patch. react-native start has to work instead of your script ui-kitten bootstrap @eva-design/eva ./app/theme/customMapping.json && react-native start --experimental-debugger

diff --git a/node_modules/@ui-kitten/metro-config/index.js b/node_modules/@ui-kitten/metro-config/index.js
index 54cb910..b1a844c 100644
--- a/node_modules/@ui-kitten/metro-config/index.js
+++ b/node_modules/@ui-kitten/metro-config/index.js
@@ -59,10 +59,11 @@ const create = (evaConfig, metroConfig) => {
     };
     const libConfig = {
         reporter: {
+            ...metroConfig?.reporter,
             update: handleMetroEvent,
         },
     };
-    return (0, lodash_merge_1.default)({}, libConfig, metroConfig);
+    return { ...metroConfig, ...libConfig }
 };
 exports.create = create;
 //# sourceMappingURL=index.js.map

I reckon this problem also was resolved in this PR but it wasn't released yet.

@laooola
Copy link

laooola commented Feb 9, 2024

We're struggling with the exact same issue. Any perspective on when the fix will be released?

@t4ngth00
Copy link

Hello, is there any update on the issue?

@Bertram25
Copy link
Author

Hey @zhenkaGo , sorry it's been a while on my end too.

I tried your patch and unfortunately, it didn't work.
To try and make things easier on both end, I added a minimal app displaying the bug here:
https://github.com/Bertram25/minimal-ui-kitten-app

On this repository, if I didn't do anything wrong, applying your patch doesn't solve the issue either.
I hope this will help understanding the bug and hopefully get to the root of it.

Cheers,

@zhenkaGo
Copy link

Hi. @Bertram25. I've checked your mini app. If you still has this problem try one more patch. I don't why this problem still exist in RN. They had to solved them here.

diff --git a/node_modules/@react-native/community-cli-plugin/dist/commands/start/runServer.js b/node_modules/@react-native/community-cli-plugin/dist/commands/start/runServer.js
index 114e1da..49747d3 100644
--- a/node_modules/@react-native/community-cli-plugin/dist/commands/start/runServer.js
+++ b/node_modules/@react-native/community-cli-plugin/dist/commands/start/runServer.js
@@ -99,9 +99,12 @@ async function runServer(_argv, ctx, args) {
     },
   });
   let reportEvent;
-  const terminal = new _metroCore.Terminal(process.stdout);
-  const ReporterImpl = getReporterImpl(args.customLogReporterPath);
-  const terminalReporter = new ReporterImpl(terminal);
+  let terminalReporter = metroConfig.reporter;
+  if (args.customLogReporterPath) {
+    const terminal = new _metroCore.Terminal(process.stdout);
+    const ReporterImpl = getReporterImpl(args.customLogReporterPath);
+    terminalReporter = args.customLogReporterPath ? new ReporterImpl(terminal) : metroConfig.reporter;
+  }
   // $FlowIgnore[cannot-write] Assigning to readonly property
   metroConfig.reporter = {
     update(event) {

@Bertram25
Copy link
Author

@zhenkaGo hey, I know it's been a long time but I wanted to confirm adding this patch solve the issue (using patch-package)

Fun fact, we found the same kind of issue in React 0.75 only for iOS and it seems the issue is gone on React 0.76.
As for me, you can close this issue. Thanks a lot for the support.

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

No branches or pull requests

5 participants