Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add workaround for Function.toString() to docs #163

Closed
saucesteals opened this issue Oct 31, 2021 · 9 comments
Closed

Add workaround for Function.toString() to docs #163

saucesteals opened this issue Oct 31, 2021 · 9 comments

Comments

@saucesteals
Copy link

saucesteals commented Oct 31, 2021

Lots of libraries rely on Function.toString() to function, so I think it's important to provide some sort of "hacky" solution to be able to use bytenode with the lib.

Ref: #93
Not only can you not use functions with puppeteer, but puppeteer now wraps all the string options (ex. waitForSelector("selector")) in a function, then calls .toString() on the wrapped function (or one that you've provided) before sending it to the browser

Ref: #34 (comment)
Framework uses Function.toString() to check if Function is a class

As mentioned in #93 (comment) , a workaround could be just importing the function from a plain .js file, and while this will work, it may be a little unnecessary and annoying at times

A simple fix that I'd like to propose to get added to the documentation (or at least a reference to this issue) is the ability to override .toString() after bytenode breaks it

For example, to make a quick puppeteer fix:

const makeUnsafeFunction = (fnString) => {
    const fn = new Function(fnString);
    fn.toString = () => fnString;
    return fn
}

const unsafeWaitForSelector = (selector, opts) => {
    return page.waitForSelector(makeUnsafeFunction(`() => document.querySelector("${selector}")`), opts);
}

await unsafeWaitForSelector("#selector")

Or a fix for checking if a Function is a class:

class YourClass {}

YourClass.toString = () => "class YourClass"

This is in hopes to save people's time figuring out a good solution (because I know it would've saved mine 😅)

@OsamaAbbas
Copy link
Collaborator

I like the idea. Thank you for your contribution. I will add it to the README file later.

@OsamaAbbas
Copy link
Collaborator

Another suggestion that occured to me based on your idea: we may make bytenode extend the Function.prototype to add a method like .preserveSourceCode(). I have a general idea now of what is should look like and do, but I will implement it later.

The end goal is to make your examples like this:

page.waitForSelector((() => document.querySelector("${selector}")).preserveSourceCode(), opts);

and

class YourClass {}

YourClass.preserveSourceCode();

@saucesteals
Copy link
Author

Definitely interesting but my guess is that there isn't gonna be a simple way to approach this and would have to do actual ast parsing - and at that point, it may as well just be comments on top of function declarations that preserve the source code since that seems to be what a lot of people have mentioned in past issues

@OsamaAbbas
Copy link
Collaborator

You are probably correct. I wrote my previous comment half asleep!

@saucesteals
Copy link
Author

I think it could be relatively easy though considering how easy it is to parse and walk the js ast tree with modern packages
Purely a guess so correct me if I'm wrong but if the cached code just relies on some offset/position from the source that it tries to lookup when calling Function.prototype.toString, it should be relatively easy to just move the function (or even just the necessary parts of it) to the position in the dummy code that v8 would look into

And to be honest, since this feature is ever so rarely actually needed, you could just get away with two comments, one that marks the start of the source that you want to be preserved and one at the end. Do some simple regex matching or even pure js splitting and move those parts to the dummy code

@OsamaAbbas
Copy link
Collaborator

I added your workaround in the README file. Thank you.

@zhangriyueming
Copy link

I think it could be relatively easy though considering how easy it is to parse and walk the js ast tree with modern packages Purely a guess so correct me if I'm wrong but if the cached code just relies on some offset/position from the source that it tries to lookup when calling Function.prototype.toString, it should be relatively easy to just move the function (or even just the necessary parts of it) to the position in the dummy code that v8 would look into

And to be honest, since this feature is ever so rarely actually needed, you could just get away with two comments, one that marks the start of the source that you want to be preserved and one at the end. Do some simple regex matching or even pure js splitting and move those parts to the dummy code

Check here already a solution.

@DanielHougaard
Copy link

Please add an actual implementation of toString(). This approach makes it harder and harder to maintain your codebase when you need to edit the code inside the string.

@zhouzhili
Copy link

Is the preserveSourceCode method implemented?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants