Content removal ahead of time

[dakiesse]

sweetalert2-react-content/src/index.js

Line 45 in ec05a41

params.onClose = (element) => {

why not .onAfterClose? Unmounting happens before closing now and you can see jerking when the closing.

[zenflow]

The most likely reason for this is simply that onAfterClose did not exist at the time this package was originally drafted

• initial commit in this repo in February 2018 f9af0c8
• onAfterClose added in sweetalert2 in March 2018 feature: add onAfterClose config option sweetalert2#1030

[dakiesse]

@zenflow will it be changed?

[zenflow]

I think we should change it, but not to onAfterClose.. After a little experimentation, I found that neither onClose nor onAfterClose are ideal, since neither are called when the current modal is interrupted by another modal.

Example: A modal is opened, and before it is closed, another modal opens. The onClose & onAfterClose methods will never be called for the first modal.

This scenario is not typical, but is possible, and probably happens in some apps.

In this scenario, the result is memory leaks and the possibly bugs (if the mounted react component has some activity based on timers, network, etc.).

I think we should implement some new callback option(s) in the sweetalert2 package, where it's possible to implement "cleanup" code which we can rely on being called. For example, onDestroy (which would be called when the modal's life is over, regardless of if it was interrupted, or actually closed), or onUnRender (which would be called to clean up from every rendering). If we implemented onUnRender then here we could use onRender and onUnRender and easily take care of #73 at the same time.

/cc @limonte

[limonte]

I think we should implement some new callback option(s) in the sweetalert2 package, where it's possible to implement "cleanup" code which we can rely on being called. For example, onDestroy (which would be called when the modal's life is over, regardless of if it was interrupted, or actually closed), or onUnRender (which would be called to clean up from every rendering). If we implemented onUnRender then here we could use onRender and onUnRender and easily take care of #73 at the same time.

/cc @limonte

Sounds like a great idea, but I don't know how to implement it 🤷‍♂️ Let's take this simple code:

Swal.fire('one')
Swal.fire('two')

Here Swal.fire('two') will dispose the first modal. In other words, the first modal doesn't dispose itself, but another instance cleaning up DOM tree before rendering itself. In resetOldContainer() function, the second modal knows nothing about the params of the first one, therefore the onDestroy param of the first modal can't be accessed.

[zenflow]

@limonte I think we would need to:

1. Store the current Swal instance in the global state (e.g. globalState.currentInstance)
2. Add a swal._destroy() instance method that can take care of cleanup, including calling the user-provided onDestroy callback
3. Near the start of the swal._main method:

if (globalState.currentInstance) {
  globalState.currentInstance._destroy()
}
globalState.currentInstance = this

@gverni Would love to get your thoughts on all of this

[limonte]

Thank you for the suggestion @zenflow The related PR is ready for your review: sweetalert2/sweetalert2#1872

[limonte]

🎉 This issue has been resolved in version 3.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