0

My Blazor component has some associated JavaScript, which performs (async) animations.

MyComponent.razor

protected override async Task OnAfterRenderAsync(bool firstRender)
{
  if (someCondition & jsModule != null)
    await jsModule.InvokeVoidAsync("startAnimation", "#my-component");
}


public async ValueTask DisposeAsync()
{
  if (jsModule != null)
    await jsModule.InvokeVoidAsync("stopAnimationPrematurely", "#my-component");
}

MyComponent.razor.js

export function startAnimation(id) {
  let element = document.getElementById(id);
  element.addEventListener(
    'animationend',
    function() { element.classList.remove("cool-animation") },
    { once: true }
  );
  element.classList.add("cool-animation");
}


export function stopAnimationPrematurely(id) {
  let element = document.getElementById(id);      // fails here
  element.removeEventListener(
    'animationend',
    function() { element.classList.remove("cool-animation") },
    { once: true }
  );
  element.classList.remove("cool-animation");
}

As you can see, the animation cleans up after itself (via { once: true }).

However when the user clicks to a different page or component - and thus the blazor component is destroyed - there could be an animation in progress. If I don't remove the js event listener then I'll get a memory leak. So in DisposeAsync() I invoke js cleanup code which explicitly calls removeEventListener().

The problem is that by the time the js code runs, the component is already destroyed - so the DOM element is missing, the id is invalid, and thus the js cleanup fails (and throws).

This is very surprising. There is a race condition between the various lifecycle methods and disposal. In MudBlazor they also encountered this and introduced some really hard to understand (undocumented) locking as a workaround.

How can I deal with this without workarounds or hacks? (If that's not possible, please show a working solution, even using locking or whatever... a hacky solution is better than nothing.)

Improve this question
2
  • If element is removed from the dom - won't all event listeners and animations be cleaned up automatically anyway? Commented Dec 27, 2022 at 11:48
  • @Evk Thanks, that was my initial thought too. But apparently it's not a sure thing, and also depends on the browser. As you can imagine, I'd prefer to do it properly to avoid memory leaks. Commented Dec 27, 2022 at 11:53

3 Answers 3

Reset to default
2

I had a similar problem here. However some artificial Task.Delays were necessary to achieve the race condition as far as I remember.

The accepted is interesting, but I think it's more like a workaround and doesn't address the core issue.

Thanks for raising this topic and asking on git. Since this is really a difficult topic as it seems. I found javiercn's replies help in shining some light on the subject.

Several things to consider:

1.) You are using "pure" JS to reference elements in Blazor document.getElementById(id);. IMHO that is not recommended or at least not the blazor-ry way, since Blazor doesn't know about those elements. IMHO I'd split all my JS calls in either "really pure JS", which means scripts at the bottom of <body> that Blazor don't know nothing about and that are not invoked by JS interop, for example with plain onclick="console.log('hi')" qithout the @...

-or-

using ElementReferences with @ref. See here and here. The caveat here is, that they can only reliably be used inn OnAfterRenderAsync.

(have not tested them in DisposeAsync, does that even make sense to use them here?)

2.) Correct me here, if I'm wrong, but when you actually add an event listener to an element and that element gets removed from the DOM, then you are already fine, is what I have assumed. The listener is a property of the element and removed with it, is what I have thought. Is there really a memory leak? (Is that the "do nothing" approach javiercn mentioned on github?) - TBH in this case, I did not consider having to run cleanup, but you made me think about that!

Maybe, However there would be a problem if you have the event listener at a global object, like window. On the other hand, you can always remove it from there inside the DisposeAsync, because that elements surely still exists then.

3.) Async LifeCycle methods order can be very confusing, but generally, I'd say, OnAfterRenderAsync is not invoked, when a component is regularly disposing/removed while navigating inside Blazor. However it's another story if you leave the side hard, like closing the tab, going to blank page ect. See my answer here.

In summary

  • probably doing nothing, might be OK. Using ES6 modules and variables inside there, that have a 1:1 correlation to your elements, would at least not result in a growing memory leak, even if there were still some content (which the GC porbably cleans, like javiercn said) - (which also would be overwritten when the component would be re-creaeted)

  • the mutation observer seems like the most solid pure JS approach for max. reliability

  • try using a bool flag (or lock) in DisposeAsync to prevent a 'still going on' OnAfterRenderAsync (which is probably a very rare edge case, which I was concerned with here)

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

