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

[experimental] avoid initial double render #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GavinJoyce
Copy link
Owner

@GavinJoyce GavinJoyce commented Mar 2, 2017

This is an experimental approach for #18 which avoids an initial double render when using InboundActions.

Instead of InboundActions, there is now a Sender and Receiver mixin. actionReceiver is no longer needed and we no longer set anything in an afterRender (which was the cause of the double render).

import Em from 'ember';
import Sender from 'ember-component-inbound-actions/sender';

export default Em.Controller.extend(Sender, {
  actions: {
    reset: function() {
      //the old way
      this.get('addressForm').send('reset');

      //the new (experimental) way
      this.sendTo('name-form', 'reset');
    }
  }
});
{{name-form}} <!-- new way -->
{{address-form actionReceiver=addressForm}} <!-- old way -->

<button id="reset-from-action" {{action 'reset'}}>Reset</button>
//name-form.js

import Em from 'ember';
import Receiver from 'ember-component-inbound-actions/receiver';

export default Em.Component.extend(Receiver, {
  name: 'Larry David',
  actions: {
    reset() {
      this.set('name', '');
    }
  }
});

If there are multiple components with the same name in a template, they can be given unique actionReceiverNames:

{{random-color-component
  actionReceiverName='left'
  onClick=(action 'changeRightBackground')
}}

{{random-color-component
  actionReceiverName='right'
  onClick=(action 'changeLeftBackground')
}}
import Em from 'ember';
import Sender from 'ember-component-inbound-actions/sender';

export default Em.Component.extend(Sender, {
  actions: {
    changeLeftBackground() {
      this.sendTo('left', 'changeColor');
    },

    changeRightBackground() {
      this.sendTo('right', 'changeColor');
    },
  }
});

siblings

TODO:

  • handle nested components
  • create a send-to helper?
  • support custom names?
  • rename actionReceiverName to sendToName
  • tests
    • acceptance
    • ensure no memory leaks
  • working with ember canary

/fyi @kellyselden I'd be interested to hear your thoughts on this approach

@GavinJoyce GavinJoyce force-pushed the gj/spike-non-double-render-actions branch from 3de1a57 to a111e67 Compare March 2, 2017 20:35
@GavinJoyce GavinJoyce changed the title [spike] avoid initial double render [experimental] avoid initial double render Mar 2, 2017

export function registerReceiver(component) {
let componentName = getComponentRegistryKey(component);
REGISTERED_RECEIVERS[componentName] = component;
Copy link

Choose a reason for hiding this comment

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

Should this fail if there's already a registered receiver for the component name?

I'm imagining a scenario where you have something like:

{{random-color-component onClick=(action foo)}}

...

Does the second component overwrite the first here?
// Many moons later another engineer adds 
{{random-color-component onClick=(action bar)}}

Copy link

@nolaneo nolaneo Mar 3, 2017

Choose a reason for hiding this comment

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

Or is this scenario handled elsewhere?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It currently does overwrite it. 👍 on raising an assertion, we can instruct to use a custom sendToName

Copy link
Owner Author

Choose a reason for hiding this comment

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

Another alternative is to map the name to multiple instances and send the action to all matching ones that are registered.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the flexibility of keeping lists mapped to names:

{{my-component sendToName="foo"}}
{{my-component sendToName="foo"}}
{{my-component sendToName="bar"}}
{{my-component sendToName="bar"}}

Thus "name" becomes a group instead of a key.

@kellyselden
Copy link
Contributor

I like the idea of a {{send-to}} helper. Then you would avoid the Receiver mixin on controllers.

@kellyselden
Copy link
Contributor

I like this approach. Code looks good to me.

Copy link
Contributor

@kellyselden kellyselden left a comment

Choose a reason for hiding this comment

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

Now that #24 landed, you should update the ember version badge in the readme.

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.

3 participants