Clean lastGlobalRef on destruction of {{create-ref}} modifier

[tniezurawski]

It fixes a memory leak where lastGlobalRef keep an application around in tests:

Closes: https://github.com/lifeart/ember-ref-bucket/issues/50

🙋‍♂️ For reviewers' consideration

As mentioned by @lifeart

We could try do few things:

• register destructor inside modifier, but binded to app instance (getOwner(this)).
• implement app-initializer flow to register / remove globalRef

We indeed can do two things. I feel like cleaning up after modifier is destroyed is a better idea. app-initializer can do it as well but that would be a solution to satisfy the tests.

I think that currently, in the normal usage of an application lastGlobalRef is kept longer than needed. I think the referenced object is only released when a new {{create-ref}} overrides it. So we often have a bit more in memory than necessary. That's why I took the route of the modifier cleaning up after itself.

Help needed

I'm not sure how to write a test for that. I was thinking about checking if lastGlobalRef is cleaned up after changing isToggled:

test('it cleans up `lastGlobalRef` after itself ', async function (assert) {
  this.set('isToggled', true);
  await render(
    hbs`{{#if this.isToggled}}<div {{create-ref "foo"}}>octane</div>`
  );

  // Somehow assert that `lastGlobalRef` is set

  this.set('isToggled', false);

  // Somehow assert that `lastGlobalRef` is no longer set
});

but how can I access lastGlobalRef in tests?

[lifeart]

Hi @tniezurawski! looks like to test this logic you need to use {{create-global-ref "foo"}}. To avoid unexpected nulling we could check that this state will work:

this.isHidden = false;

// render code

<div {{create-global-ref "foo"}}>stable</div>

{{#if this.isHidden}}
  <div {{create-global-ref "bar"}}>unstable</div>
{{/if}}

// check refs exists for foo/bar

this.isHidden = true;

// check refs exists for foo/bar at the moment, I assume error, because cleanup of "bar" modifier will null globalRef, and foo unable to resolve it

[tniezurawski]

One thing that is slightly worrying that I don't understand is that when I run tests in the addon with the dummy app, the dummy app is still hanging around but because of some different reason that I don't understand:

BTW I used my fork in a big application that uses this addon and I see the change fixes the issue ✅ Here's a screenshot from the reproduction repo where it is also fixed:

[lifeart]

but how can I access lastGlobalRef in tests?

You need to write logic to work with create-global-ref (there is no need for direct access)

[lifeart]

nodeFor from https://github.com/lifeart/ember-ref-bucket/blob/master/addon/index.js#L6C17-L6C24 may help to verify https://github.com/lifeart/ember-ref-bucket/pull/51#issuecomment-1643550481

nodeFor(resolveGlobalRef(), 'bar') should return node, no matter isHidden is toggled or not

[lifeart]

Hi @tniezurawski! Thank you for contribution! 5.0.4 published