Skip to content

Detecting lifecycle - #2180

Merged
vsavkin merged 3 commits into
angular:masterfrom
vsavkin:detecting_lifecycle
May 27, 2015
Merged

Detecting lifecycle#2180
vsavkin merged 3 commits into
angular:masterfrom
vsavkin:detecting_lifecycle

Conversation

@vsavkin

@vsavkin vsavkin commented May 27, 2015

Copy link
Copy Markdown
Contributor

Closes #2120

/cc @yjbanov

This PR contains only the reflective version of the feature. @kegluneq will have to update the transformer to update the registered type info after this PR is merged. I use the same reflector object, so I hope it won't be very difficult.

@vicb

vicb commented May 27, 2015

Copy link
Copy Markdown
Contributor

I don't think this "Closes #2120", more discussion needs to happen there ?

@vsavkin
vsavkin force-pushed the detecting_lifecycle branch from 921f29f to be71672 Compare May 27, 2015 15:24
@vsavkin

vsavkin commented May 27, 2015

Copy link
Copy Markdown
Contributor Author

@vicb The OnChange interface is already in master. This PR does not change the name. What else apart from names require the discussion?

@vicb

vicb commented May 27, 2015

Copy link
Copy Markdown
Contributor

The structural type check for JS ?

@vsavkin
vsavkin force-pushed the detecting_lifecycle branch from be71672 to 4394394 Compare May 27, 2015 16:06
@vsavkin

vsavkin commented May 27, 2015

Copy link
Copy Markdown
Contributor Author

Misko and I talked about it yesterday and decided on that. This is the only way to do checks in JS, and you can always override the behavior using the lifecycle property in the annotation.

Do you think this will be confusing?

@vicb

vicb commented May 27, 2015

Copy link
Copy Markdown
Contributor

Why don't we drop the lifecycle property then ? (keeping both is confusing IMO).

@vsavkin

vsavkin commented May 27, 2015

Copy link
Copy Markdown
Contributor Author

We need to be able to detect this info statically. If you add onChange reflectively, then you need a way to tell Angular that onChange will be there.

@vsavkin
vsavkin force-pushed the detecting_lifecycle branch from 4394394 to 6eba14b Compare May 27, 2015 16:59
@vicb

vicb commented May 27, 2015

Copy link
Copy Markdown
Contributor

LGTM when Travis is green.
As discussed we'll open an issue to discuss the structural type check/renaming

@vicb vicb added @lgtm action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels May 27, 2015
@vsavkin
vsavkin force-pushed the detecting_lifecycle branch 2 times, most recently from a9ff18a to ebefd7e Compare May 27, 2015 21:19
@vsavkin
vsavkin force-pushed the detecting_lifecycle branch from ebefd7e to 09af57c Compare May 27, 2015 23:10
@vsavkin
vsavkin force-pushed the detecting_lifecycle branch from 09af57c to 30b6542 Compare May 27, 2015 23:23
@vsavkin
vsavkin merged commit 30b6542 into angular:master May 27, 2015
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 6, 2019

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

See More

@mhevery mhevery

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Interface-based lifecycle methods

4 participants

@vsavkin @vicb @mhevery @googlebot