You're right, it's a complex problem! I did solve it though, using a MutationObserver as javier recommended. I wouldn't use any of the other approaches, because he's right about one thing - if the async work is started in js, it should be cleaned up in js too. I wish there were a reliable way to hook into this part of the lifecycle via managed cs, but if they won't give us one, it's best to use a MutationObserver.
Re 2, many say not to worry about explicitly calling removeEventListener, but I disagree - I've not found proof or docs of this anywhere, so can't assume the GC will save me. For a very large app (which adds and removes loads of stuff, dynamically, and routinely) I can't risk having a massive memory leak, so I opted to release everything to be sure. Re locking, I wouldn't go that route, I think it's asking for trouble; if you do though, make sure to test it thoroughly. I think a MutationObserver is the best option.
I agree that this principle of removing through JS sounds right to me as well! But I also think he's right, when saying, that there is not really a sensible way to achieve this with JS-interop / C#, it must be done on the client side (browser), bc only it know the right "moment". (and like he said, the setup should be triggered at the time, when the JS interop initalizes the JS, which needs cleanup, in the first place.) Hence MutationObserver. I also think I might be a bit more cautious/concerned with such kind of memory leaks than some other ppl I talked to irl (who prefere 'do nothing').
1

Basically your problem is that the Renderer has rebuilt it's DOM and passed it to the Browser before it gets round to disposing the component (that is now no longer attached to the Render Tree) and running the code. The DOM element is history as you've found out.

So you need to make sure your Animation stopping code runs before the Render gets a chance to update the DOM.

Pre Net7.0 this would not have been possible. Net7.0 introduced a new feature to the NavigationManager. It now has an async method called OnLocationChanging which it calls before raising the LocationChanged event. It's primarily designed to prevent navigation away from a dirty Edit Form. You take the provided LocationChangingContext and call PreventNavigation to cancel the navigation event (No LocationChanged event gets raised).

You don't want to cancel the navigation event, just delay it till you've done the necessary housework. It's Task based, so NavigationManager waits till it completes before raising the LocationChanged event (the Router uses this event to trigger routing,....). I think we can use it to achieve your goal.

Here's some code that demonstrates the pattern I believe you can use.

@implements IDisposable
@page "/"

<PageTitle>Index</PageTitle>

<h1>Hello, world!</h1>

Welcome to your new app.

<SurveyPrompt Title="How is Blazor working for you?" />

@code {
    [Inject] public NavigationManager NavManager { get; set; } = default!;

    private IDisposable? NavDisposer;

    protected override void OnInitialized()
        => NavDisposer = NavManager.RegisterLocationChangingHandler(OnLocationChanging);

    private async ValueTask OnLocationChanging(LocationChangingContext context)
        => await this.SortTheAnimation();

    public async ValueTask SortTheAnimation()
    {
        // mock calling the js to do the cleanup and wait for the task to complete
        // Make the Navigation Manager wait
        await Task.Delay(3000);
    }

    public void Dispose()
        => this.NavDisposer?.Dispose();
}

Test it and let me know.

Improve this answer

5 Comments

Thanks MrC! I'm still on v6, and hoped to avoid v7 for now - but you've forced me to upgrade! "Thanks!" :-) It'll take time to get up and running, so I'll just mark this answer, it feels correct. I'll update if anything is amiss. BTW, I asked on the repo and they recommended to use a MutationObserver, which is another good option, but heavier than this one. On the other hand, that solution would always works, whereas this one is only for page changes - it won't work unless the URL is changed. So, some tradeoffs. Thanks and Happy New Year!
Interesting update: In that repo issue the reason for this was explained: "we do not want to delay updating the UI just because a component decides to take 10 seconds to dispose". Also the MutationObserver was recommended instead of the OnLocalChanging approach, but I think there's room for both, depending on circumstances.
I jumped to 7.0 at the beginning of this month. There were a few things I wanted to use but the above was the main reason having been a (small) contributor to that feature and amongst a few who were a thorn in the Blazor team's side to get it implemented.
Just read your comment and the Repo issue. I agree on the comment about making the UI wait, so maybe? the MutationObserver is the better option. I'm not wedded to my answer, it was a bit of lateral thinking on something I understood well. I'd do some testing and see what you get. Good New year coding.
Funnily enough I read the release notes last month and your feature was the only one that appealed to me so as to upgrade sooner rather than later (I typically wait a few months or even a whole version before upgrading). Also, I added a feature request asking for a managed way to deal with js disposal; doubt they'll consider it, but it seems like a pain point for some people, especially library authors (MudBlazor components jump through hoops to deal with this).
1

Another approach: I also asked on the repo, and was told to use a MutationObserver.

Tradeoffs:

  • The accepted answer is a lighter option. But it's only appropriate when there are page URL changes - it won't work when one is just removing the component (no URL change in that case).
  • The MutationObserver feels heavier, but it would always work - it's not dependent on URL changes.

Also, I added a request on the repo to provide us with a managed way to handle disposal in blazor-to-js interop. If you need that functionality, please upvote it.

UPDATE

There is a critique of the various approaches in that repo issue. It certainly looks like a MutationObserver is the best approach, albeit a little more work.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.