Feature/object oriented

[zenflow]

I remembered a pattern I learned from @substack, that lets you to create prototypical classes that can be instantiated without the new keyword..

sweetalert2/src/sweetalert2.js

Lines 81 to 84 in a26d125

	// handle things when constructor is invoked without the `new` keyword
	if (!(this instanceof SweetAlert)) {
	return new SweetAlert(...args)
	}

Using this we can enable a standard OO interface immediately, with almost full (see below) backwards compatibility.

So basically, you can still do await swal('foo'), but you can also do await new swal('foo'), and more importantly, class extends swal { _main(params) { ... }}. The swal instance has 1 public property: params and it is Object.freezed. It's public methods are all the methods in src/instanceMethods.js (which are also still available as actually-static methods, until the next major release), plus then(), catch() and finally() to make it a thenable.

---

Breaking change

The current changes as is will break any code depending on swal('foo') instanceof Promise being true, since swal will no longer return a Promise instance, but Swal instance which is thenable. It will be treated the same as a Promise in most cases, but there is the potential for this to break some expectations somewhere. I think it's probably negligible though, especially when considering the gain.

Another approach would be to make swal('foo') instanceof Promise be true, by just setting up the prototype chain, but that would only make it look like an instance of Promise, and could lead to more confusing bugs than the alternative of being just a simple thenable.

---

Benefits

Just to make explicit:

Extensions can be defined concisely (i.e. currently there's a lot of boilerplate, especially when there's accompanying tests), and can be typed properly (see sweetalert2/sweetalert2-react-content#15 (comment)).

...not to mention now we can think about what public instance methods/properties would make sense and be convenient. In the next major we can deprecate all the static methods that should be instance methods (and make them just instance methods). This change will also help in having a better organized API. Related: sweetalert2/sweetalert2.github.io#15 (comment)

---

[zenflow]

@limonte please review and let me know what you think, and then I have one more matter to discuss in regards to the mixin method.

[zenflow]

Check out c4e7b70 and let me know if you want any more examples. (edit: examples of extending Swal)

[zenflow]

Here's another example of extending Swal... the withCopyrightInFooter example from #968 adapted:

import Swal from 'sweetalert2'
export class CopyrightSwal extends Swal {
  _main (params) {
    return super._main({
      ...params,
      footer: (params.footer ? params.footer + '<br/>' : '') + 'Copyright Rick Sanchez'
    })
  }
}

or as a higher-order component, for better modularity and composibility:

export function withCopyright (Swal) {
  return class extends Swal {
    _main (params) {
      return super._main({
        ...params,
        footer: (params.footer ? params.footer + '<br/>' : '') + 'Copyright Rick Sanchez'
      })
    }
  }
}

const CopyrightSwal = withCopyright(Swal)

---

Now lets try extending argsToParams...

class FooterShorthandSwal extends Swal {
  static argsToParams (args) {
    if (typeof args[0] === 'string') { // if first arg is a string, make it the `footer` param...
      return {
        ...Swal.argsToParams(args.slice(1)),
        footer: args[3]
    } else { // otherwise, just proxy to parent method
      return Swal.argsToParams(args)
    }
  }
}

---

Did you know, Swal.prototype._main is an async (promise-returning) function?

class MySwal extends Swal {
  async _main (params) {
    // do something with `params`
    let result = await super._main(params)
    // do something with `result`
    return result
  }
}

[limonte]

This is the fundamental change which will make SweetAlert2 codebase much more maintainable and enjoyable to work with! You are the hero of my day @zenflow!

Just one nit to change:

[limonte]

ctor is quite a unique variable name, constructor would be better IMO

[zenflow]

I defined ctor just to make the reference shorter, it's not really needed. If you want the ref to be constructor then we might as well just make leave it this.constructor which is probably better than either. Sound ok?

[limonte]

Yup 👍

[zenflow]

Done in 1a49289

[zenflow]

@limonte ok the part about mixin.. I updated it to be OO using prototypical inheritance also, so that it is somewhat aligned with the new kind of extension, but pre-es6 oo patterns aren't 100% compatible with ES classes..

Currently, the following interoperation of Swal.mixin and extends Swalwill not work...

class ExtendedSwal extends Swal {}
const MixinSwal = ExtendedSwal.mixin({})
MixinSwal('foo') // <-- throws "Uncaught TypeError: Class constructor ExtendedSwal cannot be invoked without 'new'"
new MixinSwal('foo') // <-- throws the same error. the es super-constructor can not be `.apply`ed to the instance because `apply` can't signal the `new` keyword.

I suggest fixing it by making a small breaking change. Luckily the .mixin method is very new and not even documented yet. Let's make it use proper ES classes. That means that the class it returns must be instantiated with the new keyword. But it will allow us to mix .mixins, and extends together (specifically it will allow us to use .mixin after extending, which is the time when I'd want the fast-and-loose mixin method, not before).

If you would prefer to keep the ability to invoke mixin swals without the new keyword, I think we can handle that (edit: we can't). I can modularize the main constructor method a wee bit more (i.e. extract a doPreFlightCheck function) and then duplicate the new tiny constructor in mixin.js.. that fixes the problem by avoiding the need to apply the parent constructor... ... ... oh but shit.. child classes can extend the constructor adding/changing logic, so it's not safe to assume we know what the parent constructor does..

[zenflow]

@limonte You ok with making that little breaking change to the mixin method?

[zenflow]

I think we should change all the tests and documentation to say Swal instead of swal and SweetAlert instead of sweetAlert, to reflect the new OO paradigm. Just change the function name, not use the new keyword. It could get confusing when we are talking about a hypothetical var named swal (instance) vs Swal (class).

Can I go ahead with this?

[limonte]

I think we should change all the tests and documentation to say Swal instead of swal and SweetAlert instead of sweetAlert, to reflect the new OO paradigm. Just change the function name, not use the new keyword. It could get confusing when we are talking about a hypothetical var named swal (instance) vs Swal (class).

Can I go ahead with this?

YES PLEASE!

The whole plugin is starting to look so nice 💎

[zenflow]

In 79aaea3 I just added a static Swal.fire(...args) method that simply creates and returns a new instance of itself, for use with extended MySwal classes, since es classes require you to use the new keyword, but linters complain about using the new keyword for side-effects:

e.g.

import Swal from 'sweetalert2'

class MySwal extends Swal {/* ... */}

new MySwal('foo', 'bar') // <-- Standard lint error: "Do not use 'new' for side effects."

MySwal.fire('foo', 'bar') // <-- do this instead 

Maybe in the next major we can "not use 'new' for side effects" and make the constructor a pure function by deprecating simply calling await Swal(...args), and adding a swal.run() instance method that does the side-effects. Then Swal.fire(...args) would behave the same, but would be a shorthand for calling new Swal(...args).run().

[zenflow]

I'm totally open to renaming Swal.fire btw

[zenflow]

I suggest fixing it by making a small breaking change. Luckily the .mixin method is very new and not even documented yet. Let's make it use proper ES classes.

Done in 7866171

edit: This was reverted and replaced with a better solution in PR #1043. You can now invoke a MySwal that was created with Swal.mixin without the new keyword (or the fire static method).

You must use MySwal.fire(...args) for the swals you create with .mixin.

[zenflow]

I think we should change all the tests and documentation to say Swal instead of swal and SweetAlert instead of sweetAlert

Done in 6c2ee37 and 7fbce93

[zenflow]

Ready to merge IMO..

Next step will be to separate logic for distinct features into HOCs, ready to be extracted into separate packages in the next major...

• toasts
• form-builder
• global defaults, i.e. setDefaults/resetDefaults, which is deprecated but will be nice to have for legacy apps
• "queue" which is kinda unnecessary too now that control-flow is so simple to do yourself with async/await. The nice thing about modularization like this is that we don't have to agree about the validity of features.. It can exist, and people can maintain it and use it, separately, without affecting the precious core.
• maybe even the "input" feature?!?! this would allow a space for experimentation and innovation with this feature

[zenflow]

I can't wait to integrate this change into sweetalert2-react-content

[limonte]

@zenflow I can't understand what is the reason of gzipped dist size grown 0.5KB? In this PR there's just reorganization of the code, which shouldn't lead to the bigger dist.

[zenflow]

I tried e9692d6 but that barely made a difference

Also fixed a little problem here #1041, but I don't think that will make much of a difference either..

[zenflow]

@limonte ahh maybe this is it.. this is the compiled mixin function now (jsdoc comment excluded):

var _extends$2 = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; };

var _createClass = function () { function defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } } return function (Constructor, protoProps, staticProps) { if (protoProps) defineProperties(Constructor.prototype, protoProps); if (staticProps) defineProperties(Constructor, staticProps); return Constructor; }; }();

