core(page-functions): remove all *String exports
#14374
Conversation
core/lib/page-functions.js
Outdated
| getNodeDetails.dependencies = [ | ||
| getNodePath, | ||
| getNodeSelector, | ||
| getBoundingClientRect, | ||
| getOuterHTMLSnippet, | ||
| getNodeLabel, | ||
| ]; |
There was a problem hiding this comment.
What if we just override the toString function on getNodeDetails to include the deps? It's the only place we would need to do it.
There was a problem hiding this comment.
Tacking on a property to Function is iffy enough, I draw the line at overriding Object.prototype.toString :)
There was a problem hiding this comment.
In the realm of js hackery, though, it does seem much more idiomatic to override a function's toString to customize what happens when it's serialized to a string than to have a magic property for it. That's toString's literal purpose.
There was a problem hiding this comment.
Tacking on a property to Function is iffy enough, I draw the line at overriding Object.prototype.toString
I wasn't suggesting overriding Object.prototype.toString. I was suggesting we do getNodeDetails.toString = () => {...} or something
There was a problem hiding this comment.
I know, s/override/shadow
This reverts commit 1725988.
4 participants
All but one of these can be dropped just by using their function directly.
getNodeDetailsis special because it has a number of other page-functions that must be included to work, which was solved by making its*Stringexport wrap those deps (so callers wouldn't have to remember to). Instead,I introducewe setExecutionContext.serializeDependenciesandsomeFn.dependenciesto do the same.toString