Skip to content

add target support for pipe#357

Open
mum-never-proud wants to merge 1 commit into
DockYard:masterfrom
mum-never-proud:add-support-pipe-target
Open

add target support for pipe#357
mum-never-proud wants to merge 1 commit into
DockYard:masterfrom
mum-never-proud:add-support-pipe-target

Conversation

@mum-never-proud

Copy link
Copy Markdown
Contributor

Closes #178

Changes proposed in this pull request

both pipe and pipe-action will now support target attribute and resolve in the provided context.

Though I have written the test cases and covered the scenarios my instinct still says I am missing something ;)

Let me know for any changes or enhancements!

thank you!

@snewcomer

snewcomer commented Apr 5, 2020

Copy link
Copy Markdown
Contributor

@mum-never-proud Thanks for the PR! Do you think it is possible to implement binding context and currying args with the new Ember helpers without modifications to the library? My initial feeling is we ca (also action will be deprecated in the future)

https://guides.emberjs.com/release/components/helper-functions/

@snewcomer snewcomer closed this Apr 5, 2020
@snewcomer snewcomer reopened this Apr 5, 2020
@mum-never-proud

Copy link
Copy Markdown
Contributor Author

Hey thanks for the ref, I have gone through the page....Still I don't think we can use it to resolve the result in the given context.... May be I am a bit dumb here...

I couldn't see any helper that would help me resolve in the custom context

A dumb e.g If I wanna pipe the result of A component and set a property in A service I don't think I can achieve the same using any of the helpers in that page....

Also when it comes to mixed contexts like calling a function from service I couldn't see it happening

Pardon me for my bad English but can you let me know how we can solve the above scenario?

@snewcomer

Copy link
Copy Markdown
Contributor

My feeling is users have an acceptable escape hatch today. Here is an example (although a crude example).

In a normal component, we have @action decorators to bind the class it came from and can be used in any context, thus allowing you to refer to the this.calculateService in said bound function. Also we have @tracked as well to help us monitor changes and propagate them to the template...

  test('it resolves the action in the given context', async function(assert) {
    const calculateService = this.owner.lookup('service:calculate')
    class Component {
      add(x, y) { return x + y; }
      square(x) { return x * x; }
      constructor() {
        this.calculateService = calculateService;
        this.squareRoot = function(x) {
          this.calculateService.set('value', Math.sqrt(x));
        }.bind(this);
      }
      // somewhere, somehow we can rely on @tracked
      get value() {
        return this.calculateService.value;
      }
    }

    const instance = new Component();
    this.value = instance.value;
    this.add = instance.add;
    this.square = instance.square;
    this.squareRoot = instance.squareRoot;

    await render(hbs`
      <p>{{this.value}}</p>
      <button
        onclick={{pipe (fn add 2 4) square squareRoot}}>
        Calculate
      </button>
    `);

    assert.equal(find('p').textContent.trim(), '0', 'precond - should render 0');
    await click('button');
    assert.equal(find('p').textContent.trim(), '6', 'should render 6');
  });

I can try to help and add some examples if you would like...

@mum-never-proud

Copy link
Copy Markdown
Contributor Author

@snewcomer thanks for a wonderful example :)

Idk just because I just started contributing to OS, my view might be different or totally wrong in this case

I agree that users have an acceptable escape hatch these days to me i think the reason we develop such wonderful libraries to

  • reduce the burden on them (reduce few keystrokes)
  • provide a fully tested and optimized code
  • exposing a simple API for a relatively complex code

so if we are letting the user do stuff in this case

this.squareRoot = function(x) {
  this.calculateService.set('value', Math.sqrt(x));
}.bind(this);

then I think the user can do other things as well that this library provides.

hope I ain't blunt, I just wanted to understand things that I am relatively new to :)

#178 I find that syntax simpler than currently proposed

pardon me if I didn't understand your solution and thanks for your patience 💪

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.

Should/can pipe respect the action's target attribute?

2 participants