Add update() method

[limonte]

I believe this is much better than #1152

Renderers were introduced recently, there are currently 2 renderers: https://github.com/sweetalert2/sweetalert2/tree/master/src/utils/dom/renderers

Renderers allow to re-render particular parts of a popup without re-initialization:

I'm quite happy with this implementation. What do you think @zenflow?

[coveralls]

Pull Request Test Coverage Report for Build 4519

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 20 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.01%) to 90.489%

---

Files with Coverage Reduction	New Missed Lines	%
dist/sweetalert2.js	20	90.49%

Totals
Change from base Build 4517:	-0.01%
Covered Lines:	1136
Relevant Lines:	1206

---

💛 - Coveralls

[limonte]

@zenflow after the green light from, I'll cover the new functionality with tests to keep code coverage on a 90+ level 🥇

[zenflow]

@limonte sorry for the delay in getting to this. I've given this review a lot of thought, and I'm generally pretty happy with the change. I'm also really happy about the renderers. I agree it's a great step forward in the organization of the codebase. The next logical step might be to colocate the bits of pre-rendering (construction or setup) logic with their respective renderers.

I linked this against sweetalert2-react-content to test out the API from the point-of-view of an extension. I had to do some reorganization (and fix a prexisting bug sweetalert2/sweetalert2-react-content#58) but it was easy to get the enhancer to work with this feature. You can check out https://github.com/sweetalert2/sweetalert2-react-content/tree/feature/integrate-update-method .. note that the instance properties prefixed __ need to be fixed to be truly private.

Some issues:

The first two are related: Firstly, it would nice if each extension did not need to manage it's own params object; i.e. storing it and updating it. Currently, if you (as an extension) want to know the complete set of effective params from inside the update method (not just the subset that are being updated) you need to track that yourself. It's not that much extra code or complexity, but it might not be necessary, i.e. we might be able to handle it in the core. Secondly, the instance's params property (i.e. the "outer" params, which were defined with the user's call to Swal) should probably be updated when the update method is called, as this would likely be expected. As far as I can figure, the only way to do this properly, while still giving subclasses the ability to control the params given to the superclass, would be to have a "protected" method that you can extend that does the actual work, and the update method just updates the instances public params property and calls the protected method. This update method could also solve the first issue by passing the complete set of [outer] params to the protected method.

The other issue is one that is already existing, that I contributed to, that this PR is also contributing to: the exposing of isValidParameter, isDeprecatedParameter & isUpdatableParameter as public methods.. what use case do they serve? and are they expected to be implemented in relevant extensions (that add or remove params)? That's a little bit of overhead, and is likely to be forgotten (even if documented), so I would not necessarily trust them. In most cases, I think it is enough for the public API to log warnings (or throw errors) when you try to do something invalid, so what do you think about deprecating these methods in the public API and removing them in the next major? For now I'm cool with either adding this new isUpdatableParameter method or not, either way.

[limonte]

oops, wrong button

[limonte]

I'll just ship this feature as it is

[zenflow]

@limonte Disappointed that we could not work on this so that it remains easy to extend Swal..

Also, in my comment I indicated a problem/bug:

the instance's params property (i.e. the "outer" params, which were defined with the user's call to Swal) should probably be updated when the update method is called, as this would likely be expected

[gverni]

Also, in my comment I indicated a problem/bug:

Happy to work on that @limonte @zenflow if you think it's worth it. My only question (to better understand how it can be used) is what would be the use case for that @zenflow? Let's say that a developer change the swal type: in which condition does he/she wants to check again the value of that property? Is it to implement a kind of state machine on a running swal? And does that affect the mixin as well (in case there is one)?

[limonte]

@zenflow I personally do not see any of those as critical issues, if they are we will fix them and ship as patch releases because they will not affect public APIs.

Please elaborate a bit more on:

• which are potential use-cases which won't work as expected
• what is actual behavior
• what is expected behavior

Proper issue-reports (with some code, actual and expected behaviors) and/or PRs are very welcome.

[zenflow]

Opened #1386 ...

The other recommendations were critical to Swal being easy to extend.. something I worked very hard on.

Those recommendations included changes to the "protected" API (i.e. extensions depend on it; changes to it can break code outside of this repo) as well as the public API (e.g. removing isValidParameter, isDeprecatedParameter & isUpdatableParameter from the public API).

I'm not sure why you are so hasty to make the major release, when there are still many things in the API that could have been fixed or improved, e.g. #1373, but now we are stuck with for another major version, unless you are fine with making major breaking changes in patch releases.

[limonte]

I'm not sure why you are so hasty

The process of releasing a new major took 11 days: aa657e5...v8.0.0 I wouldn't call it hasty. Everybody noticed PRs with "BREAKING CHANGE" in their titles and was able to participate.

We all have limited time, I personally don't need anything from #1373 or #1386 for my projects, that is why I didn't work on them. At the same time, new major contains some really important changes (e.g. ES modules support) which people need and my responsibility as a mantainer fo this project is to ship that as soon as possible.

#1373 has appeared after I started the new major process and it lacks use-cases with code examples, actual and expected behaviors, that's why I didn't work on it. I will ask for clarifications there, but it's a bit impolite to drop issues without following issue templates and expect them to be done.