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

Fix/repeat one time #357

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix/repeat one time #357

wants to merge 4 commits into from

Conversation

rluba
Copy link

@rluba rluba commented Jul 13, 2018

This is a crude attempt at fixing #356.

The fix for the if case was relatively straight-forward. But to disable the fast-pass for the "subview without lifecycle methods" case, I had to rely on instruction.initiatedByBehavior, because I couldn't find a better indicator.

I just started working with Aurelia a few weeks ago, so I’m sure there’s a more elegant way that I couldn’t discover. Let me know what you think!

@bigopon
Copy link
Member

bigopon commented Aug 21, 2018

@rluba @EisenbergEffect

I think all the changes are good except using initiatedByBehavior. By default checking that will also make it false positive for any custom element / custom attribute that doesn't actually require lifecycle. I think we have to introduce new property on If metadata via:

const meta = metada.get(metdata.resource, If);
meta.behaviorRequiresLifecycle = true;

and check:

function behaviorRequiresLifecycle(instruction) {
  let t = instruction.type;
  let name = t.elementName !== null ? t.elementName : t.attributeName;
  return lifecycleOptionalBehaviors.indexOf(name) === -1 && (t.handlesAttached || t.handlesBind || t.handlesCreated || t.handlesDetached || t.handlesUnbind)
    || t.viewFactory && viewsRequireLifecycle(t.viewFactory)
    || instruction.viewFactory && viewsRequireLifecycle(instruction.viewFactory)
    || instruction.type.behaviorRequiresLifecycle;
}

Note: naming is for discussion.

@@ -9,7 +9,8 @@ function behaviorRequiresLifecycle(instruction) {
let name = t.elementName !== null ? t.elementName : t.attributeName;
return lifecycleOptionalBehaviors.indexOf(name) === -1 && (t.handlesAttached || t.handlesBind || t.handlesCreated || t.handlesDetached || t.handlesUnbind)
|| t.viewFactory && viewsRequireLifecycle(t.viewFactory)
|| instruction.viewFactory && viewsRequireLifecycle(instruction.viewFactory);
|| instruction.viewFactory && viewsRequireLifecycle(instruction.viewFactory)
|| instruction.initiatedByBehavior;
Copy link
Member

@bigopon bigopon Aug 21, 2018

Choose a reason for hiding this comment

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

We can not do this, as it includes all custom elements, and custom attributes, thus could possibly make it a breaking change.

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