facebook / metro

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

Live refresh and console.log logging stop working in expo server when @babel/plugin-proposal-class-properties is included #877

Open mrflip opened 2 years ago

mrflip commented 2 years ago

tl;dr: I have an MRE where including a certain long-used babel plugin causes some part of the metro-babel-react-native-expo-server stack to eat its brain and stop doing live refresh or displaying error messages.

This was one of the worst gaslighting debug experiences I've encountered, and I had to bisection search every aspect of complexity in our entire project to find that it was a line in our babel.config.js that we added in February 2021. Given that, I am filing but reports against both expo and metro to make it discoverable. But since it is (a) likely some complex interaction among metro, babel, expo, Apple, watchman, react-native, the Bretton-Woods system, Church-Turing Thesis, expo-server, expo-cli, or what, and (b) has a low-cost workaround, you'll likely want to also k-thx-bye-closed this bug.

Please do note the spooky way it manifests itself in practice though.

What is the current behavior?

See this repo:

https://github.com/mrflip/bugreport-expo-babel-class-properties

With "@babel/plugin-proposal-class-properties" in its babel.config.js, expo start does not display console logs or do live refresh. Comment it out, clear cache, and restart, and it behaves correctly.

function ExpoBabelConfig(api) {
  api.cache(true)
  return {
    presets: [['babel-preset-expo', { lazyImports: true }]],
    plugins: [
      ["@babel/plugin-proposal-class-properties"], // sadness here
    ],
  }
}
module.exports = ExpoBabelConfig

To reproduce:

  1. git clone https://github.com/mrflip/bugreport-expo-babel-class-properties.git
  2. Run npm install ; npx expo start --ios --clear
  3. Change the favoriteLetter
  4. You should not see it refresh or see the console log in the terminal; you might see the console in the debugger.
  5. git checkout WorksJustFine
  6. npm install ; npx expo start --ios --clear
  7. Changing favoriteLetter should now cause a new line in the server console, and an auto-refresh of the app.
  8. Switching back and forth while clearing cache makes it reliably appear and disappear

That's nice and deterministic but until I isolated the problem, if the server was running the brokenness would not start right away -- probably, as parts of the cached program were transpiled, they started losing the sparkle magic that lets metro pay attention to them.

A couple times I caught it where if I changed favoriteLetter from "A" to "B" it would go through the motions of refreshing but stay at A; changing it to "C" would then show B, and so on (i.e. it stayed one save behind). Also, the logs might stop showing in the terminal window but show up for a different while in the react-native-debugger (dev tools), then eventually also give up on its own timeline in dev tools. As things decayed, logs from early in the execution might come out, but at some point no further entries were sent. That is: say the program had steps A B C D E; after copying over the babel config, for some while I would see logs and get refresh from A B C D and E and everything looked fine. After a time though I would only see logging from A B C, and some number of edits later would see logs from A and part of B, and eventually none. So some event makes the magic stop for the rest of an execution.

