sweetalert2.github.io icon indicating copy to clipboard operation
sweetalert2.github.io copied to clipboard

Document the merging strategy of mixin params

Open limonte opened this issue 3 years ago • 0 comments

Hi @zenflow , thanks for your reply!

There is one reason why I consider deep merging better. Before version 9.0.0 there were separate options for confirmButtonClass, cancelButtonClass etc. If you take the code I gave in the description and adapt it to an older version of swal, you will get this:

const test = Swal.mixin({
  // ...
  confirmButtonClass: 'green',
  cancelButtonClass: 'red',
});

test.fire({ 
  // ...
  confirmButtonClass: 'orange',
});

Here the test.fire call will override confirm button class to 'orange', but leave cancel button class intact. In my opinion, this discrepancy in behavior between <9.0.0 and 9.0.0 is an unnecessary backward incompatible change, only caused by the fact that 9.0.0 uses shallow merging strategy for aggregate options it introduces (customClasses, hideClass and maybe some others). Someone who needs to upgrade swal <9.0.0 to ≥9.0.0 is likely to change confirmButtonClass, cancelButtonClass etc. into customClasses: { ... } mechanically and only then discover that merging doesn't work as before.

It is possible to use a custom function instead of swal.mixin as you suggest, but deep merging, among the three options you listed, seems the most intuitive to me because it matches the behavior of swal <9.0.0. So I would prefer to have it out of the box. Although, it may already be too late to make such changes in swal.mixin...

Anyway, I am of course not insisting. I just wanted to hear your thoughts and motivation.

Before I forget, I would also like to suggest documenting the current shallow merging strategy on https://sweetalert2.github.io/. Docs for swal.mixin are barely existent:

image

Originally posted by @earshinov in https://github.com/sweetalert2/sweetalert2/issues/2039#issuecomment-683940267

limonte avatar Sep 01 '20 11:09 limonte