Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Aspecting Methods Fragile #66

Closed
kitsonk opened this issue Sep 27, 2016 · 2 comments · Fixed by #87
Closed

Aspecting Methods Fragile #66

kitsonk opened this issue Sep 27, 2016 · 2 comments · Fixed by #87
Assignees
Milestone

Comments

@kitsonk
Copy link
Member

kitsonk commented Sep 27, 2016

Aspecting methods can be fragile and dangerous, especially with the diamond problem.

For example:

import compose from 'dojo-compose/compose';

const createFoo = compose({
    foo() {
        console.log('createFoo');
    }
});

const createFooAspect = createFoo
    .around('foo', function (origFn) {
        return function (...args: any[]) {
            console.log('createFooAspect');
            origFn.apply(this, args);
        });
    });

const createFooBar = createFoo
    .mixin({
        mixin: {
            bar() {}
        }
    });

const createFooBarAspect = createFooAspect
    .mixin(createFooBar);

const fooBarAspect = createFooBarAspect();
fooBarAspect.foo(); // Only logs `createFoo`

Which is totally surprising, because the developer may not have had knowledge that createFooBar was going to overwrite the aspected foo().

@kitsonk
Copy link
Member Author

kitsonk commented Sep 27, 2016

@novemberborn and I were discussing... what might make sense to solve this is to retain a reference to each prototype mixed in, and instead of mixin in a new prototype into a base, we unwind the whole process and create a new prototype based on all the unique prototypes that are part of the final factory. Also, applying advice after each step in the prototype "reducer".

@kitsonk
Copy link
Member Author

kitsonk commented Oct 3, 2016

@maier49 and I had further discussions and talked about a different approach to mixins that would also address this problem...

@kitsonk kitsonk self-assigned this Oct 17, 2016
@dylans dylans added this to the 2016.10 milestone Oct 24, 2016
kitsonk added a commit that referenced this issue Oct 26, 2016
* Refactor private factory data
* Rename copyProperties to assignProperties
* Clone factory private state data
* Refactor factory creation to better handle application of advice to factory prototypes

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

Successfully merging a pull request may close this issue.

2 participants