API consistency: property getters vs methods #280
Comments
ebidel
changed the title
|
I'm fine with the others being getters.
|
We can't have everything as getters/setters since some of them are async.
Agreed. Some time ago we decided on a simple rule to tell if something is a getter or a method:
The rule is currently broken in a few places for different historic reasons, and we weren't proactive enough to fix it. E.g. If there are no objection to this codestyle, i'm happy to fix all the outliers. |
|
Thanks for the explanation. Agreed that async calls should remain methods that return promises.
A nice side effect I hadn't thought of! I guess another reason to keep methods everywhere is for later when if/when we add chaining. Chaining methods is more natural.
@aslushnikov, can you drop that in at the top of the API docs if we're sticking with those rules?They'll be new to everyone else coming into the project :) |
|
@aslushnikov can we bring this back, address API consistency for 1.0. Good first bug is fine.
Yes please! Just ran into this again with |
|
@ebidel sg! |
The patch converts all the getters in the codebase into the methods. For example, the `request.url` getter becomes the `request.url()` method. This is done in order to unify the API and make it more predictable. The general rule for all further changes would be: - there are no getters/fields exposed in the api - the only exceptions are "namespaces", e.g. `page.keyboard` Fixes puppeteer#280. BREAKING CHANGE: This patch ditches getters and replaces them with methods throughout the API. The following methods were added instead of the fields: - dialog.type() - consoleMessage.args() - consoleMessage.text() - consoleMessage.type() - request.headers() - request.method() - request.postData() - request.resourceType() - request.url() - response.headers() - response.ok() - response.status() - response.url()
The patch converts all the getters in the codebase into the methods. For example, the `request.url` getter becomes the `request.url()` method. This is done in order to unify the API and make it more predictable. The general rule for all further changes would be: - there are no getters/fields exposed in the api - the only exceptions are "namespaces", e.g. `page.keyboard` Fixes #280. BREAKING CHANGE: This patch ditches getters and replaces them with methods throughout the API. The following methods were added instead of the fields: - dialog.type() - consoleMessage.args() - consoleMessage.text() - consoleMessage.type() - request.headers() - request.method() - request.postData() - request.resourceType() - request.url() - response.headers() - response.ok() - response.status() - response.url()
Conversation from #272 (comment). Some properties of the public API are exposed as methods:
browser.version()page.protocolWSEndpoint(),page.url(),page.viewport()frame.title(),frame.url(),frame.name()Others are properties:
page.mouse,page.tracingdialog.typerequest.url,request.urlresponse.urlQ: Is there a reason all of these aren't ES5 getters? At the very least, we should be consistent across the API surface.
The text was updated successfully, but these errors were encountered: