-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Why swal.fire? #1438
Comments
We migrated the code-base to be object-oriented (i.e. we changed the The problem with this is that you do nothing with the result of the call (the instance) you will be violating the eslint "no-new" rule, which is a common rule to be enabled, and part of JavaScript Standard Style. function sayHi () {
new Swal('Hi') // violates the eslint "no-new" rule
} The function sayHi () {
Swal.fire('Hi') // does not violate the eslint "no-new" rule
} related: When we did most of the OO refactoring during version 7, we used a hack to allow instantiation without the |
Thanks @zenflow , appreciate the thorough explanation. |
@WillGoldstein No problem.. this should also serve other people wondering the same thing |
To be clear, will Swal(...) work just usually have a lint error so the fire method was added, or does it not work and .fire is mandatory? Also, why not include a (lowercase) swal method for backwards-compatibility and mark it deprecated? Or did you? |
It is included Please read the release notes for more info: https://github.com/sweetalert2/sweetalert2/releases/tag/v8.0.0 |
Wait, the lowercase version is a synonym for the class?
I'm confused, is it a breaking change or did you make the lowercase be
backwards-compatible?
…On Mon, Jun 17, 2019, 3:22 AM Limon Monte ***@***.***> wrote:
To be clear, will Swal(...) work just usually have a lint error so the
fire method was added, or does it not work and .fire is mandatory?
.fire() is mandatory.
Also, why not include a (lowercase) swal method for
backwards-compatibility and mark it deprecated? Or did you?
It is included
[image: image]
<https://user-images.githubusercontent.com/6059356/59585703-cc3f7000-90e9-11e9-8a32-60b73cbff810.png>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1438?email_source=notifications&email_token=AAAYAUED4PRZRVYW4DWWSBDP243URA5CNFSM4G4H2XFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX2ITXQ#issuecomment-502565342>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAYAUD43AJ7Y6D3B2BQHNTP243URANCNFSM4G4H2XFA>
.
|
The lowercase is there for backwards-compatibility, yes. But it is recommended to use |
@WillGoldstein You can use the following in your top-level script (just under the sweetalert2 library) to preserve code that rely on the legacy swal(...) syntax: if (window.Swal && typeof window.Swal.fire === "function") {
const original = window.swal;
window.swal = function () {
window.Swal.fire.call(original, ...arguments);
};
} I hope this helps. |
Just wondering, why the addition of a breaking change like .fire which seems completely unnecessary at first glance?
The text was updated successfully, but these errors were encountered: