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

[BUGFIX beta] Fix basic component and helper resolution in engines. #14135

Merged
merged 3 commits into from
Aug 27, 2016

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Aug 26, 2016

Rebase and update of #14112 (to use symbolTable throughout) which is a rebase of #14055.

Contains the following fixes:

  • Clone additional parent dependencies into engine instances. Specifically service:-glimmer-environment (only for glimmer) and renderer:-dom or renderer:-inert
  • Ensure that Cache lookups are performed using the correct owner.
  • Ensure that engines are destroyed before top-level views so that top-level
    views are not re-instantiated during engine teardown.

dgeb and others added 2 commits August 26, 2016 13:57
…ces.

An engine instance needs to share the following with its parent:

* `service:-glimmer-environment` (only for glimmer)
* `renderer:-dom` or `renderer:-inert`, depending on whether the environment
  is interactive
Ensure that Cache lookups are performed using the correct owner.

Ensure that engines are destroyed _before_ top-level views so that top-level
views are not re-instantiated during engine teardown.
@asakusuma
Copy link
Contributor

Looks like Applications with autoboot set to false do not autoboot is failing but only in Firefox and with feature flags off.

@Serabe
Copy link
Member

Serabe commented Aug 26, 2016

@asakusuma it seems to be failing only sometimes for some merges :$
image

@rwjblue
Copy link
Member Author

rwjblue commented Aug 26, 2016

Yes, we have an intermittent issue with FF_Current.

@rwjblue
Copy link
Member Author

rwjblue commented Aug 26, 2016

I updated the owner specific stuff to ensure that we do not use the environments owner (which in the case of engines would be the consuming apps not the engines). Still need to add a couple tests around this (where the engine and the app both have a given component/helper but the "right" one is used in each context).

@rwjblue rwjblue force-pushed the glimmer-owner branch 3 times, most recently from aafd6fe to 02814c2 Compare August 27, 2016 03:10
Ensure that the owner being used is not the environments default
`Owner`, but it is the owner that the template is being looked up by.

For late bound templates this means the components owner, and for normal
templates (looked up from registry) it is passed to the factories
`.create` method by the container itself.
@rwjblue rwjblue merged commit e19a71d into master Aug 27, 2016
@rwjblue rwjblue deleted the glimmer-owner branch August 27, 2016 03:35
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.

4 participants