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

Getting error in web workers #24

Closed
GuskiS opened this issue Dec 13, 2019 · 30 comments
Closed

Getting error in web workers #24

GuskiS opened this issue Dec 13, 2019 · 30 comments

Comments

@GuskiS
Copy link

GuskiS commented Dec 13, 2019

I'm getting error when using with web workers:
Uncaught ReferenceError: $RefreshReg$ is not defined

@pmmmwh
Copy link
Owner

pmmmwh commented Dec 17, 2019

Can you provide me some steps to reproduce? I'm not too familiar with Web Workers and don't have much context on what is broken.

@ro-savage
Copy link
Contributor

ro-savage commented Dec 20, 2019

@pmmmwh - I haven't had much time to looking into this yet.

But I believe the issue is caused because workers don't have window. And at least with https://github.com/developit/workerize-loader and https://github.com/GoogleChromeLabs/worker-plugin it suggests changing your webpack config to use this to allow for Hot Module Replacement

  output: {
    globalObject: "this"
  },

When you do this Webpack is throwing on

bootstrap:22 Uncaught ReferenceError: window is not defined
    at __webpack_require__ (bootstrap:22)
    at bootstrap:97
    at bootstrap:97

Which is lines (

if (window && window.$RefreshSetup$) {
cleanup = window.$RefreshSetup$(module.i);
}
)

It appears if you reference window at all here webpack throws even if its to check if window exists.

Changing these lines to use this seems to work for me for both when I've changed the globalObject to this and to when I have left it as window.

@rndm2
Copy link

rndm2 commented Mar 3, 2020

I am getting same error as author with version 0.2.0
I have added component that used in worker to babel ignore

@pmmmwh
Copy link
Owner

pmmmwh commented Mar 4, 2020

I am getting same error as author with version 0.2.0
I have added component that used in worker to babel ignore

Can you provide more info on your setup?

@rndm2
Copy link

rndm2 commented Mar 4, 2020

I am getting same error as author with version 0.2.0
I have added component that used in worker to babel ignore

Can you provide more info on your setup?

We are using worker-loader from webpack to load workers to DOM.

      {
        test: /\.worker\.js$/,
        use: {
          loader: 'worker-loader',
          options: {
            fallback: true,
            name: 'scripts/[hash].worker.js',
            publicPath: '/',
          },
        },
      },

I don't know why, but only last of the components chain have this error.
I added this component to babel ignore.

    "development": {
      "ignore": [
        "**/Element.js",
        "^^^ This needed to avoid transform of Element.js in Web Worker with react-refresh/babel, possible in future it can be removed"
      ],
      "plugins": [
        ["babel-plugin-module-resolver", { "root": ["./src/routes/builder"] }],
        ["@babel/plugin-transform-runtime", { "regenerator": true }],
        "transform-flow-strip-types",
        "@babel/plugin-syntax-export-default-from",
        "@babel/plugin-syntax-dynamic-import",
        "@babel/plugin-proposal-class-properties",
        "react-refresh/babel"
      ]
    },

The chain is Worker -> renderMarkup -> nodesToString -> Provider -> Node Component -> Element Component

It looks like problem caused by react-refresh/babel

@pmmmwh
Copy link
Owner

pmmmwh commented Apr 1, 2020

It looks like problem caused by react-refresh/babel

I cannot comment on this without having a look at a reproducible example.

I'll close this for now, since #29 addressed the original issue.
If you still face this issue, please feel free to open another issue.

@GuskiS
Copy link
Author

GuskiS commented Jul 31, 2020

Hmm, still getting some weird error:
image
When I click on the 220 line in devtools I see this output and there is no line 220:

module.exports = __webpack_public_path__ + "static/js/autocable.chunk.worker.js"

But the worker code is fine and it is running as expected 🤔
Using latest version.

@pmmmwh
Copy link
Owner

pmmmwh commented Jul 31, 2020

Hmm, still getting some weird error

The plugin now rewrites all all references to $RefreshReg$ and $RefreshSig$ so this is most likely a configuration error.

Did you exclude the worker file from being processed by react-refresh/babel or the plugin? You either have to set it up so that it passes through both of them or none of them.

@GuskiS
Copy link
Author

GuskiS commented Jul 31, 2020

I'm using this configuration: https://codesandbox.io/s/using-craco-ei2ek?file=/craco.config.js
And in code:

const worker = comlink.wrap(new Worker("lib/AutoCable/index.worker.ts", { name: "autocable", type: "module" }));

