Add "computed" and "observer" attribute to @property decorator #18

danielvanmil · 2017-11-21T14:17:49Z

Add @computedProperty decorator

googlebot · 2017-11-21T14:17:52Z

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. 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.
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.

danielvanmil · 2017-11-21T14:19:32Z

I signed it!

googlebot · 2017-11-21T14:19:35Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
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.

43081j · 2017-11-21T23:18:15Z

This should probably be renamed as its actually support for computed in the @property decorator, rather than a standalone decorator.

Looks good though, nice PR 👍

43081j · 2017-11-21T23:19:58Z

src/decorators.ts

@@ -38,6 +38,7 @@ export interface PropertyOptions {
  notify?: boolean;
  reflectToAttribute?: boolean;
  readOnly?: boolean;
+  computed: string;


This should probably be optional like the others

43081j · 2017-12-19T11:01:31Z

@aomarks can we get this merged?

@danielvanmil can you rebase onto master to resolve the conflicts? maybe while you're in here too, it could be worth adding value to PropertyOptions as that is missing too (i guess its any).

aomarks · 2017-12-22T23:59:59Z

CHANGELOG.md

@@ -2,6 +2,12 @@

 ## [Unreleased]

+## [0.1.2-alpha.2] - 2017-11-28
+- Added "observer" attribute to @property annotation


Can you move both items to the unreleased section and remove these alpha releases?

aomarks · 2017-12-23T00:00:05Z

bower.json

@@ -1,7 +1,7 @@
 {
  "name": "polymer-decorators",
  "homepage": "https://github.com/Polymer/polymer-decorators",
-  "version": "0.1.1",
+  "version": "0.1.2-alpha.2",


Don't modify version.

aomarks · 2017-12-23T00:00:08Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "@polymer/decorators",
-  "version": "0.1.1",
+  "version": "0.1.2-alpha.2",


Don't modify version.

aomarks · 2017-12-23T00:01:15Z

src/decorators.ts

@@ -52,7 +54,9 @@ export function property(options?: PropertyOptions) {
    const reflectToAttribute: boolean =
        options && options.reflectToAttribute || false;
    const readOnly: boolean = options && options.readOnly || false;
-
+    const computed: string = options && options.computed || "";


Can you run npm run format? Should switch quotes to single.

danielvanmil · 2017-12-27T17:50:43Z

rebased

danielvanmil · 2017-12-27T18:01:24Z

processed "code review"

danielvanmil · 2017-12-27T18:02:22Z

@43081j: please merge

43081j · 2017-12-27T18:44:26Z

test/integration/decorators.ts

@@ -74,6 +74,18 @@ suite('TypeScript Decorators', function() {
      const propValue = testElement.readOnlyString;
      chai.assert.equal(propValue, 'initial value');
    });
+
+    test('computed property should should return computed value', function() {


just a tiny typo here, should should

43081j · 2017-12-27T18:45:07Z

test/integration/decorators.ts

@@ -74,6 +74,18 @@ suite('TypeScript Decorators', function() {
      const propValue = testElement.readOnlyString;
      chai.assert.equal(propValue, 'initial value');
    });
+
+    test('computed property should should return computed value', function() {
+      testElement.computedString = "new value";


i guess this should be setting reflectedString to yahoo? so the following computed result is computed yahoo? by the looks of the test element source.

43081j · 2017-12-27T18:45:38Z

test/integration/elements/test-element.ts

@@ -9,7 +9,7 @@
 * rights grant found at http://polymer.github.io/PATENTS.txt
 */

-/// <reference path="../bower_components/polymer-decorators/global.d.ts" />
+/// <reference path="bower_components/polymer-decorators/global.d.ts" />


your build fails because this path can't be found, probably just needs setting back to ../?

43081j · 2017-12-27T18:46:12Z

test/integration/elements/test-element.ts

@@ -18,9 +18,6 @@ class TestElement extends Polymer.Element {
  @property({notify: true})
  aNum: number = 42;

-  @property()
-  doesntNotify: boolean = true;


there's a failing test causing your build to fail which depends on this. maybe just restore this property

43081j · 2017-12-27T18:47:04Z

added a couple of comments to help you get the CI build passing. but other than those small changes, looks good to me.

@aomarks can merge if he approves too

aomarks · 2017-12-27T20:14:55Z

Merged. Thanks @danielvanmil and @43081j !

added computed property

dd72844

43081j reviewed Nov 21, 2017

View reviewed changes

43081j mentioned this pull request Nov 22, 2017

add @computed decorator #20

Merged

danielm added 2 commits November 23, 2017 10:21

make computed also optional

20771dc

Prepare 0.1.2-alpha.1 release

277829b

danielvanmil changed the title ~~Add @computedProperty decorator~~ Add @computed attribute to @property decorator Nov 23, 2017

danielvanmil changed the title ~~Add @computed attribute to @property decorator~~ Add "computed" attribute to @property decorator Nov 23, 2017

danielvanmil added 4 commits November 23, 2017 11:50

Set correct email

7ccdca9

fixed test

6339e90

add observe attribute to @Property

a893833

prepare release

743dcfd

danielvanmil changed the title ~~Add "computed" attribute to @property decorator~~ Add "computed" and "observer" attribute to @property decorator Nov 28, 2017

justinfagnani mentioned this pull request Dec 21, 2017

Decide on how to implement computed properties #25

Closed

aomarks requested changes Dec 23, 2017

View reviewed changes

aomarks mentioned this pull request Dec 23, 2017

add a simple observer readme section #23

Merged

danielvanmil closed this Dec 27, 2017

danielvanmil force-pushed the master branch from 743dcfd to 9bcac25 Compare December 27, 2017 17:07

Add "computed" and "observer" attribute to @Property decorator

6109eab

danielvanmil reopened this Dec 27, 2017

danielvanmil added 4 commits December 27, 2017 18:57

removed versions

7f2e696

ran formatter

e6bc5a5

rollback version

a854447

rollback version

e370598

danielvanmil closed this Dec 27, 2017

danielvanmil reopened this Dec 27, 2017

43081j reviewed Dec 27, 2017

View reviewed changes

danielvanmil added 2 commits December 27, 2017 20:39

fixed path and missing change

14a6494

fixed typo

513744d

aomarks merged commit d6b3fa3 into Polymer:master Dec 27, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "computed" and "observer" attribute to @property decorator #18

Add "computed" and "observer" attribute to @property decorator #18

danielvanmil commented Nov 21, 2017

googlebot commented Nov 21, 2017

danielvanmil commented Nov 21, 2017

googlebot commented Nov 21, 2017

43081j commented Nov 21, 2017

43081j Nov 21, 2017

43081j commented Dec 19, 2017

aomarks Dec 22, 2017

aomarks Dec 23, 2017

aomarks Dec 23, 2017

aomarks Dec 23, 2017

danielvanmil commented Dec 27, 2017

danielvanmil commented Dec 27, 2017

danielvanmil commented Dec 27, 2017

43081j Dec 27, 2017

43081j Dec 27, 2017

danielvanmil Dec 27, 2017

43081j Dec 27, 2017

43081j Dec 27, 2017

43081j commented Dec 27, 2017

aomarks commented Dec 27, 2017

Add "computed" and "observer" attribute to @property decorator #18

Add "computed" and "observer" attribute to @property decorator #18

Conversation

danielvanmil commented Nov 21, 2017

googlebot commented Nov 21, 2017

danielvanmil commented Nov 21, 2017

googlebot commented Nov 21, 2017

43081j commented Nov 21, 2017

Choose a reason for hiding this comment

43081j commented Dec 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielvanmil commented Dec 27, 2017

danielvanmil commented Dec 27, 2017

danielvanmil commented Dec 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

43081j commented Dec 27, 2017

aomarks commented Dec 27, 2017