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

After refresh error overlay is not disposed correctly #553

Closed
sorodrigo opened this issue Dec 30, 2021 · 8 comments
Closed

After refresh error overlay is not disposed correctly #553

sorodrigo opened this issue Dec 30, 2021 · 8 comments

Comments

@sorodrigo
Copy link

Refresh works fine updating the nodes, but the event listeners stop working

fast.refresh.bug.mp4

.

@sorodrigo
Copy link
Author

I found the root cause the error overlay iframe is covering all the screen but it's transparent. I tried a lot of different configs and nothing worked 🙁 . I ended up using this hack:

  window.addEventListener('focusin', () => {
    try {
      // Fix for fast-refresh. Error Overlay goes wild.
      const iframes = Array.from(document.body.querySelectorAll('iframe'));
      const badIframe = iframes.find(
        (el) =>
          el.style.position === 'fixed' &&
          el.style.top === '0px' &&
          el.style.left === '0px' &&
          el.style.height === '100%' &&
          el.style.width === '100%',
      );
      if (badIframe) document.body.removeChild(badIframe);
    } catch {}
  });

@sorodrigo
Copy link
Author

Ok I have even more context now, this is the problematic code:

/**
 * Clears Webpack compilation errors and dismisses the compile error overlay.
 * @returns {void}
 */
function clearCompileError() {
  // HERE!!!! <=======================================
  // root and currentMode are both "null" and it seems that the ensureRootExists never gets called
  if (!root || currentMode !== 'compileError') {
    return;
  }

  currentCompileErrorMessage = '';
  currentMode = null;
  cleanup();
}

@sorodrigo sorodrigo changed the title After refresh all event listeners are lost After refresh error overlay is not disposed correctly Jan 6, 2022
@sorodrigo
Copy link
Author

sorodrigo commented Jan 6, 2022

Fixed here ✅
@pmmmwh+react-refresh-webpack-plugin+0.5.4.patch

diff --git a/node_modules/@pmmmwh/react-refresh-webpack-plugin/overlay/index.js b/node_modules/@pmmmwh/react-refresh-webpack-plugin/overlay/index.js
index 0a4d003..91cb0b2 100644
--- a/node_modules/@pmmmwh/react-refresh-webpack-plugin/overlay/index.js
+++ b/node_modules/@pmmmwh/react-refresh-webpack-plugin/overlay/index.js
@@ -225,6 +225,30 @@ function render() {
   });
 }

+/**
+ * Looks for stale rootIframe in body and removes it
+ * @returns {void}
+ */
+function seekAndDestroyStaleIframe() {
+  try {
+    const iframes = Array.from(document.body.querySelectorAll('iframe'));
+    const staleIframe = iframes.find(
+      (el) =>
+        el.style.position === 'fixed' &&
+        el.style.top === '0px' &&
+        el.style.left === '0px' &&
+        el.style.height === '100%' &&
+        el.style.width === '100%',
+    );
+    if (staleIframe) {
+      document.body.removeChild(staleIframe);
+    }
+  } catch (e) {
+    console.error('Error while disposing ReactErrorOverlay');
+    console.error(e);
+  }
+}
+
 /**
  * Destroys the state of the overlay.
  * @returns {void}
@@ -243,6 +267,7 @@ function cleanup() {
  */
 function clearCompileError() {
   if (!root || currentMode !== 'compileError') {
+    seekAndDestroyStaleIframe();
     return;
   }

@pmmmwh
Copy link
Owner

pmmmwh commented Jan 6, 2022

Hi, can you create a reproducible example for this issue? Is it certain that our iframe was blocking the page but was invisible? (I don't recall using visibility in the iframe) Are there any events that happened before this?

@sorodrigo
Copy link
Author

sorodrigo commented Jan 6, 2022

When I inspect the iframe there's no body just a script with some JS and some characters code. So I think that's why the iframe appears transparent. Also, the iframe ID is missing.

This is happening to me in a https://github.com/expo/expo-cli web application. I know they are already including react-dev-utils in the webpack entry point.

I also noticed that every time there's a fast refresh I get an error from one of webpackHotDevClient dependencies about process being undefined while reading process.platform.

I tried the following:

  1. removing react-dev-utils from webpack entry. => But then no fast-refresh happened at all.
  2. Replacing react-dev-utils with the following:
// require.resolve('webpack-dev-server/client') + '?/',
// require.resolve('webpack/hot/dev-server'),

But then no fast-refresh happened at all.
3. Trying to include the react-dev-utils via plugin overlay.module option but then it just crashed.

That's why I resorted to the hack above. And it currently fixes my issue 😅 .

As for a reproduction, maybe this works:

const path = require('path');
const createExpoWebpackConfigAsync = require('@expo/webpack-config');
const ReactRefreshWebpackPlugin = require('@pmmmwh/react-refresh-webpack-plugin');

module.exports = async function (env, argv) {
  const config = await createExpoWebpackConfigAsync(env, argv);
  const isDevelopment = config.mode === 'development';

  if (isDevelopment) {
    config.devServer.compress = false;
    config.devServer.hot = true;
  }

  config.module.rules = [
    {
      test: /\.(js|mjs|jsx|ts|tsx|web.js)$/,
      include: path.resolve(__dirname),
      loader: require.resolve('babel-loader'),
      options: {
        cacheDirectory: true,
        plugins: [
          isDevelopment && require.resolve('react-refresh/babel')
        ].filter(Boolean),
      },
    },
    {
      test: /\.(png|jpe?g|gif|svg)$/i,
      use: [
        {
          loader: 'file-loader',
          options: {
            name: 'static/images/[name].[contenthash].[ext]',
          },
        },
      ],
    },
    {
      test: /\.(android.js|ios.js|native.js)$/,
      exclude: /node_modules/,
      use: {
        loader: 'ignore-loader',
      },
    },
  ];

  const plugins = [
    isDevelopment && new ReactRefreshWebpackPlugin(),
  ].filter(Boolean);

  config.plugins = [
    ...config.plugins,
    ...plugins,
  ];
  // Finally return the new config for the CLI to use.
  return config;
};

Tagging an expo-cli maintainer so they're in the loop too. @Kudo

@leoafarias
Copy link

Seeing the same issue on expo. Iframe being rendered, on top and process error.

I have created an RE @pmmmwh @sorodrigo

https://github.com/leoafarias/expo-fast-refresh-bug

@pmmmwh
Copy link
Owner

pmmmwh commented Feb 7, 2022

This is a bug of react-error-overlay.

You can resolve this by pinning the version to 6.0.9 until this PR is merged and published:

{
  "resolutions": {
    "react-error-overlay": "6.0.9"
  }
}

If you want to properly configure the overlay experience, you can use this configuration:

new ReactRefreshPlugin({
  overlay: {
    entry: false,
    module: require.resolve('react-dev-utils/refreshOverlayInterop'),
    sockIntegration: false,
  },
});

@pmmmwh pmmmwh closed this as completed Feb 7, 2022
@sorodrigo
Copy link
Author

@leoafarias @pmmmwh I tried above snippet and I was getting an error, turns out we need react-dev-utils@11.0.0 this fixes it:

{
  "resolutions": {
    "react-error-overlay": "6.0.9",
    "react-dev-utils": "11.0.0"
  }
}

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

3 participants