While this is reproducible on a Mac (see expo/expo#19290), it does not manifest on this codesandbox.io container. That could be due to the switch from Mac to linux, or the code sandbox may be inspiring its own refreshes. I'm curious if this is reproducible on other OS'es.

https://codesandbox.io/s/react-native-expo-example-9m93t?file=/package.json

What is the expected behavior?

Lovely live refresh and informative logs.

My Metro configuration and Metro, node, yarn/npm version and operating system.

(**) in our actual project we have the auto-generated metro config file:

// Learn more https://docs.expo.io/guides/customizing-metro
const { getDefaultConfig } = require('expo/metro-config');
module.exports = getDefaultConfig(__dirname);
robhogan commented 2 years ago

Well, this is interesting. Thanks for the great repro, that gave me a solid starting point.

With a bit more digging I think I've eventually pinned it down to this pure Node JS repro: https://github.com/robhogan/metro-issue-877/blob/main/index.js (run with yarn; node .)

That breaks fast refresh because this type annotation: https://github.com/facebook/react-native/blob/v0.70.1/Libraries/WebSocket/WebSocket.js#L85

Is transpiled via the this combination of plugins to an own property of WebSocket, as the repro demonstrates.

class BaseClass {
  constructor() {
    this.customProp = "foo";
  }
}

class ChildClass extends BaseClass {
  // customProp: ?string  - The transformation removes this...
  constructor(...args) {
    super(...args);
    _defineProperty(this, "customProp", void 0); // ...and adds this
  }
}

In particular, that clobbers the custom setter added by its base class EventTarget at 1, 2. So the assignment to onopen in the runtime HMRClient doesn't cause an event listener to be added, we never get a callback when the socket opens, and we never flush the queue of messages back over the websocket connection - and borked we are.

Issue with @babel/plugin-transform-flow-strip-types and class properties

The underlying issue here (maybe not the only one) is with @babel/plugin-transform-flow-strip-types:

https://github.com/robhogan/metro-issue-877/blob/main/flow-strip-types-issue.js

This strips the customProp class property (whether or not it's annotated with a type), changing the behaviour of modern JS (class fields are a published part of ES2022) when used alone. It shouldn't do that..

I'll have to leave this there for today, but I'll follow up with @motiz88 on this one - I have a feeling fixing this could be tricky.

Thanks again for the report - and in the meantime "@babel/plugin-proposal-class-properties" is already part of the preset, so you don't need to add it.

motiz88 commented 2 years ago

Seems related to https://babeljs.io/docs/en/babel-plugin-transform-flow-strip-types#allowdeclarefields - the Babel docs say this will become the default in Babel 8, and IIRC I've advocated for not changing the default in Metro before Babel does, but I'm open to changing my mind if we think this is a net positive.

bndkt commented 1 year ago

I have the same problem. I do not use @babel/plugin-proposal-class-properties directly, but it is included in several of my dependencies. I noticed the error after upgrading metro, so I iterated through all the recent versions and found that the error occurs once I upgrade from 0.73.2 to 0.73.3. Hope that info can help to pin down the cause.

motiz88 commented 1 year ago

@robhogan @jacdebug We should add this (changing the default allowDeclareFields setting) to the list of breaking changes planned for inclusion in the new transformer.

pencilcheck commented 1 year ago

Any updates? Both live refresh and console.log stops working when adding those in babel

aakashgupta0205 commented 1 year ago

Hi, any updates on this? Similar to @bndkt, while I am not using this plugin directly, it is included in several of my dependencies. Is the solution then to uninstall all these dependencies?

da45 commented 1 year ago

After one week, I solved the issue,

  1. I removed node_modules and package-lock.json
  2. run npx expo start -c, it works fine, Hope this will help
xseignard commented 1 year ago

Hello 👋 I have the same error and I was not able to fix it by removing @babel/plugin-proposal-class-properties as it is needed by react-native itself (random example) and some parts of our code.

I had to do this UGLY HACK, PLEASE DON'T DO IT 😬

if (__DEV__) {
  const primitiveTypes = ["string", "number", "boolean"];
  const logLevels = ["log", "debug", "info", "warn", "error"];

  const transformArgs = (args) => {
    return args.map((arg) => {
      if (arg === undefined) return "undefined";
      if (arg instanceof Error) {
        if (arg.stack) return arg.stack;
        return arg.toString();
      }
      if (arg instanceof Date) return arg.toString();
      if (primitiveTypes.includes(typeof arg)) {
        return arg.toString();
      } else {
        return JSON.stringify(arg);
      }
    });
  };

  const consoleProxy = new Proxy(console, {
    get: (target, prop) => {
      if (logLevels.includes(prop)) {
        return (...args) => {
          // we proxy the call to itself, but we transform the arguments to strings before
          // so that they are printed in the terminal
          return target[prop].apply(this, transformArgs(args));
        };
      }
      return target[prop];
    },
  });

  // eslint-disable-next-line no-global-assign
  console = consoleProxy;
}

Anyone to find a cleaner way to fix this? 🙏

steida commented 1 year ago

I had the same problem (console.log not working in Android) and it looks like a recently updated Babel fixed it.

Sguo12 commented 1 year ago

any update on this?

Gamote commented 9 months ago

We experience the same behaviour, no hot-reloading and no console.log.

Issue visible in this MRE: https://github.com/Gamote/expo-babel-void-optional-chain-issue

marcel-mintouge commented 8 months ago

Any update on this? I've tried @xseignard fix but didn't work

ttebify commented 8 months ago

This problem occurred after I installed a package. All I had to do to fix it was to delete my node_modules folder and yarn.lock file, and then reinstall my project dependencies.

Note: I tried all the solutions above, but none of them worked.

ChromeQ commented 7 months ago

I'm also seeing this issue after upgrading my expo SDK v49 to v50, which meant a lot of packages were updated so I can't know which one could be causing this. Is there any new information or workarounds found as none of the suggestions so far have helped.

Lurtroxx commented 7 months ago

Same issues here!

jtvargas commented 7 months ago

Same issue here

marcel-mintouge commented 7 months ago

Have you seen this already? https://github.com/expo/expo/issues/26254#issuecomment-1917973569

shoko-dev commented 7 months ago

Same issues here! Migrated from sdk 48 to 50. hot restart no longer works. I tried deleting node modules, expo doesn't help. package.json

emmyduruc commented 5 months ago

same issue here

shoko-dev commented 5 months ago

same issue here

Hi, try migrate to expo router. It helped me.

Lurtroxx commented 5 months ago

THE PACKAGE "REACT-NATIVE-VIDEO" has caused this to happen in my app. I updated to the react-native-video beta, and it stopped. Why it was doing that... I don't know, but it took me 250 hours of personal time, and 3 weeks to figure it out... Horrible.

drakulic-goran commented 5 months ago

Same issues here! Migrated from sdk 48 to 50. hot restart no longer works. I tried deleting node modules, expo doesn't help. package.json

Is there any solution?

Lurtroxx commented 5 months ago

@drakulic-goran Check your packages.

drakulic-goran commented 5 months ago

@Lurtroxx any special package? I try everything, check all packages, delete node_modules, package-lock.json, cache, reinstall all, and nothing helped... Migrate from SDK 50 to 51 make this form me.

Lurtroxx commented 5 months ago

@drakulic-goran for me, it was this: https://github.com/facebook/metro/issues/877#issuecomment-2115388285

drakulic-goran commented 5 months ago

@Lurtroxx for me I do not know...

alazypig commented 4 months ago

got the same issue.

delphinebugner commented 3 months ago

Got a similar issue, no console.log in terminal (only using the JS inspector in a browser). Hot refresh is working though.

Commenting to following the thread 👀

Edit: we're in a monorepo, so our metro.config looks pretty much like the one from expo doc: https://docs.expo.dev/guides/monorepos/#create-our-first-app

Also using NativeWind and react-native-svg-transformer, which have impacts on metro config

0xAskar commented 3 months ago

Any update by anyone? I just realized after upgrading expo I get no logs in our VSCode terminal. we're using expo 51

tyom commented 3 months ago

I had this issue for a while. I am using a monorepo with pnpm with changes in Metro config as in the pnpm monorepo example by Cedric.

My issue was that the console log only logged strings and numbers and completely ignored objects/arrays. It would ignore the whole log even if an empty array was set as a second argument to the console log.

I've tried many different things and ended up with a console proxy workaround that would JSON.strinfify all arguments to the console log. As I looked at this issue again, I noticed that the metro configuration was changed recently in Cedric's example. One of the lines to disable hierarchical look-up was removed after upgrading to Expo 51. As I tried commenting out that line in my Metro config the console logs started working again! It even worked after I uncommented it again.

NiccoloCase commented 3 months ago

any update?

drakulic-goran commented 2 months ago

Any update?

delphinebugner commented 2 months ago

Guys they are back 🥹

image

Simply used react-native start instead of expo start (and did a magnificent-temporary-cmd+maj+F to replace all my aliases @/. not working)

But there's definitely something there for me