Skip to content
This repository has been archived by the owner on Nov 17, 2020. It is now read-only.

displayName #31

Open
jamiebuilds opened this issue Aug 4, 2015 · 10 comments
Open

displayName #31

jamiebuilds opened this issue Aug 4, 2015 · 10 comments

Comments

@jamiebuilds
Copy link
Member

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
Copy link
Member Author

Thoughts @marionettejs/marionette-core ?

@jamesplease
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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

@jamesplease
Copy link
Member

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

@jamiebuilds
Copy link
Member Author

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
Copy link
Member

I'm a 👍 marionettejs/backbone.marionette#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
Copy link
Member Author

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

🔥

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
Copy link
Member Author

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants