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

[DOC beta] Add API docs for the component lifecycle hooks. #12501

Closed
wants to merge 1 commit into from

Conversation

bmac
Copy link
Member

@bmac bmac commented Oct 18, 2015

@rwjblue
Copy link
Member

rwjblue commented Oct 18, 2015

Firstly, thank you for writing these API docs.

Secondly, I am unsure that we want these documented directly prior to angle bracket components landing. However, as that timeframe seems to be pushing out past the 2.4 era I feel that it is pretty important to allow folks to move forward today.

I'll try to get consensus on this from the rest of the core team so we can land these docs.

@@ -456,6 +456,182 @@ var Component = View.extend(TargetActionSupport, {
@public
@property positionalParams
*/

/**
`didInitAttrs` runs on the first render after a component was
Copy link
Member

Choose a reason for hiding this comment

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

This runs when the component is initialized (aka created), but before render. Technically, this is called from the components constructor so any properties set here properly shape the object.

@rwjblue
Copy link
Member

rwjblue commented Oct 18, 2015

We should also make it clear which of these are ran in a server environment (aka fastboot) and which are only ran in the browser. IIRC, didInsertElement, didUpdate, and didRender are not called via the visit API (but @tomdale / @chancancode / @wycats should confirm).

@bmac bmac force-pushed the lifecycle-hooks branch 2 times, most recently from f423d8b to ae276ff Compare October 18, 2015 23:29
@wycats
Copy link
Member

wycats commented Oct 19, 2015

I'd be ok with landing docs for these hooks that do not include any arguments, and I'd like to strip the arguments if possible.

@rwjblue
Copy link
Member

rwjblue commented Oct 19, 2015

@wycats - I'd be OK with stripping the args, but we would need to do the arity check and provide existing args with a deprecation for at least one LTS cycle.

@bmac - Can you remove reference to the arguments in these docs?

@wycats
Copy link
Member

wycats commented Oct 19, 2015

@rwjblue by the time a second LTS cycle goes by, there's a good chance we'll have the final args. Perhaps the right thing to do is to just emit a warning saying that the current args are unstable and should not be relied on.

@rwjblue
Copy link
Member

rwjblue commented Oct 19, 2015

@wycats - Ya, I'm happy either way. I just don't want to remove the args completely in a single version and therefore breaking any apps using them. Adding a warning if we detect the args are being used seems totally fine to me. In practice it will mean that folks just don't use the arguments, which is kinda what we want anyways.

@tomdale
Copy link
Member

tomdale commented Oct 19, 2015

Agreed on exposing these without arguments. @bmac, it would be nice if we could flesh these out a bit. Specifically, we should describe why you would want to use a particular hook and give concrete examples. We may also want to better describe the lifecycle in terms of the element and not just in relation to other hooks.

For example, this is didRender:

didRender runs after didInsertElement (it also runs on subsequent re-renders).

Implicitly, this means the component will always have an element by the time this hook gets called; we may want to make that fact explicit.

@artych
Copy link

artych commented Oct 19, 2015

I'd suggest to consider example based on ember-collection component - https://github.com/emberjs/ember-collection/blob/master/addon/components/ember-collection.js.
There are many hooks used there and it will be helpful to explain usage logic.
I think it's also helpful to land docs for getAttr and setAttr.

@workmanw
Copy link

I think this may go along with what @tomdale is saying, but it would be really helpful to explain when these things will actually fire. Not just "on rerender", but explaining under what circumstances it would rerender (IE list sorting, ... what else? That's really all I know). I think this would help to just orient yourself while reading, but also it would be good to know for testing purposes.

@rwjblue
Copy link
Member

rwjblue commented Apr 10, 2016

I think this was replaced by #13250, sorry for dropping the ball here @bmac.

@rwjblue rwjblue closed this Apr 10, 2016
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

6 participants