Context is lost

[dy]

If we call a function with defined context, the handler loses it.

import { augmentor } from 'augmentor'
  let fn = augmentor(function () { console.log(this) })
  fn.call({ x: 1 })

// null

Is that expected?

[WebReflection]

I augment a bound function, when needed, would that work? Cause otherwise you have N context interfering with a single stack, which sounds a bit like a footgun, don't you think?

[dy]

Yes, makes sense. I though possibly ignoring it could be fine, in case if user'd like to keep initial function context:

let f = augmentor( (function() { console.log(this) }).bind(someContext) )
f.call(otherContext) // ignored

Not sure what option is better.

[WebReflection]

The point is, every augmented fn has one stack assigned. If people think they can bind a function once and use it as prototype method, as example, they would cause troubles to themselves, 'cause each invoke would interfere with other contexts too, so I didn't bother to pass the context around, as it should be clear that every instance should have an augmented version of any function they like per se, as bound function, so that no footgun happens.

Accordingly, I think this might be closed as won't fix, and kept in this repo history as reference/answer for whoever thinks a single augmented function with multiple context makes sense.

They could wrap the bound function using a self and update/change such self before each invocation, with the context they want, but that's an opt-in that requires you to understand how the augmented stack works, so it's less footgun prone, as it'd be explicit, not allowed by the library out of the box.

Does this reason well?

// for classes, as used in heresy
this.method = augmentor(this.method.bind(this));

[dy]

Does this reason well?

Logically, yes. On the downside, though, it can be a bit unexpected for fn.call, fn.bind and fn.apply to be useless on augmented functions.

[WebReflection]

Then I'll put an explicit entry in the README to avoid surprises.

[dy]

Let me show potential practical case. Imagine if morphdom-like morpher is used for components. Or jQuery would support reactive aspects.

$('.items-list').use(function handleAction () {
  // `this` is current element reference here, like in jQuery `$(elements).each()`
  console.log(this)

  let [loading, setLoading] = useState(false)
  let [items, setItems] = useState([])
  useEffect(() => {
    setLoading(true)
    load('/items').then(items => {
      setLoading(false)
      setItems(items)
    })
  }, [])

  return loading ? <>loading...</> : <ul>...items</ul>
})

Here, handleAction can be augmented function, but it is supposed to switch context, depending on the returned html. With fn.call(element) that is trivial. Creating an augmented function for every new element is a bit more complicated.

[WebReflection]

I've added the F.A.Q. regarding this here https://github.com/WebReflection/augmentor#faq, and I'll keep this as won't fix, as I've explained there why this is not a good idea, but there's also an example to use a shared context, and watch out the side effects bit.

[WebReflection]

P.S. in that very example, you could just pass along a reference instead of using this, but all the quirks around useEffect side effects are still valid, so you are navigating dangerous waters there, imho.

$('.items-list').use((item) => { ...

[dy]

Implemented exactly that technique in enhook/augmentor. Thanks!

[WebReflection]

that's neat, but there are at least 2 things off:

1. fn.call(ctx, ...args) is the equivalent of fn.apply(ctx, args), which is also more performant with, or without, transpilers around
2. if you require agumentor that's not enough, as when you pass a callback, the reference to useState, or any other hook, should be global, or accessible in the composed scope, if machinery is around, so while you solved the context gotcha, you need to simplify useState and friends retrieval

happy to know more how that goes though, but also thanks for sharing, it's a god trick, after all 👍

[WebReflection]

So ... you know what? Instead of letting people ask me, open issues, or write weird solutions to this (IMHO) edge case, augmentor is now providing a contextual(callback) utility that more or less does what you did, except it also cleans up last retained context after itself, so no memory leaks should ever happen.

The updated example in the README shows this:

import {contextual} from 'augmentor';

const textInjector = contextual(function (text) {
  this.textContent = text;
});

textInjector.call(div, 'hello');
textInjector.call(p, 'there!');

so that your enhook(fn) can be just a reference to augmentor.contextual utility.

I hope this makes you happier, as now you have a core method to achieve what you're after 👋

[dy]

That's awesome. Let me hook that up.

As for 2.

you need to simplify useState and friends retrieval

Now hooks are retrieved by any-hooks/augmentor as https://github.com/unihooks/enhook/blob/master/augmentor.js. Is there something that seems doubtful? The point is to have cross-framework hooks.

On another point - in augmentor < 1.x there was destroy method. Is there any way to dispose augmented function now? Seems that tests need to tear down previously hooked functions.

[dy]

There's an issue with contextual. If that's being called via setState, it loses context.

let fn = contextual(function () {
  console.log(this)
  let [, bump] = useState()
  useEffect(bump, [])
})
fn.call({ foo: 'bar' })
// { foo: 'bar' }
// null

I suppose that's desired behavior, but enhook keeps last provided context for all subsequent effect calls.

[WebReflection]

I suppose that's desired behavior,

Yes, that's the most evil thing I can think of, as there's no way to stop retaining the last context, which could be null, but also the heaviest runtime object ever created.

But that being said, I don't think your solution works neither, as as soon as you have more than a context, you don't which this is referring to, but also this is not retained ... right?

[WebReflection]

Actually ... I see what you mean there ... last context should indeed be available for the next useState, so I can't clean it up.

Well, that's going to be fixed like ... now.

[dy]

Awesome https://github.com/unihooks/enhook/blob/master/src/provider/augmentor.js. Thanks @WebReflection!