mantoni / mochify.js

☕️ TDD with Browserify, Mocha, Headless Chrome and WebDriver
MIT License
346 stars 57 forks source link

Wrap webdriver script content in IIFE, removing need for evaluateReturn #275

Closed m90 closed 2 years ago

m90 commented 2 years ago

Picks up: https://github.com/mantoni/mochify.js/pull/232/files/5aecae9f8be4f783f27877808fe4496481cc71dd#r667336402

Just prepending return seems to limit ourselves to immediately return the first expression in the given script, but wrapping the script in an IIFE seems to work as expected.

This should probably merged after #274 so it can be rebased against rewrite before and pass the tests (it does pass locally for me).

mantoni commented 2 years ago

From my investigation you're right, all injected scripts are wrapped in a function call. Therefore the wrapper isn't needed and we can simply use return directly.

I've allowed myself to add two commits to this PR.

1) The first one is a refinement / alternative to the return based implementation, where I'm now calling executeAsyncScript. The "invisible" function wrapped used in these invocations received a callback as the last argument. I'm also happy to go with a simple synchronous return instead, just wanted to let you know. What do you think? 2) The second moves wrapping the client in a self-invoking function into setup-client, so mostly whitespace changes. I figured this makes the client easier to read and we can control the final semicolon.

m90 commented 2 years ago

Looks good to me as well, that makes it pretty clear what's going on now. I'd have no objections against merging this.

m90 commented 2 years ago

I rebased the branch so it also contains the integration tests for webdriver, seems everything is passing.

mantoni commented 2 years ago

Great! Thank you 🙏