-
-
Notifications
You must be signed in to change notification settings - Fork 55
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
Composing a view can access the parent property #222
Comments
@leon-andria yep, compose inherits from its parent. In any case, someone from the core team should be more suitable for the final answer |
in fact, according to this https://github.com/aurelia/templating-resources/blob/master/src/compose.js#L73 is in fact the expected behavior |
Composing a view automatically inherits its binding context from the parent if no view-model is provided, otherwise there would be nothing to bind to. If this is not desired, you can provide a fake view model like this |
@rob, thanks for your clarification, it's what I expected before to ask if it's a bug or not ?
In my child, I still get "Hello" from the parent. |
Yes, the compose element enables flowing through the binding context and override context. This is different from standard components which are fully encapsulated. This means that with compose, if the binding engine doesn't find a match on the composed view model, it can walk the binding context tree. (Traditional components can't because the trees aren't connected.) We wanted to provide this as a possibility for certain scenarios where it made sense. However, it's not advisable to take advantage of this except in rare cases, since it creates unnecessary coupling from child to parent. If you would like an option to explicitly prevent this, we could extend the compose element with a property "inherit-context" which could be used to turn this off. It would be very simple to add. In the link referenced above we would simply not pass the context of the compose element along to the composition engine. If we don't do that, it cannot connect them. Let me know your thoughts on this. Also, I'd love to hear from @alvarezmario and @jdanyow |
@rob, "inherit-context=true" would be good if you want it explicitly. Otherwise, it's like very odd that you put it off and inside the view it-self you know that you are not allow to use parent context. For me, keep it like this but highlight this in the documentation of Compose. However, I would expect to use a predefined property to explicitly get my parent context, rather than just ${monouche}, i would like ${ViewParentContext.manouche}. This avoid property scope confusion. |
I read this, and was very intrigued. We encountered exactly this issue, where by accident a binding 'happened' because of the larger scope with a compose. |
If we were starting over, we would probably not inherit the context by default, but require the property to be set first. However, since we made the opposite default choice, and we don't want to break backwards compatibility at this point, the only solution I see is to add the ability to turn this off. Again, it's not perfect, but it should enable the scenario. In a future breaking change, we could flip-flop the default. We're not quite ready to do that just yet though. Does that make sense? |
I understand your reasoning for not wanting to break backwards compatibility. An option to turn it off is fine as a workaround! |
I'd also appreciate such an option! |
@EisenbergEffect To keep compatibility, having a global setting
The default is cascading to make sure that we keep the compatibility. Since we opened this issue, we got a lot of issue from many developper and changed some guidelines in our process. Think it like having 10 more developpers working in an Application and there is a scoping collision because we use compose feature, it was a nightmare to find out the bug. |
What if we did this:
I think for the most part this will do what developers expect and won't require any changes for typical compose use-cases. @EisenbergEffect, all, thoughts? |
Yes. Let's do it 😀
…On Apr 18, 2017 7:17 AM, "Jeremy Danyow" ***@***.***> wrote:
What if we did this:
1. view -only compose continues to inherit the binding context *(no
change)*.
<compose view="something.html"></compose>`
<compose view.bind="myProperty"></compose>`
2. view-model compose does not inherit the binding context *(change)*.
<compose view-model="something"></compose>`
<compose view-model.bind="myProperty"></compose>`
<compose view-model="something" view="another.html"></compose>`
3. Developers can force the compose to inherit the binding context by
adding an inherit-binding-context attribute to the compose element
*(change)*.
<compose view-model="something" inherit-binding-context></compose>`
4. For a period of time (TBD), aurelia.use.legacyComposeBehavior()
will preserve the compose behavior we have today *(change)*.
I think for the most part this will do what developers expect and won't
require any changes for typical compose use-cases.
@EisenbergEffect <https://github.com/EisenbergEffect>, all, thoughts?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#222 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAIBncZlDKSlwgHAsT-r59Jz4PV7bznwks5rxMXfgaJpZM4IcQh1>
.
|
@jdanyow, is it on the near future roadmap to implement your proposal? I also want to suggest an amendment. I really want to use I am one of the minority who want to use inheritBindingContext for some cases. Hope we are also covered by Aurelia. Thanks. |
I use 1.1.5 version of framework. I found following issue related to this improvement: |
@szogun1987 as a workaround, you can predefine a placeholder for
|
We have 2 views: main.html and child.html.
Inside main.html, we do a compose on child.html:
Note that no model is passed.
Now, in main.js, we specify in the activate method:
Now, in child.html:
The result is that in the child.html, we got "Hello".
Does this is an expected behavior or it's a bug ? This means Compose can inherits from its parent ?
The text was updated successfully, but these errors were encountered: