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 a simple observer readme section #23

Merged
merged 3 commits into from
Dec 27, 2017
Merged

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Dec 6, 2017

Updated the readme a tiny bit to explain how to do a simple observer (a property's observer).

would typescript complain about this? doesn't seem to locally but wasn't sure about accessing a private method on the prototype.

@@ -126,6 +126,14 @@ class TestElement extends Polymer.Element {
@property()
bar: string = 'yes';

// @observe does not support simple observers,
// you can already do that via a normal property
@property({ observer: TestElement.prototype.onBazChanged })
Copy link
Member

Choose a reason for hiding this comment

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

This depends on #18 right? That requires that observer is a string, which is also how Polymer normally works (AFAIK you can't pass a function reference in the normal Polymer block, or at least that's an undocumented feature if you can).

Copy link
Contributor Author

@43081j 43081j Dec 23, 2017

Choose a reason for hiding this comment

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

It depends on #18 (PropertyOptions has no observer in master). so yes we need to change it to string|((val: any, old: any) => void) or something i suppose, maybe in this PR?

You can pass a function reference as of very recently due to Polymer/polymer#4574. it doesn't seem to be documented well, if at all.

its probably best we recommend this, though, as its a little stronger typed than the string version.

Copy link
Member

Choose a reason for hiding this comment

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

It depends on #18 (PropertyOptions has no observer in master). so yes we need to change it to string|((val: any, old: any) => void) or something i suppose, maybe in this PR?

You can pass a function reference as of very recently due to Polymer/polymer#4574. it doesn't seem to be documented well, if at all.

Ah, I missed that change. Yes, would be great to also add the new signature to this PR, string|((val: any, old: any) => void) seems right.

BTW we should also update https://github.com/Polymer/polymer/blob/master/types/extra-types.d.ts to match https://github.com/Polymer/polymer/blob/master/externs/polymer-externs.js#L24, maybe in Polymer/polymer#5000?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup can do. i added the observer definition here too now.

@aomarks aomarks merged commit 40edb35 into Polymer:master Dec 27, 2017
@43081j 43081j deleted the observer-readme branch December 27, 2017 20:22
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.

2 participants