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

Cleanup fastboot hooks and misc view cleanup #14361

Merged
merged 6 commits into from
Sep 27, 2016
Merged

Conversation

chancancode
Copy link
Member

Fixes #14353

We should not be using this class directly
We do not allow apps to accessing `this.element` in non-interactive
environments, such as FastBoot. This was enforced by not calling hooks
that would expose the “element” (which is often not a real DOM element)
such as `didInsertElement`. However, since we sometimes run “update”
hooks (such as `didUpdateAttrs`) that happen after the element has been
set, there is a chance that we would leak the “element” inadvertently.

This fixes the issue by making `this.element` a getter that throws in
non-interactive mode. To address cases where apps are relying on the
current accidental behavior, we added `ViewUtils.getViewElement` as a
private (“intimate”) API to forcefully retrieve the “element”. This
should balance the need for warning against incidental usage and still
provide a workaround for cases where the fake element is needed, such as
custom serializations.

As always, using private (“intimate”) APIs are strongly discouraged, not
fully supported and might not be unreliable. However, we will try our
best to keep it around and deprecate between LTS releases before
removing it.

Note: the main issue here is that the “element” you get back is likely
not a real DOM element with an undocumented set of APIs. This core issue
is not really addressed here and the supported APIs and internals of the
returned object is subject to change.
@chancancode chancancode added this to the 2.9.0 milestone Sep 27, 2016
This is a follow-up to #14233.

At the September 2016 core team face-to-face meeting, we discussed and
clarified the intended features for life-cycle hooks in non-iterative
environments (i.e. FastBoot).

We agreed that components on FastBoot should only react to data changes
via the `didReceiveAttrs` and `didUpdateAttrs` hooks. (There is also
deprecated `didInitAttrs` hook.)

The main change is that we have been incorrectly calling `willRender`
and `willUpdate` in non-interactive mode so far (at least since we last
checked and added the tests). The problem is that `{will,did}Render` and
`{will,did}Update` are intended to work as a pair so running one but not
the other could introduce unexpected bugs for applications.
The deprecated `Ember.View` has been removed a while ago. The exports
in this file is not used anywhere inside the `ember-views` package and
also not re-exported from the package index. Contrary to the outdated
documentation, we don’t export the `Ember.View` global also, so this is
just dead code.

I kept the file around for now until we finish migrating the
documentation.
@chancancode chancancode force-pushed the cleanup-fastboot-hooks branch from 6fd3130 to daab186 Compare September 27, 2016 04:16
@rwjblue rwjblue merged commit ca66d19 into master Sep 27, 2016
@rwjblue rwjblue deleted the cleanup-fastboot-hooks branch September 27, 2016 10:42
@rwjblue
Copy link
Member

rwjblue commented Sep 27, 2016

Thank you @chancancode!

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