I’d like additional input on this PR. Number one: What is the best way to resolve the Cyprus anti-pattern of assigning return values? Number 2: Regarding const addEventListener = win.EventTarget.prototype.addEventListener;
is it actually violating the eslint unbound-method rule? If my hypothesis is wrong, then what is the correct way to write it?
Hello there,
Some things I would do:
- Fix the type issue with
onBeforeLoad
- there is no point passing in a function that takes4
args, when the caller only expects1
. - Dig into Cypress docs to see what methods already exist to wait for a page to load. The whole workflow seems off to me. The reliance on a global variable throws a red flag. The function’s purpose is just to signal (wait) for the page to load, and to do this does some weird stuff with globals (
addEventListener
) - this is what the TS error is going on about, because it is dangerous to play with the references the way that code is. This all seems unnecessary.
Actually, thinking about it, I would first try see why the blamer added this in the first place, by removing it entirely, and seeing if the test works without it. We might have needed it with an older version of Cypress, and now do not
Hope this helps