Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

fixes #841: bring back the `extended` hook #1960

Closed
wants to merge 1 commit into from
@michaelficarra
Collaborator

I say we go for it.

@jashkenas
Owner

Thanks for the implementation -- we should definitely decide this one way or another before 1.2.1, but let's not merge this quite yet...

@michaelficarra
Collaborator

Of course.

@michaelficarra
Collaborator

@jashkenas: we're going to need another release soon (a new minor version), so how are we going to handle this? I say we merge it.

@michaelficarra michaelficarra referenced this pull request
Closed

1.3.0 #2135

@jashkenas
Owner

I had a nice long chat with @wycats about Ember's need for the extended hook, and I'm afraid I persuaded myself that we shouldn't do it.

There are good arguments for running extended before the class body is defined -- to make functions that it adds to the class object available within the class body; and there are good arguments for running extended after the class body is defined -- in case it wants to iterate through the prototype and enhance properties of a certain kind. Given that both are valid use cases, I don't think we should do either. Instead:

class A extends B
  B.extended(this)
  ... class body ...


class A extends B
  ... class body ...
  B.extended(this)

... or many other possible uses of dynamic hooks during class definition. Hard-coding a single extended hook would add magic to our class semantics (above and beyond JS) while hiding the much more powerful abilities of executable class bodies and JavaScript exposed prototypes. I don't think it's worth it.

@jashkenas jashkenas closed this
@michaelficarra
Collaborator

I can accept that reasoning. Thanks for looking into this.

@wycats

We also discussed the possibility of adding in a compiler hook so that it would be easier to maintain an Ember variant of CoffeeScript without forking the entire project.

Thoughts?

@michaelficarra
Collaborator

@wycats: Like #1968? Be sure to read #1023 as well.

@grayrest

My extended use case would also work with a compiler hook.

@jashkenas
Owner

I think in this case you'd just want a finer-grained breakdown of the "class" compilation function in nodes.coffee ... then you'd have (ideally) a single function to override. It's not an "events" thing, I don't think.

@wycats

I'd optimally want to be able to subclass the compiler and override some hooks, rather than try to do something with events.

@collin

I'm doing some terribly awful things to coffeescript and "classes" https://github.com/collin/pathology

I would also like to be able to flip some switches and override the 'class' part of the compiler.

But I think it's potentially a recipe for sadness. Say if I want to use two libraries that need to compiler overridden in different incompatible ways.

What are the use cases for class extension hooks other than wrapping around the class body? I can see re-opening a class, but not much else.

@nathansobo

What about beforeExtended and afterExtended to cover the two cases discussed above? As it stands users of my library will have to call @extended manually at the top of the class body, which isn't horrible but could be prettier.

@collin

I think a better hook would be executeClassBody, which would be passed in a function to be executed as the class body.

This would allow for extensions that hook before extension, after extension and allow for re-opening of classes.

@jashkenas you mention other scenarios that you don't want to trample on, I'd like to know what those are in case I'm completely missing the boat on this.

@lancejpollard

+10

Adding a compiler hook would be ideal so as to avoid creating a custom fork of coffeescript that does something like: https://gist.github.com/2365817.

@d4tocchini

Thanks @viatropos.

The extended hook was not enough for Ember compatibility. I added defineProperty, defineStaticProperty & extend hooks. Results in many atomic ember.reopen & ember.reopenClass, but it does the trick without screwing up lexographic ordering of methods executed in class bodies. Bottom-line, CoffeeScript & Ember should get along.

A more generalized mechanism for extending the compiler would be awesome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 21, 2011
  1. @michaelficarra