I don't have any other configuration for that worker 🤔

@pmmmwh
Copy link
Owner

pmmmwh commented Aug 1, 2020

I don't have any other configuration for that worker 🤔

You SHOULD have. You should have added processing/chunking behaviour for that worker because CRA does not support Workers out of the box. I would assume you added something like worker-plugin?

There are two choices.

  1. use the plugins options from it (ref) and add this plugin to the list.

  2. Exclude the loader file from Babel's processing, which means you have to modify the rule to add exclusions (which I do think is a bit verbose), or just move it out of your source folder.

Here's a demonstration of approach 2 -
https://codesandbox.io/s/using-craco-workers-qip7m

@GuskiS
Copy link
Author

GuskiS commented Aug 2, 2020

My apologies, somehow tunnel-read what you meant. Yes, I'm using worker-plugin. Will implement 1st suggested approach tomorrow, will report if there are any issues.

This might be useful to add to documentation, maybe worker section? As it requires additional changes for workers 🤔

@pmmmwh
Copy link
Owner

pmmmwh commented Aug 2, 2020

This might be useful to add to documentation, maybe worker section? As it requires additional changes for workers 🤔

I'll probably add a TROUBLESHOOTING.md soon and move all the FAQs there from the readme. I've been hoping to rewrite documentation so stuff will be clearer.

@GuskiS
Copy link
Author

GuskiS commented Aug 5, 2020

First approach didn't work, tried both ways - with named plugin or by initializing directly. I'm getting the same error.

Second approach feels like a lot of work, cause my worker is quite complicated.

@pmmmwh
Copy link
Owner

pmmmwh commented Aug 5, 2020

First approach didn't work, tried both ways - with named plugin or by initializing directly. I'm getting the same error.

Second approach feels like a lot of work, cause my worker is quite complicated.

Can you create a reproducible example? There's no way I can offer more help without any context.

@whatwg6
Copy link

whatwg6 commented Aug 11, 2020

i have same problem because of using react-refresh/babel in production by mistake, So just using react-refresh/babel in dev

@GuskiS
Copy link
Author

GuskiS commented Aug 11, 2020

I'm not using it in production, as I'm getting error in dev.
Also, I'm not able to recreate broken example.

@pmmmwh
Copy link
Owner

pmmmwh commented Aug 12, 2020

I'm not using it in production, as I'm getting error in dev.
Also, I'm not able to recreate broken example.

I'm not sure how to help further.

Clearly here the problem is that there exist some files that are linked by the worker plugin that evaded processing by this plugin, but got caught processed by the Babel plugin. This is something that I cannot help with (unfortunately), and would rely on either worker plugin wiring things up correctly, or tweaking Babel manually.

I can take a look at the craco config if provided, then maybe I can see if something is wrong.

@akphi
Copy link

akphi commented Aug 12, 2020

@pmmmwh I don't know much about the inner workings of this plugin, and honestly, I'm just looking everywhere to see bits of information. Looks like on next.js side, they also faced similar issue with worker-loader and managed to find a workaround

Personally, I don't know if I did the right thing but here's what I did. I create 2 rules for babel in webpack config:

  1. With react-refresh plugin but exclude the worker from the scope
  2. Without react-refresh plugin but include only the worker
    This doesn't work as when I run my app, the error overlay shows up with the same error message $RefreshSig$ is not defined
// ...
    module: {
      rules: [
        {
          test: /\.(js|ts)x?$/,
          exclude: [
           /node_modules/,
           'app/workers/myWorker.ts',
          ],
          use: [
            {
              loader: require.resolve('babel-loader'),
              options: {
                cacheDirectory: true,
                presets: [
                  '@babel/preset-env',
                  '@babel/preset-react',
                  '@babel/preset-typescript',
                ],
                plugins: [
                  // ... other plugins
                  isEnvDevelopment && 'react-refresh/babel'
                ].filter(Boolean)
              }
            },
          ]
        },
       // repeat the block another time to make sure the Worker is transpiled (since I use TS)
       {
          test: /Worker\.(js|ts)x?$/,
          exclude: [
           /node_modules/,
          ],
          use: [
            {
              loader: require.resolve('babel-loader'),
              options: {
                cacheDirectory: true,
                presets: [
                  '@babel/preset-env',
                  '@babel/preset-react',
                  '@babel/preset-typescript',
                ],
                plugins: [
                  // ... other plugins
                  // isEnvDevelopment && 'react-refresh/babel'
                ].filter(Boolean)
              }
            },
          ]
        }
    plugins: [
      new WorkerPlugin(),
      new ReactRefreshWebpackPlugin(),
      // ...
};

@pmmmwh
Copy link
Owner

pmmmwh commented Aug 12, 2020

@pmmmwh I don't know much about the inner workings of this plugin, and honestly, I'm just looking everywhere to see bits of information. Looks like on next.js side, they also faced similar issue with worker-loader and managed to find a workaround

That workaround won't work for us as the helpers we inject are hoisted in the Webpack global, not the actual global scope (window, global, self). We rewrite all helpers when we detect its usage (so if the file is not being processed by the plugin, we cannot do anything).

Personally, I don't know if I did the right thing but here's what I did. I create 2 rules for babel in webpack config

Try inverting the order of the two blocks - the more specific test rule should always come first.

@akphi
Copy link

akphi commented Aug 12, 2020

@pmmmwh Thanks, inverting the order doesn't help.

Regarding what you said earlier:

Clearly here the problem is that there exist some files that are linked by the worker plugin that evaded processing by this plugin, but got caught processed by the Babel plugin. This is something that I cannot help with (unfortunately), and would rely on either worker plugin wiring things up correctly, or tweaking Babel manually.

Does this mean if I could have react-refresh/babel not looking at this worker file at all then it should be ok right?
(I really hoped my trick earlier would work but it didn't so I wonder why...)

@pmmmwh
Copy link
Owner

pmmmwh commented Aug 12, 2020

Does this mean if I could have react-refresh/babel not looking at this worker file at all then it should be ok right?
(I really hoped my trick earlier would work but it didn't so I wonder why...)

Theoretically yes.

However upon looking at worker plugin issues it seems that it is slightly more complicated because some plugin hooks would bleed into child compilers (which is what worker plugin uses). I will have to take a look at the final bundle to see what is truly messed up.

(If possible I would love a repro)

@akphi
Copy link

akphi commented Aug 12, 2020

@pmmmwh Thank you so much! Can't wait to be able to try this out.
I will try to create a repro for you, my codebase is internal so might take some time for me to create a minimal reproducible though

@pmmmwh
Copy link
Owner

pmmmwh commented Aug 12, 2020

@pmmmwh Thank you so much! Can't wait to be able to try this out.
I will try to create a repro for you, my codebase is internal so might take some time for me to create a minimal reproducible though

I'll try creating my own in the mean time 🤣

Honestly though, I'm quite shocked that somehow it is more difficult to get things working with Workers than with non-browser renderers 🙈 I really want this to be as smooth as possible ...

@pmmmwh
Copy link
Owner

pmmmwh commented Aug 12, 2020

@blacksteed232 Seems like your second config block have a typo? Your worker's name is app/workers/myWorker.ts while you're matching Worker\.(js|ts)x?$

@pmmmwh
Copy link
Owner

pmmmwh commented Aug 12, 2020

@blacksteed232 @GuskiS

I have some workarounds.

Sloppy

  • In the entry of your Worker, add the following two lines:

    self.$RefreshReg$ = () => {};
    self.$RefreshSig$$ = () => () => {};
  • This is basically a "polyfill" for helpers expected by react-refresh/babel

Simple

  • You don't have to mess with configurations
  • Ensure all exports in the whole code path does not contain anything in PascalCase, which will make the Babel plugin do nothing when it hits your files
  • In general, the PascalCase naming scheme should be reserved for React components only, and I assume there wouldn't exist any React components within a Web Worker
  • Slight issue is when you have dependencies with that naming scheme, but most setups bypass node_modules from Babel anyways

Robust

  • Similar to what @blacksteed232 attempted, but with Webpack's oneOf rule

  • Alter the Webpack module rules (the section for JS-related files) as follows:

    {
      rules: [
        // JS-related rules only
        {
          oneOf: [
            {
              test: /\.[jt]s$/,
              include: '<Your worker files here>',
              exclude: /node_modules/,
              use: {
                loader: 'babel-loader',
                options: {
                  // Your Babel config here
                },
              },
            },
            {
              test: /\.[jt]sx?$/,
              include: '<Your files here>',
              exclude: ['<Your worker files here>', /node_modules/],
              use: {
                loader: 'babel-loader',
                options: {
                  // Your Babel config here
                },
              },
            },
          ],
        },
        // Any other rules, such as CSS
        {
          test: /\.css$/,
          use: ['style-loader', 'css-loader'],
        },
      ],
    }

Edit:

I also would suggest to not run ReactRefreshPlugin through WorkerPlugin - messing with module templates in child compilers for a different realm does not end well from my testing while figuring out the two solutions above. It COULD be made to work, but I currently don't see a good path towards that goal.

I do understand that it is a bit of work to add this configuration, but I also hope that people can understand that react-refresh and this plugin weren't designed to be used "standalone" or in non-React related code paths. For the case of Web Workers, in my honest opinion, I think they should be compiled from the start separately (in terms of TypeScript/Babel transpilation) - import/exports are not available natively, and the global object is different.

Hopefully this provided some insight and hopefully the workarounds above have solved your problems!

@GuskiS
Copy link
Author

GuskiS commented Aug 12, 2020

Thanks, when I added:

(global as any).$RefreshReg$ = () => {};
(global as any).$RefreshSig$$ = () => () => {};

it didn't show error. (CRA eslint rule didn't allow me to use self)

But that got me thinking, could it be possible that I'm somehow importing that code as worker and directly?
For example, that file contains class, in my code it is only used as worker(new Worker("./myworker.worker.ts")), but in few typing files I use it as type definition to see what methods are being exposed.

Could something in bundling process mess it up?
I haven't analyzed bundle output in a while, will give it a try later.

@akphi
Copy link

akphi commented Aug 12, 2020

@pmmmwh I can confirm that the Robust approach work, the problem for my snippet above is not about the wrong RegExp but more that my worker imports code from other places in the project that has PascalCase.

Due to that, Sloppy and Simple didn't quite work. But by doing Robust, i.e. well-isolating jsx and non-jsx files, I could get my code to work. Though your Simple explanation really shed some lights on the problem I got, I started by commenting out the imports.

So thank you so much @pmmmwh!!

I was super anxious that I can't really use this nice plugin since I have quite a complicated setup (i.e. Mobx, Typescript, worker-plugin, etc.), so finger-cross.

@pmmmwh
Copy link
Owner

pmmmwh commented Aug 12, 2020

Happy that the resolutions worked for both of you!

But that got me thinking, could it be possible that I'm somehow importing that code as worker and directly?
For example, that file contains class, in my code it is only used as worker(new Worker("./myworker.worker.ts")), but in few typing files I use it as type definition to see what methods are being exposed.

Could something in bundling process mess it up?
I haven't analyzed bundle output in a while, will give it a try later.

If you are on TypeScript 3.8+, I highly suggest checking out the import type syntax, this option for TypeScript and this option for Babel's TypeScript transpilation. Type imports ensures that no side effects will be ran from the imported module, and they will be erased completely after bundling.

@akphi
Copy link

akphi commented Aug 12, 2020

@pmmmwh Thanks, I did not pay attention to this when the version was released. Seems like the default should work fine for us as of now since the import statement will be purged if it's type-only.

My web-worker was actually using a function from another module, so I think babel takes those into account as well, if I don't comment out the imports and just don't use them, everything works fine.

@shelbyspeegle
Copy link

I wanted to post this here in case someone else is encountering the same issue as I was.

We are using worker-loader instead of worker-plugin.

After trying the suggested Robust solution, I still kept getting the error Cannot read properties of undefined (reading 'signature') from the worker bundle (__webpack_require__.$Refresh$ being undefined), despite verifying that worker files were excluded from babel-loader. This could still be some misconfiguration on my part.

Fix inspired by

Usage in webpack configuration is to replace ReactRefreshWebpackPlugin with an extended WorkerSafeReactRefreshWebpackPlugin.

import { Template } from 'webpack';
import ReactRefreshWebpackPlugin from '@pmmmwh/react-refresh-webpack-plugin';

// This ensures we don't get runtime errors by ensuring workers have stubbed out expected functions.
export default class WorkerSafeReactRefreshWebpackPlugin extends ReactRefreshWebpackPlugin {
  apply(compiler) {
    super.apply(compiler);

    compiler.hooks.compilation.tap('ReactRefreshWebpackPlugin', (compilation) => {
      const hookVars = compilation.mainTemplate.hooks.localVars;

      hookVars.tap('ReactFreshWebpackPlugin', (source) =>
        Template.asString([
          source,
          '',
          '// noop fns to prevent runtime errors during initialization',
          '__webpack_require__.$Refresh$ = { register: function () {}, signature: function() { return function(type) { return type; } } };',
        ]),
      );
    });
  }
}

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

7 participants