Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document the merging strategy of mixin params #118

Open
limonte opened this issue Sep 1, 2020 · 0 comments
Open

Document the merging strategy of mixin params #118

limonte opened this issue Sep 1, 2020 · 0 comments

Comments

@limonte
Copy link
Member

limonte commented Sep 1, 2020

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 sweetalert2/sweetalert2#2039 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant