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

Using array-type macros with additional dependent keys? #375

Open
jayjayjpg opened this issue Aug 21, 2017 · 9 comments
Open

Using array-type macros with additional dependent keys? #375

jayjayjpg opened this issue Aug 21, 2017 · 9 comments

Comments

@jayjayjpg
Copy link

I have a case in which I'd like to create a computed property based on an array and another dependent key using the awesome-macro for map. Is it currently possible to let an Array macro not only depend on the target array, but also any other dependent keys, similar to the following?

// target array: fruits, add. dep. key: selectedFruit
 fruitItems: map('fruits', (fruit) => {
   fruit.isSelected = fruit.type === this.get('selectedFruit');
   return fruit;
}).depend('selectedFruit'),

@kellyselden
Copy link
Owner

So It looks like you can abuse the API I made:

fruitItems: map('fruits', (fruit) => {
  fruit.isSelected = fruit.type === this.get('selectedFruit');
  return fruit;
}, 'selectedFruit')

If it works though I would not promise it always works, we should make the API official via tests and README, or come up with a real API like the one you suggested.

@jayjayjpg
Copy link
Author

@kellyselden Thanks for your quick feedback. I just tried it out the map example, but realised that the getter will lose its context inside the callback function as this evaluates to undefined.

And I agree that it would be great to either have this working with the implicit markup

fruitItems: map('fruits', (fruit) => {
 // ...
}, 'selectedFruit');

or a more explicit API as I mentioned earlier. Happy to help with getting the test cases setup for either direction as well.

@kellyselden
Copy link
Owner

It makes sense now. Since map and others just call their Ember counterpart array.map(callback), this is always going to be undefined in the callback. We could alter all the array macros to do something like:

array.map((...args) => callback.apply(this, args))

@jayjayjpg
Copy link
Author

@kellyselden That'd be great - I can also work on a PR for altering the array macros that way

@kellyselden
Copy link
Owner

I would love that! If you do that, can you also add a test for each macro you change like this please:

test('context is correct', function(assert) {
  let callback = sinon.spy();

  let { subject } = compute({
    computed: map('array', callback),
    properties: {
      array: [0]
    }
  });

  assert.strictEqual(callback.thisValues[1], subject);
});

@jayjayjpg
Copy link
Author

@kellyselden I'm a bit stuck on how to improve the macros. E.g. for the map macro, I realized that not only the Ember counterpart of array.map is returned, but instead the result of a normalizeArray function from the addon, which itself returns a lazyComputed macro helper. I'm not too familiar with the macro helpers yet, so I was wondering if you could point me to a point on where to get started with implementing these changes. Also happy to provide a failing test case first if this helps.

@kellyselden
Copy link
Owner

#379 should give you an idea of how to proceed. Let me know if you have any problems.

@lolmaus
Copy link
Contributor

lolmaus commented May 15, 2018

Almost a year later, is there a solution?

I can imagine something like this:

{
  foo: "Foo",
  bars: ["bar", "Bar", "BAR"],

  result: map("bars", "foo", (bar, i, bars, foo) => bar + foo) // => ["barFoo", "BarFoo", "BARFoo"]
}

@lolmaus
Copy link
Contributor

lolmaus commented May 15, 2018

The callback passed to map receives three normal iteration arguments: item, index and array, and then as much arguments as the number of extra dependencies.

The current workaround is the following, which is way more verbose than it should be.

  computed("bars.[]", "foo", (bars, foo) => {
    return bars.map(bar => bar + foo);
  }),

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

No branches or pull requests

3 participants