marionettejs / backbone-metal

Classes, Mixins, Errors, and more.
ISC License
31 stars 7 forks source link

displayName #31

Open jamiebuilds opened 9 years ago

jamiebuilds commented 9 years ago

With a little bit of love Metal.Class could support (the somewhat controversial) displayName

Metal.Class.extend({
  displayName: 'Person'
});

Which could take stack items like these:

module.exports.Marionette.ItemView.extend.constructor
module.exports.Marionette.ItemView.extend.sayHello

And replace them with

Person
Person.prototype.sayHello
jamiebuilds commented 9 years ago

Thoughts @marionettejs/marionette-core ?

jamesplease commented 9 years ago

I have thought quite a lot about giving my constructors names. I forget the use case atm. But the line of thinking usually goes..

'Oh man that would be SO useful.'

'but oh so gross, so no.'

I will think about what that use case was and post another comment. fwiw it did not have to deal with stack traces

: D

jamesplease commented 9 years ago

Oh I think it may have had to do with some magic, like...

  1. automatically search for a template with the name of the view
  2. maybe automatically give it that as a class

I pretty much do those 2 things for every view.

Ikno,ikno, gross.

jamiebuilds commented 9 years ago

So what made me think of this is Babel's support for displayName in React:

It will transform:

var Cool = React.createClass({});

To:

var Cool = React.createClass({
  displayName: "Cool"
});

I was thinking I could do something similar.

jasonLaster commented 9 years ago

I think we should. It's an Mn issue too. Also chrome DevTools has done this for awhile with scope variables

jamesplease commented 9 years ago

Tbqh I don't think the stack trace solution is super useful. Not enough to warrant some build step magic, at least.

jamiebuilds commented 9 years ago

That's the thing about build step magic, it only has to be slightly useful because it's simple as hell to add ;P

Also, this is useful enough that we manually do it at work with a __name__ property on all of our classes.

paulfalgout commented 9 years ago

I'm a :+1: https://github.com/marionettejs/backbone.marionette/issues/2425

It seems that people are finding alternate ways of solving this. At least this is perhaps the most-best standard-ish way of doing it.

jamiebuilds commented 9 years ago

I have an initial version hacked together, this is the difference in the call stack:

Before

Metal.Class.extend.foo
Metal.Class.extend.constructor
MyClass.extend.constructor
(anonymous function)
MyClass.extend.baz

After:

MyClass.prototype.foo
MyClass
SubClass
superWrapper(SubClass)
SubClass.baz

:fire:

However, this change adds a bit of perf weight to subclassing. I'm wondering if we can create a "debug" version of this library to include? And we can use process.env.NODE_ENV to make the code different (using this)

If anyone wants to try out this version of the library, try this out.

jamiebuilds commented 9 years ago

Note to self: I need to make the displayName property non-enumerable