@bmansurov - could you look over this one?

Why is this a problem?
It's not a case of nice, it's a case of convenience and using mfExtend makes writing code a lot easier and I don't see why this would cause a problem.

In T159505#3077528, @Jdlrobson wrote:

Why is this a problem?
It's not a case of nice, it's a case of convenience and using mfExtend makes writing code a lot easier and I don't see why this would cause a problem.

Then pitch it as an upstream change rather than fork the codebase.

I did a while back... happy to do this again.
I think the word fork is a little strong though - it's a helper function.

This sounds reasonable, and it's not much extra code to support.

I'm not sure how I feel about having two different ways to do the same things, though. It makes people confused about what the difference is, and it makes grepping harder.

It could be an optional third argument to inheritClass if you are worried about grepping.

Note React has a similar method that allows this React.createClass({})

In T159505, @Jdforrester-WMF wrote:

Though it's "nice" to define the prototype as an object and so not have to do foo.prototype.bar = baz a bunch, it's not the way we write all our other code so sharing/upstreaming is much harder.

Yeah, the added value is quite minimal (not very "enabling") and I'd err on the side of minimalism and consistency to not add this new method to the library.

However I do like @matmarex's idea of adding this to inheritClass as a third argument. That could work well.

I'm worried about the class definition pattern we have though. The current mfExtend implementation doesn't seem to scale to OOjs class features beyond methods.

Current convention:

function Example() {
}

/* Setup */
OO.initClass/inheritClass/mixinClass();

/* Events */

/**
 * @event foo
 */

/* Static Properties */

Example.static.foo = ;

/* Static Methods */

Example.static.foo = ;

/* Methods */

Example.prototype.foo = ;

One thing to be aware of is that inheritClass/initClass must be the first thing after the class constructor is declared because

1. inheritClass will re-define the prototype. As such, doing it later will silently blow away previous assignments.
2. initClass is what sets up the .static container. Before that, no properties can be assigned to undefined/non-object.

This means we'd have to do methods near Setup, and moving everything else down seems counter-intuitive (and inconsistent). Let's come up with some ideas that would scale to the full feature set of OOjs classes in some intuitive manner? Perhaps static can be part of it, too?

So.. if someone writes a patch for this will someone in the OOjs team commit to reviewing and merging it? Or would they prefer to be responsible for providing it?
Guidance needed on how to proceed @Jdforrester-WMF

@Jdlrobson Please do so, if you've got time for it. We're gonna review it.

Change 347047 had a related patch set uploaded (by Jdlrobson):
[oojs/core@master] inheritClass takes optional third argument to help creating classes

https://gerrit.wikimedia.org/r/347047

I've written some tests. I just wanted to check we're okay with the proposed behaviour before I go about implementing it.

Change 347049 had a related patch set uploaded (by Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Prepare for adoption of OOjs core helper function

https://gerrit.wikimedia.org/r/347049

@Jdlrobson As mentioned, patches are welcome, however, please do be prepared to address the design questions from T159505#3085195. Without that, I don't think we should accept a mere third-parameter as-is. It would do more harm than good, providing only more inconsistency for little added value.

If that's all we're adding one could just as easily assign to the prototype multiple times as most projects do now - or call extend(Class.prototype, { .. }) right after calling inherit() without it being a third parameter. Certainly better for isolation, and allows the prototype methods to remain in their conventional position after events and static properties.

I'm also in favor of removing mfExtend. I've recently watched The Post JavaScript Apocalypse - Douglas Crockford which I think is relevant to the discussion here.

Change 347049 abandoned by Jdlrobson:
Prepare for adoption of OOjs core helper function

Reason:
lack of interest... not interested in working on this anymore.

https://gerrit.wikimedia.org/r/347049

@Krinkle I guess a static object could be a 4th argument if necessary, but right now that seems like over-engineering. Are we going to need this? The third argument makes it convenient to build simple classes.

Why can't static properties be set as before (are you saying calling initClass with a prototype before defining static properties is problematic?)

Change 347047 abandoned by Jdlrobson:
inheritClass takes optional third argument to help creating classes

Reason:
not working on this

https://gerrit.wikimedia.org/r/347047