GoogleChromeLabs / comlink

Comlink makes WebWorkers enjoyable.
Apache License 2.0
11k stars 382 forks source link

Improve two-way communication test #621

Open Nemikolh opened 1 year ago

Nemikolh commented 1 year ago

Hello! :wave:

TLDR;

This PR improves the two-way communication test to fail if WireValueType and MessageType would happen to overlap again in the future.

Full story

First, I want to thank you for your work on Comlink. This is one of the most elegant, well written library I've ever come across :star: :clap:

I recently noticed that the test for two-way communication does not do enough work to actually fail in versions of Comlink < 4.3.1. I encountered this issue while trying to port two-way communication to our fork which is stuck at 4.3.0.

To see how this PR helps you can use this bash script: ```bash # To run this, you need jq, npm, git # Checkout this branch BRANCH=fix-two-way-iframe-test git remote add Nemikolh git@github.com:Nemikolh/comlink.git git fetch Nemikolh git checkout $BRANCH || { echo "Oopsie"; exit 1; } # Hacky way to keep our test around mkdir -p tmp/fixtures cp tests/tsconfig.json tmp/ cp tests/type-checks.ts tmp/ cp tests/two-way-iframe.comlink.test.js tmp/ cp tests/fixtures/two-way-iframe.html tmp/fixtures/ cp package.json package.json.tmp cp tsconfig.json tsconfig.json.tmp cp rollup.config.mjs rollup.config.mjs.tmp # Checkout v4.3.0 git checkout v4.3.0 || { echo "Oopsie"; exit 1; } # Make sure we run our latest tests rm -rf tests && mv tmp tests mv package.json.tmp package.json mv rollup.config.mjs.tmp rollup.config.mjs && rm rollup.config.js mv tsconfig.json.tmp tsconfig.json # Use older TypeScript version (otherwise build fails) echo "$(jq '.devDependencies.typescript = "4.6.4"' package.json)" > package.json # Run those tests on v4.3.0 npm install npm test # Cleanup rm rollup.config.mjs git clean -f tests git restore tests git restore package.json git restore tsconfig.json git restore rollup.config.js # Move back to the branch and run the tests again git checkout $BRANCH npm install npm test ```

If you run this script you'll see this error in your console (you might need to scroll up a bit :scroll:):

AssertionError: expected DOMException{ 
     stack: 'Error: Failed to execute \'postMessage\' on \'Window\': 
                 A MessagePort could not be cloned because it was not transferred.\n   
      at Object.postMessage (dist/esm/comlink.mjs:240:48)\n    
      at dist/esm/comlink.mjs:126:16' 
} to equal ''
at Context.<anonymous> (tests/two-way-iframe.comlink.test.js:61:22)

If you run the script again but replace BRANCH=fix-two-way-iframe-test by origin/main (or equivalent), you can see that the test pass (you'll need to scroll up again to view the result of the first karma start):

23 02 2023 19:46:55.221:INFO [launcher]: Starting browser ChromeHeadless
23 02 2023 19:46:55.479:INFO [Chrome Headless 110.0.5481.100 (Linux x86_64)]: Connected on socket MGZ7B5cMMyiLbvGDAAAB with id 54251662
Chrome Headless 110.0.5481.100 (Linux x86_64): Executed 1 of 1 SUCCESS (0.019 secs / 0.005 secs)

This shows that the test is missing a few bits.