var _get = function get(object, property, receiver) { if (object === null) object = Function.prototype; var desc = Object.getOwnPropertyDescriptor(object, property); if (desc === undefined) { var parent = Object.getPrototypeOf(object); if (parent === null) { return undefined; } else { return get(parent, property, receiver); } } else if ("value" in desc) { return desc.value; } else { var getter = desc.get; if (getter === undefined) { return undefined; } return getter.call(receiver); } };

function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }

function _possibleConstructorReturn(self, call) { if (!self) { throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); } return call && (typeof call === "object" || typeof call === "function") ? call : self; }

function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new TypeError("Super expression must either be null or a function, not " + typeof superClass); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, enumerable: false, writable: true, configurable: true } }); if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass; }

function mixin(mixinParams) {
  var Swal = this;
  return function (_Swal) {
    _inherits(MixinSwal, _Swal);

    function MixinSwal() {
      _classCallCheck(this, MixinSwal);

      return _possibleConstructorReturn(this, (MixinSwal.__proto__ || Object.getPrototypeOf(MixinSwal)).apply(this, arguments));
    }

    _createClass(MixinSwal, [{
      key: "_main",
      value: function _main(params) {
        return _get(MixinSwal.prototype.__proto__ || Object.getPrototypeOf(MixinSwal.prototype), "_main", this).call(this, _extends$2({}, mixinParams, params));
      }
    }]);

    return MixinSwal;
  }(Swal);
}

[limonte]

This change is so good, I will merge it and make a new release right now!