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 more explanation for page.evaluateHandle() #1867

Merged
merged 10 commits into from Feb 22, 2018

Conversation

yelisaveta
Copy link
Contributor

No description provided.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@yelisaveta yelisaveta changed the title Add more explanation for page.evaluateHandle() #1014 Add more explanation for page.evaluateHandle() Jan 21, 2018
@yelisaveta
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

docs/api.md Outdated
@@ -695,22 +695,21 @@ List of all available devices is available in the source code: [DeviceDescriptor
#### page.evaluate(pageFunction, ...args)
- `pageFunction` <[function]|[string]> Function to be evaluated in the page context
- `...args` <...[Serializable]|[JSHandle]> Arguments to pass to `pageFunction`
- returns: <[Promise]<[Serializable]>> Resolves to the return value of `pageFunction`
- returns: <[Promise]<[Serializable]>> Promise which resolves to the return value of `pageFunction` as a javascript-object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JavaScript object.


// to show the difference between two methods
console.log(`jsObject is NOT equal to jsHandle: ${jsObject !== jsHandle}`); // prints true
console.log(`jsObject is equal to jsHandle.jsonValue(): ${jsObject === (await jsHandle.jsonValue())}`); // prints true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the best result to show int the demo, but I can't think of anything better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, this example might just be part of the docs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea. I'm also thinking we should just have these snippets and explanation as part of the docs. It's a less compelling standalone demo.

@yelisaveta
Copy link
Contributor Author

@ebidel please take a look now: I've removed the example file and moved it to the api.md.

docs/api.md Outdated

The only difference between `page.evaluate` and `page.evaluateHandler` is that `page.evaluate` returns JSON representation of the object and `page.evaluateHandler` returns in-page object (JSHandle):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

evaluateHandler -> evaluateHandle on both of these.

docs/api.md Outdated
const aWindowHandle = await page.evaluateHandle(() => Promise.resolve(window));
aWindowHandle; // Handle for the window object.
```
// returns JavaScript object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"returns a JSON object" to be consistent with your description.

docs/api.md Outdated

The only difference between `page.evaluate` and `page.evaluateHandler` is that `page.evaluate` returns JSON representation of the object and `page.evaluateHandler` returns in-page object (JSHandle):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returns JSON representation -> returns a JSON representation

@yelisaveta
Copy link
Contributor Author

@ebidel I've updated the PR, please take a look. Thanks!

@yelisaveta
Copy link
Contributor Author

Hi @ebidel @JoelEinbinder this PR is ready to be reviewed again, very quick one. Thanks.

Copy link
Contributor

@aslushnikov aslushnikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this.

@@ -729,19 +728,26 @@ Shortcut for [page.mainFrame().evaluate(pageFunction, ...args)](#frameevaluatepa
#### page.evaluateHandle(pageFunction, ...args)
- `pageFunction` <[function]|[string]> Function to be evaluated in the page context
- `...args` <...[Serializable]|[JSHandle]> Arguments to pass to `pageFunction`
- returns: <[Promise]<[JSHandle]>> Resolves to the return value of `pageFunction`
- returns: <[Promise]<[JSHandle]>> Promise which resolves to the return value of `pageFunction` as in-page object (JSHandle)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many evaluateHandle calls: page.evaluateHandle, frame.evaluateHandle and elementHandle.evaluateHandle. We try to keep documentation consistent between them all; so all the changes has to be mirrored there as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I will update there as well.

docs/api.md Outdated

// both lines below print true
console.log(`jsObject is NOT equal to jsHandle: ${jsObject !== jsHandle}`);
console.log(`jsObject is equal to jsHandle.jsonValue(): ${jsObject === (await jsHandle.jsonValue())}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is actually the case: triple-equals checks for identity equality, which is not the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But identity equality (aka strict equality) means that type and values are the same. In this case it shows that jsObject is the same type and value as jsHandle.jsonValue().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yelisaveta in case of non-primitives, it means that two objects are the same by identity, not by value.

e.g.:

console.log({} === {}); // false
let a = {};
let b = a;
let c = a;
console.log(b === c); // true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for a delay - I've now understood why I used strict equality check - initially I've created an example to demonstrate the difference between these two methods, see here: 53ae964

Examples were using pageFunction which returned element.innterHTML - which is a String. That's why strict equality worked correctly for me.

I see that having these snippets in the docs without the pageFunction definition looks confusing.

docs/api.md Outdated
const jsHandle = await page.evaluateHandle(pageFunction, elementHandle);

// both lines below print true
console.log(`jsObject is NOT equal to jsHandle: ${jsObject !== jsHandle}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd drop this console.log – even two consecutive page.evaluate will result in strict equality failing as well:

const jsObject1 = await page.evaluate(pageFunction, elementHandle);
const jsObject2 = await page.evaluate(pageFunction, elementHandle);
console.log(jsObject1 === jsObject2); // false, unless pageFunction returns primitives

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as my comment above - it was working for me since I was using the pageFunction that returns element.innterHTML.

@yelisaveta
Copy link
Contributor Author

@aslushnikov PR has been updated, please take a look. Thank you.

@aslushnikov aslushnikov merged commit 090590a into puppeteer:master Feb 22, 2018
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

Successfully merging this pull request may close these issues.

None yet

4 participants