comlink-loader icon indicating copy to clipboard operation
comlink-loader copied to clipboard

Fix function type error in README

Open ahuglajbclajep opened this issue 4 years ago • 7 comments

Fix type error of greet function in README. See also #1.

Also, TypeScript does not allow the addition of ".ts" when importing *.ts. https://www.typescriptlang.org/docs/handbook/modules.html#import I also fix the README small mistake about this behavior.

ahuglajbclajep avatar Feb 27 '20 19:02 ahuglajbclajep

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

googlebot avatar Feb 27 '20 19:02 googlebot

@googlebot I signed it!

ahuglajbclajep avatar Feb 27 '20 19:02 ahuglajbclajep

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

googlebot avatar Feb 27 '20 19:02 googlebot

@developit I’d love your feedback, please review my PR.

ahuglajbclajep avatar Mar 04 '20 22:03 ahuglajbclajep

Maybe in the webpack.config.js there should also be a comment about additional loaders for typescript, like ts-loader?

So the config looks like:

module.exports = {
  module: {
    rules: [
      {
        test: /\.worker\.(js|ts)$/i,
        use: [{
          loader: 'comlink-loader',
          options: {
            singleton: true
          }
        },
        "ts-loader"]
      }
    ]
  }
}

Otherwise there will / may be build-failures like

 Module parse failed: Unexpected token (2:25)
    File was processed with these loaders:
     * ../../node_modules/comlink-loader/dist/comlink-worker-loader.js
    You may need an additional loader to handle the result of these loaders.
    | import { expose } from 'comlink';
    ...

gfoidl avatar Aug 11 '20 19:08 gfoidl

@gfoidl The README is correct in this regard, as it works if you write the comlink-loader first, as shown below.

module.exports = {
  module: {
    rules: [
      {
        test: /\.worker\.[tj]s$/,
        loader: "comlink-loader?singleton",
      },
      {
        test: /\.[tj]sx?$/,
        loader: "ts-loader",
        exclude: /node_modules/,
      }
    ]
  }
}

ahuglajbclajep avatar Aug 12 '20 15:08 ahuglajbclajep

Ahhh, that's good to know. Thank you very much! 👍 😃

gfoidl avatar Aug 12 '20 16:08 gfoidl