HubSpot / mixen

Combine Javascript classes on the fly
http://github.hubspot.com/mixen
MIT License
85 stars 2 forks source link

Nested/combined mixens still don't work in some cases #8

Open timmfin opened 10 years ago

timmfin commented 10 years ago

I found a case where the fixes/improvements in https://github.com/HubSpot/mixen/pull/5/files don't work. Here is an erroring spec:

it 'should be able to mixin mixens (not constructor), mixin order changed', ->
  order = ''

  class Module1
    init: ->
      super
      order += '-1-'

  class Module2
    init: ->
      super
      order += '-2-'

  class Module12 extends Mixen(Module1, Module2)
    init: ->
      super
      order += '-12-'

  class Module3
    init: ->
      super
      order += '-3-'

  class Module312 extends Mixen(Module12, Module3)
    init: ->
      super
      order += '-312-'

  inst = new Module312
  inst.init()

  expect(order).toBe('-3--2--1--12--312-')

It is the same test as "should be able to mixin mixens (not constructor)" but switches the order of Module3 and Module12 in definition of Module312.

When run, the output is -1--12--312-, so the init func of Module2 and Module3 are never called.

ps: I know the maintainers might not have time right now to look into this, but I wanted to write the bug so others were aware. (I took a little bit of a shot, but didn't have much luck in my first attempt).

zackbloom commented 10 years ago

Thanks Tim, reproducing it in a test was a big part of the battle.

snyderpa commented 8 years ago

haha -- I was ready to post the same test, but I am running against release 0.5.2 source and getting this:

Running "jasmine:src" (jasmine) task
Testing jasmine specs via phantom
...............x
Mixen:: should be able to traverse super chain for mixed-in mixens: failed
  RangeError: Maximum call stack size exceeded. in file:///Users/psnyder/Downloads/mixen-0.5.2/mixen.js (line 109) (1)
16 specs in 0.375s.
>> 1 failures

As a workaround, I am mixing everything into my base classes instead of just the classes that need them, which seems to contradict the stated benefits of the module -- bummer. Is there a better workaround? Is there an expectation of releasing a fix? Or does this look like a permanent limitation?