This page is out of date. Refresh to see the latest.
View
4 lib/coffee-script/nodes.js
@@ -1,7 +1,7 @@
(function() {
var Access, Arr, Assign, Base, Block, Call, Class, Closure, Code, Comment, Existence, Extends, For, IDENTIFIER, IDENTIFIER_STR, IS_STRING, If, In, Index, LEVEL_ACCESS, LEVEL_COND, LEVEL_LIST, LEVEL_OP, LEVEL_PAREN, LEVEL_TOP, Literal, METHOD_DEF, NEGATE, NO, Obj, Op, Param, Parens, RESERVED, Range, Return, SIMPLENUM, Scope, Slice, Splat, Switch, TAB, THIS, Throw, Try, UTILITIES, Value, While, YES, compact, del, ends, extend, flatten, last, merge, multident, starts, unfoldSoak, utility, _ref,
__hasProp = Object.prototype.hasOwnProperty,
- __extends = function(child, parent) { for (var key in parent) { if (__hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor; child.__super__ = parent.prototype; return child; },
+ __extends = function(child, parent) { for (var key in parent) { if (__hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor; child.__super__ = parent.prototype; if(typeof parent.extended == 'function') parent.extended(child); return child; },
__indexOf = Array.prototype.indexOf || function(item) { for (var i = 0, l = this.length; i < l; i++) { if (i in this && this[i] === item) return i; } return -1; };
Scope = require('./scope').Scope;
@@ -2542,7 +2542,7 @@
UTILITIES = {
"extends": function() {
- return "function(child, parent) { for (var key in parent) { if (" + (utility('hasProp')) + ".call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor; child.__super__ = parent.prototype; return child; }";
+ return "function(child, parent) { for (var key in parent) { if (" + (utility('hasProp')) + ".call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor; child.__super__ = parent.prototype; if(typeof parent.extended == 'function') parent.extended(child); return child; }";
},
bind: function() {
return 'function(fn, me){ return function(){ return fn.apply(me, arguments); }; }';
View
2  src/nodes.coffee
@@ -1860,7 +1860,7 @@ UTILITIES =
# Correctly set up a prototype chain for inheritance, including a reference
# to the superclass for `super()` calls, and copies of any static properties.
extends: -> """
- function(child, parent) { for (var key in parent) { if (#{utility 'hasProp'}.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor; child.__super__ = parent.prototype; return child; }
+ function(child, parent) { for (var key in parent) { if (#{utility 'hasProp'}.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor; child.__super__ = parent.prototype; if(typeof parent.extended == 'function') parent.extended(child); return child; }
"""
# Create a function bound to the current value of "this".
View
69 test/classes.coffee
@@ -272,22 +272,22 @@ test "nothing classes", ->
c = class
ok c instanceof Function
-
-
+
+
test "classes with static-level implicit objects", ->
-
+
class A
@static = one: 1
two: 2
-
+
class B
@static = one: 1,
two: 2
-
+
eq A.static.one, 1
eq A.static.two, undefined
eq (new A).two, 2
-
+
eq B.static.one, 1
eq B.static.two, 2
eq (new B).two, undefined
@@ -546,61 +546,72 @@ test "#1598: super works for static methods too", ->
'pass? ' + super
eq Child.method(), 'pass? yes'
-
+
test "#1842: Regression with bound functions within bound class methods", ->
-
+
class Store
@bound: =>
do =>
eq this, Store
-
+
Store.bound()
-
+
# And a fancier case:
-
+
class Store
-
+
eq this, Store
-
+
@bound: =>
do =>
eq this, Store
-
+
@unbound: ->
eq this, Store
-
+
instance: =>
ok this instanceof Store
-
+
Store.bound()
Store.unbound()
(new Store).instance()
-
+
test "#1876: Class @A extends A", ->
class A
class @A extends A
-
+
ok (new @A) instanceof A
-
+
test "#1813: Passing class definitions as expressions", ->
ident = (x) -> x
-
+
result = ident class A then x = 1
-
+
eq result, A
-
+
result = ident class B extends A
x = 1
-
+
eq result, B
-
+
test "#494: Named classes", ->
-
+
class A
eq A.name, 'A'
-
+
class A.B
- eq A.B.name, 'B'
-
- class A.B["C"]
+ eq A.B.name, 'B'
+
+ class A.B["C"]
ok A.B.C.name isnt 'C'
+
+test "#841: bring back the `extended` hook", ->
+ class A
+ @extended: (subclass) =>
+ subclass.extended = @extended
+ subclass.another = @extended
+ class B extends A
+ ok Object::hasOwnProperty.call B, 'extended'
+ eq B.extended, A.extended
+ ok Object::hasOwnProperty.call B, 'another'
+ eq B.another, A.extended
Something went wrong with that request. Please try again.