require-post-message-target-origin shouldn't trigger for incompatible APIs

[GauBen]

What's wrong

I'm currently developing a browser extension that communicates with native software:

import { browser, Runtime } from 'webextension-polyfill-ts'
const nativePort: Runtime.Port = browser.runtime.connectNative(APPLICATION_ID)
nativePort.postMessage({ cmd: 'Version' }) // (x) Missing the `targetOrigin` argument.

Which rule is buggy

require-post-message-target-origin is triggered but the method only accepts one argument (see runtime.Port).

Ideas

1. Enforce the name of the variable to be window, document or the likes.
2. Check the type of the variable or the existence of the import 'webextension-polyfill'/'webextension-polyfill-ts'.
3. Add a warning for web extension developers here: https://github.com/sindresorhus/eslint-plugin-unicorn/blob/v34.0.1/docs/rules/require-post-message-target-origin.md

[fisker]

Sorry, didn't know this.

As I understand, runtime.Port don't have to come from 'webextension-polyfill'/'webextension-polyfill-ts', right?

[GauBen]

You understood right, 'webextension-polyfill' (https://github.com/mozilla/webextension-polyfill) is just something that helps erasing differences between chromium and gecko. Especially, chrome.runtime becomes browser.runtime, as asked by the web extension standard.

[fisker]

Do you think disable this rule in your file or project is a good idea? If you are developing a browser extension, you probably don't need call window.postMessage.

[GauBen]

Yep, you're right, I shouldn't be using window.postMessage (https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage#using_window.postmessage_in_extensions_non-standard_inline)
I'll disable the rule in my eslintrc.

Thanks for the quick response!

[fisker]

But is it common to use worker in extension? In that case people may need self.postMessage?

[fisker]

Wait, worker.postMessage second parameter is not targetOrigin....

[fisker]

There is also MessagePort.postMessage(), Client.postMessage(), BroadcastChannel.postMessage()

[fisker]

I think we should turn this rule off...

[GauBen]

I'm not sure why this rule exists, the parameter targetOrigin is not optionnal, and the error is properly caught by tsc:

Expected 2-3 arguments, but got 1. ts(2554)
lib.dom.d.ts(18289, 31): An argument for 'targetOrigin' was not provided.

[fisker]

Our rule should support JavasScript,

When calling window.postMessage() without targetOrigin argument, browser won't throw any error, but message can't receive by any window. I remember the first time I got into this, really drive me crazy.

#1307 (comment)

[GauBen]

Ok got it, the problem is more that browsers fail silently instead of throwing an exception. What if this rule only triggers for window.postMessage by default in unicorn/recommended, and add an option to trigger on any variable like foo.postMessage?

rules: {
  'unicorn/require-post-message-target-origin': ['error', 'window' | 'any'],
}

[JounQin]

This rule should not trigger on worker_threads, either.

const msg: MainToWorkerMessage = { sharedBuffer, id, args }
new Worker('', {}).postMessage(msg)

[fregante]

I think this rule is safe only if types are available:

• Does the postMessage function have a targetOrigin argument? Then report the error

Can that be done?

[fisker]

Can that be done?

I don't think so, without type info, we can't know where the postMessage from.

[fregante]

I think this rule is safe only if types are available:

👆 Yes I'm talking about a TypeScript-only rule. My question was about looking into the types of the to ensure that the matches our expectation.

[fisker]

That will be meaningless if this #1396 (comment) is true.

[fregante]

That will be meaningless if this #1396 (comment) is true.

Indeed it is! So this rule can be dropped altogether as far as I'm concerned.

[JounQin]

That will be meaningless if this #1396 (comment) is true.

So require-post-message-target-origin should be disabled by default for .ts files in the recommended config.

[fisker]

Reopen this since {Worker,MessagePort,Client,BroadcastChannel}#postMessage() and runtime.Port#postMessage() still false positive in pure JS parsers.

[fisker]

@2000yeshu What's your solution for this?