Add Batman.Object.get('className') #404

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

airhorns commented Apr 25, 2012

  • Adds className property to all BatmanObject subclasses
  • Adds specialized implementations for Model and Controller which use
    domain-meaningful keys if present
  • Adds Batman.config.minificationErrors which will cause batman to
    error out if a class' name is calculated in such a way that is unsafe
    for minification.

@jamesmacaulay and @nciagra for review please

Why have className at all? The point of using domain-specific names was so that we only require what is needed. The way it is here, if you specify a storageKey on a model, we end up singularizing it, camelizing it, then pluralizing it again:

5907fd7#L1R2712
5907fd7#L1R1831

I would take out all the className stuff and just have one macro each for controllers and models:

class Some.ThingsController extends Batman.Controller
  @routingKey "things"
class Some.WelcomeController extends Batman.Controller
  @routingKey "welcome"

class Some.Thing extends Batman.Model
  @storageKey "things"

Since they're macros, we can easily use them to index the classes by these names as soon as they're defined (e.g. making Batman.Dispatcher's ControllerDirectory logic unnecessary).

Also, I know I suggested routeKey before, and maybe routingKey is better, but I think there's a better name to be found yet. routingName? I'd almost want to call it pathName, since that's what it boils down to 99% of the time.

Contributor

airhorns commented Apr 25, 2012

I added className for a number of reasons:

  • Other client code or plugins might want to inspect the name of a class
  • We might have a way or CoffeeScript might give us a way to auto-generate it in the future, so now we have one place that everything else will look who's implementation can change
  • It can be auto-calculated in development or non minified apps and we need somewhere to store that

I had another one but I lost it. All of these reasons are symptoms of one thing: we can't actually get the name of a class at minified runtime and we wish we could. I know we'll come up for more uses for className eventually and maybe some things we didn't do because $functionName was unreliable will resurface. For this reason, I like the idea of className, which then storageKey and routingKey or whatever can interpret.

I also hate it. I don't want to emulate a missing language feature and I'd rather only ask for the things that we do need, like storageKey and routingKey. If we did this we'd loose the ability to ask any object what it's class' name was but we have a somewhat clearer API, and we get rid of the admittedly very lame and incorrect semantics of inferring the class name from something like storageKey or what have you. We also avoid the inefficient string skullduggery to normalize all the forms.

I don't know what to... what do you think @nciagra, @tobi?

Well, my basic position is YAGNI. In a bit more detail, and to address your bullets:

  • Other client code or plugins might want to do a lot of things.
  • I really don't think it's in the cards for CoffeeScript. If it happens, then we can obviously take advantage of that in the future.
  • Batman should be fully functional in a minified environment, and $functionName breaks in a minified environment. This tells me that we should simply not rely on $functionName (and indeed, that it should be removed). The fact that it can be calculated in development really doesn't matter if we can't use it in production.

The only reason class names are used so heavily to infer things in Rails is that Ruby makes it dead easy (and reliable), and the app developer doesn't need to do anything extra.

Contributor

airhorns commented Apr 30, 2012

I think a bit of this disagreement stems from our differences regarding unminified environments. Minified environments are not the lowest common denominator because minification is opt in. We can't get around people using IE8, so we must support it, but we can choose to not minify and thus not have the nasty strings all over the place. Everyone and their mother minifies in production, so we default to throwing errors when $functionName gets called, but there's an option to turn off those errors for existing apps and quickies.

That said, I think you are right about className: we can't accurately infer it from routingKey or storageKey, because those are intentionally overridable such that they don't even pertain to the name of the class. I'll make us some macros.

Contributor

airhorns commented Apr 30, 2012

Goddamnit I change my mind again. We need className on Model for things like Shopify's STI support and Batman's polymorphism, and in these cases the className is the key I would use to represent the data "in the domain" as we have been talking about. I'm going to remove the storageKey inference for model and remove className for controller.

airhorns closed this in 2073252 Apr 30, 2012

rmosolgo removed the Code Review label Aug 7, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment