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.

Sign up for GitHub

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

Jump to bottom

set-then and execute #363

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

set-then and execute #363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9f55e95
PoC mut-then
betocantu93 Apr 24, 2020
940378e
PoC
betocantu93 Apr 24, 2020
65bb7f8
Merge branch 'master' of https://github.com/DockYard/ember-composable…
betocantu93 Nov 24, 2020
3e5b22f
opt out of the args
betocantu93 Nov 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions addon/helpers/execute.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { helper } from '@ember/component/helper';
import isPromise from '../utils/is-promise';

export function execute(actions = []) {
return function() {
let invoke = function(acc, curr) {
if (isPromise(acc)) {
return acc.then(() => curr());
}

return curr();
};

return actions.reduce((acc, curr, idx) => {
if (idx === 0) {
return curr();
}

return invoke(acc, curr);
}, undefined);
};
}

export default helper(execute);


Copy link
Contributor

Choose a reason for hiding this comment

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

So this is a copy of queue without args? What is the pain of ignoring the args in each action?

Copy link
Author

@betocantu93 betocantu93 Nov 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yes that's correct, a copy but without args, mainly to give the template ergonomics, Currently there's no way to simple chain events giving the template full control of partial application without leaks, queue and pipe 'leaks' into the chain of fns, for example using on with queue helper leaks the Dom event to the chain, using pipe leaks the return value, which might actually cause troubles if any fn in the chain returns void and any of the next function args expects maybe params to be defined or something, you can see execute as the call helper but with a promise aware chain of fns

To chain perhaps unrelated stuff like closing a dialog, creating a contact, displaying a notification and also triggering @onClick and logging to Google Analytics via a service action, all of these fns may receive args in certain way or order so leaking may have undesirable effects

function something(str, isValid) {
  console.log(str)
  if(isValid){
     this.model.save()
  }
}
<button {{on "click" (queue this.openModal (fn this.something "hey there")}} >

Click me 

<button />

The event will be leaked, and so isValid would be true... I can try to give more examples if that doesn't make sense yet?

In our use case we have a serious amount of fns down on the render tree which need to hook say to the onBlur of an input and it wouldn't be posible if we had to worry about how they are actually physically placed, we see them as kind of a series of reactions to mainly user UI interaction event which any of them is entirely responsible of partially apply the needed params

Copy link
Author

@betocantu93 betocantu93 Nov 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe call helper may receive an array of fns? Would that make more sense? 🤔🤔

//application.hbs
<BuyForm @logSomething={{fn this.log "buy_form"}} /> 


//buy-form.hbs
<SomeComponent @onClick={{execute (set this "closeDialog" true) @logSomething}} />

//some-component.hbs
<SomeDropdown as |menu|>
 <buttton {{on "click" (execute this.buyStuff menu.close (fn this.ga.sendEvent "clicked_buy_this") (fn 
  this.notificationManager.notify "thanks") (fn @onClick "clicked_buy_this")) }} >
    Buy this
 </button>
</SomeDropdown >

Opting out from leaks makes sense when dealing with deeply nested trees of components

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok that makes sense. Thx for the explanation! I think we will need to come up with a really clear name b/c when one steps back and evaluates how to call a chain of functions, I'm not sure an obvious answer stands out (and may be more confusing with two options)

Copy link
Author

Choose a reason for hiding this comment

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

Yup, totally agree, what do you think about sequence?

  • sequence

🤔🤔🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, it sounds like the functions you are using might be called in multiple places? Otherwise, you could always align the arguments with the arity of arguments if only used in one spot?

I'm also thinking about this from a typescript point of view. You have a list of expected arguments. However, this will pass nothing and could show an error. Typically, in this case you would compose multiple functions together while perhaps sharing some logic.

What are your thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

Also, it sounds like the functions you are using might be called in multiple places? Otherwise, you could always align the arguments with the arity of arguments if only used in one spot?

Yes, pretty much. I use them sparingly across the app/engines in different ways, from JS and helpers, so having full control on the template is a must, to avoid foot guns.

I'm also thinking about this from a typescript point of view. You have a list of expected arguments. However, this will pass nothing and could show an error. Typically, in this case you would compose multiple functions together while perhaps sharing some logic.

Honestly I haven't (probably should) used a lot of functional programming or composition of fns, in .js land in Ember. I often use the templates for that, but I agree, you compose multiple functions together taking the arity, order and such into account when needed to share logic or something in common, in this particular case, the only thing these fns have in common is that they were triggered by something... sometimes an event, sometimes a callback, basically like observer pattern, pretty much, these are subscribers fns. in a Template only Component land, pipe, queue, (sequence|execute) come pretty handy

Copy link
Author

Choose a reason for hiding this comment

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

Another good approach could be to just add a noargs helper...

<button {{on "click" (queue fn1 fn2 (noargs (fn fn3 myWantedArg)))}}>
function noargs(fn) {
  return () => fn()
}

24 changes: 24 additions & 0 deletions addon/helpers/mut-then.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { helper } from '@ember/component/helper';
import isPromise from '../utils/is-promise';

export function mutThen(actions = []) {
return function(...args) {
let invoke = function(acc, curr) {
if (isPromise(acc)) {
return acc.then(() => curr());
}

return curr();
};

return actions.reduce((acc, curr, idx) => {
if (idx === 0) {
return curr(...args);
}

return invoke(acc, curr);
}, undefined);
};
}

export default helper(mutThen);
1 change: 1 addition & 0 deletions app/helpers/execute.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default, execute } from 'ember-composable-helpers/helpers/execute';
1 change: 1 addition & 0 deletions app/helpers/mut-then.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default, mutThen } from 'ember-composable-helpers/helpers/mut-then';
17 changes: 17 additions & 0 deletions tests/integration/helpers/execute-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render } from '@ember/test-helpers';
import hbs from 'htmlbars-inline-precompile';

module('Integration | Helper | execute', function(hooks) {
setupRenderingTest(hooks);

// Replace this with your real tests.
test('it renders', async function(assert) {
this.set('inputValue', '1234');

await render(hbs`{{execute inputValue}}`);

assert.equal(this.element.textContent.trim(), '1234');
});
});
17 changes: 17 additions & 0 deletions tests/integration/helpers/mut-then-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render } from '@ember/test-helpers';
import hbs from 'htmlbars-inline-precompile';

module('Integration | Helper | mut-then', function(hooks) {
setupRenderingTest(hooks);

// Replace this with your real tests.
test('it renders', async function(assert) {
this.set('inputValue', '1234');

await render(hbs`{{mut-then inputValue}}`);

assert.equal(this.element.textContent.trim(), '1234');
});
});