Git Product home page Git Product logo

compose's Introduction

@dojo/compose

WARNING This package is deprecated in favor of functionality found elsewhere in Dojo 2. This package is not being further developed at this time as its feature set is redundant with other capabilities.

Build Status codecov.io npm version

A composition library, which works well in a TypeScript environment.

Background

In creating this library, we were looking to solve the following problems with Classes and inheritance in ES6+ and TypeScript:

  • Single inheritance: ES6 classes are essentially syntactic sugar around prototypal inheritance. A descended class can only derive from a single ancestor class
  • Confusion around mixins and inheritance
  • No plans to add mixins to ES6+ (original proposal was withdrawn)
  • TypeScript class decorators do not augment the shape of the underlying class based on the decorator, and were not an effective mechanism to provide mixins/traits functionality
  • ES6 classes do not support properties in the class prototype
  • TypeScript classes do support properties in the class prototype plus some visibility modifiers. However, TypeScript’s private properties are headed on a collision course with ES private properties with are currently proposed for a future version of the language
  • Both ES6 and TypeScript classes allow for the mutability of ancestor code, which can cause unexpected side effects
  • Traditional inheritance often leads to large base classes and complex inheritance chains

Prior to TypeScript 1.6, we did not have an easy way to solve this, but thankfully the TypeScript team added support for generic types which made this library possible.

Goals

Pragmatism

A purely functional library or a purely OO library does not solve the needs of our users.

Composition

Embrace the concepts of "composition" versus classical Object Oriented inheritance. The classical model follows a pattern whereby you add functionality to an ancestor by extending it. Subsequently all other descendants from that class will also inherit that functionality.

In a composition model, the preferred pattern is to create logical feature classes which are then composited together to create a resulting class. It is believed that this pattern increases code reuse, focuses on the engineering of self contained "features" with minimal cross dependency.

Factories

The other pattern supported by compose is the factory pattern. When you create a new class with compose, it will return a factory function. To create a new instance of an object, you simply call the factory function. When using constructor functions, where the new keyword is used, it limits the ability of construction to do certain things, like the ability for resource pooling.

Immutability

Also, all the classes generated by the library are "immutable". Any extension of the class will result in a new class constructor and prototype. This is in order to minimize the amount of unanticipated consequences of extension for anyone who is referencing a previous class.

The library was specifically designed to work well in a environment where TypeScript is used, to try to take advantage of TypeScript's type inference, intersection types, and union types. This in ways constrained the design, but we feel that it has created an API that is very semantically functional.

Challenges and possible changes

The TypeScript 2.2 team made a recent change to their Class implementation that improves support for mixins and composable classes, which may be sufficient for most use cases.

In parallel, we've found that there remain challenges in properly typing widgets and other composed classes, which is a potential barrier to entry for new and experienced users. Furthermore, we're finding that the promise of composition has not been fully appreciated with @dojo/widgets. For example, with the createDialog widget, it's already dependent on themeable and createWidgetBase. Both of these depend on createEvented, which depends on createDestroyable. While four layers deep isn't terrible, dojo/compose is not currently preventing us from repeating history unfortunately.

As such, we are exploring options for leveraging TypeScript 2.2 Classes for Dojo 2, which may change dojo/compose or may reduce our reliance on it. We'll have an update once we know more. Regardless of the final approach we take, Dojo 2 will have a solid solution for object composition.

Usage

To use @dojo/compose, install the package along with its required peer dependencies:

npm install @dojo/compose

# peer dependencies
npm install @dojo/core
npm install @dojo/has
npm install @dojo/shim

Features

The examples below are provided in TypeScript syntax. The package does work under JavaScript, but for clarity, the examples will only include one syntax. See below for how to utilize the package under JavaScript.

Class Creation

The library supports creating a "base" class from ES6 Classes, JavaScript constructor functions, or an object literal prototype. In addition an initialization function can be provided.

Creation

The compose module's default export is a function which creates classes. This is also available as .create() which is decorated onto the compose function.

If you want to create a new class via a prototype and create an instance of it, you would want to do something like this:

import compose from '@dojo/compose/compose';

const fooFactory = compose({
	foo: function () {
		console.log('foo');
	},
	bar: 'bar',
	qat: 1
});

const foo = fooFactory();

If you want to create a new class via an ES6/TypeScript class and create an instance of it, you would want to do something like this:

import compose from '@dojo/compose/compose';

class Foo {
	foo() {
		console.log('foo');
	};
	bar: string = 'bar';
	qat: number = 1;
}

const fooFactory = compose(Foo);

const foo = fooFactory();

You can also subclass:

import compose from '@dojo/compose/compose';

const fooFactory = compose({
	foo: function () {
		console.log('foo');
	},
	bar: 'bar',
	qat: 1
});

const myFooFactory = compose(fooFactory);

const foo = myFooFactory();

Creation with Initializer

During creation, compose takes a second optional argument, which is an initializer function. The constructor pattern for all compose classes is to take an optional options argument. Therefore the initialization function should take this optional argument:

import compose from '@dojo/compose/compose';

interface FooOptions {
	foo?: Function,
	bar?: string,
	qat?: number
}

function fooInit(options?: FooOptions) {
	if (options) {
		for (let key in options) {
			this[key] = options[key]
		}
	}
}

const fooFactory = compose({
	foo: function () {
		console.log('foo');
	},
	bar: 'bar',
	qat: 1
}, fooInit);

const foo1 = fooFactory();
const foo2 = fooFactory({
	bar: 'baz'
});

Class Extension

The compose module's default export also has a property, extend, which allows the enumerable, own properties of a literal object or the prototype of a class or ComposeFactory to be added to the prototype of a class. The type of the resulting class will be inferred and include all properties of the extending object. It can be used to extend an existing compose class like this:

import * as compose from 'dojo/compose';

let fooFactory = compose.create({
    foo: 'bar'
});

fooFactory = compose.extend(fooFactory, {
    bar: 1
});

let foo = fooFactory();

foo.foo = 'baz';
foo.bar = 2;

Or using chaining:

import * as compose from 'dojo/compose';

const fooFactory = compose.create({
    foo: 'bar'
}).extend({
    bar: 1
});

let foo = fooFactory();

foo.foo = 'baz';
foo.bar = 2;

Implementing an interface

extend can also be used to implement an interface:

import * as compose from 'dojo/compose';

interface Bar {
    bar?: number;
}

const fooFactory = compose.create({
    foo: 'bar'
}).extend<Bar>({});

Or

const fooFactory = compose.create({
    foo: 'bar'
}).extend(<Bar> {});

Adding Initialization Functions

As factories are extended or otherwise modified, it is often desirable to provide additional initialization logic for the new factory. The init method can be used to provide a new initializer to an existing factory. The type of the instance and options will default to the type of the compose factory prototype and the type of the options argument for the last provided initializer.

const createFoo = compose({
	foo: ''
}, (instance, options: { foo: string } = { foo: 'foo' }) => {
	// Instance type is inferred based on the type passed to
	// compose
	instance.foo = options.foo;
});

const createFooWithNewInitializer = createFoo
	.init((instance, options?) => {
		// If we don't type the options it defaults to { foo: string }
		instance.foo = (options && options.foo) || instance.foo;
	});

const createFooBar = createFoo
	.extend({ bar: 'bar' })
	.init((instance, options?) => {
		// Instance type is updated as the factory prototype is
		// modified, it now has foo and bar properties
		instance.foo = instance.bar = (options && options.foo) || instance.foo;
	});

Sometimes, as in the createFooBar example above, additional properties may need to be added to the options parameter of the initialize function. A new type can be specified as a generic or by explicitly typing options in the function declaration.

const createFoo = compose({
	foo: ''
}, (instance, options: { foo: string } = { foo: 'foo' }) => {
	instance.foo = options.foo;
});

const createFooBar = createFoo
	.extend({ bar: 'bar' })
	// Extend options type with generic
	.init<{ foo: string, bar: string }>((instance, options?) => {
		instance.foo = (options && options.foo) || 'foo';
		instance.bar = (options && options.bar) || 'bar';
	});

const createFooBarToo = createFoo
	.extend({ bar: 'bar' })
	// Extend options type in function signature
	.init(instance, options?: { foo: string, bar: string }) => {
		instance.foo = (options && options.foo) || 'foo';
		instance.bar = (options && options.bar) || 'bar';
	});

Merging of Arrays

When mixing in or extending classes which contain array literals as a value of a property, compose will merge these values instead of over writing, which it does with other value types.

For example, if I have an array of strings in my original class, and provide a mixin which shares the same property that is also an array, those will get merged:

const createFoo = compose({
	foo: [ 'foo' ]
});

const createBarMixin = compose({
	foo: [ 'bar' ]
});

const createFooBar = createFoo.mixin(createBarMixin);

const foo = createFooBar();

foo.foo; // [ 'foo', 'bar' ]

There are some things to note:

  • The merge process will eliminate duplicates.
  • When the factory is invoked, it will "duplicate" the array from the prototype, so createFoo.prototype.foo !== foo.foo.
  • If the source and the target are not arrays, like other mixing in, last one wins.

Using Generics

compose utilizes TypeScript generics and type inference to type the resulting classes. Most of the time, this will work without any need to declare your types. There are situations though where you may want to be more explicit about your interfaces and compose can accommodate that by passing in generics when using the API. Here is an example of creating a class that requires generics using compose:

class Foo<T> {
    foo: T;
}

class Bar<T> {
    bar(opt: T): void {
        console.log(opt);
    }
}

interface FooBarClass {
	<T, U>(): Foo<T>&Bar<U>;
}

let fooBarFactory: FooBarClass = compose(Foo).extend(Bar);

let fooBar = fooBarFactory<number, any>();

Overlaying Functionality

If you want to make modifications to the prototype of a class that are difficult to perform with simple mixins or extensions, you can use the overlay function provided on the default export of the compose module. overlay takes one argument, a function which will be passed a copy of the prototype of the existing class, and returns a new class whose type reflects the modifications made to the existing prototype:

import * as compose from 'dojo/compose';

const fooFactory = compose.create({
    foo: 'bar'
});

const myFooFactory = fooFactory.overlay(function (proto) {
    proto.foo = 'qat';
});

const myFoo = myFooFactory();
console.log(myFoo.foo); // logs "qat"

Note that as with all the functionality provided by compose, the existing class is not modified.

Adding static properties to a factory

If you want to add static methods or constants to a ComposeFactory, the static method allows you to do so. Any properties set this way cannot be altered, as the returned factory is frozen. In order to modify or remove a static property on a factory, a new factory would need to be created.

const createFoo = compose({
	foo: 1
}).static({
	doFoo(): string {
		return 'foo';
	}
});

console.log(createFoo.doFoo()); // logs 'foo'

// This will throw an error
// createFoo.doFoo = function() {
//	 return 'bar'
// }

const createNewFoo = createFoo.static({
	doFoo(): string {
		return 'bar';
	}
});

console.log(createNewFoo.doFoo()); // logs 'bar'

If a factory already has static properties, calling its static method again will not maintain those properties on the returned factory. The original factory will still maintain its static properties.

const createFoo = compose({
	foo: 1
}).static({
	doFoo(): string {
		return 'foo';
	}
})

console.log(createFoo.doFoo()); //logs 'foo'

const createFooBar = createFoo.static({
	doBar(): string {
		return 'bar';
	}
});

console.log(createFooBar.doBar()); //logs 'bar'
console.log(createFoo.doFoo()); //logs 'foo'
//console.log(createFooBar.doFoo()); Doesn't compile
//console.log(createFoo.doBar()); Doesn't compile

Static properties will also be lost when calling mixin or extend. Because of this, static properties should be applied to the 'final' factory in a chain.

Mixins

One of the goals of compose is to enable the reuse of code, and to allow clean separation of concerns. Mixins provide a way to encapsulate functionality that may be reused across many different factories.

This example shows how to create and apply a mixin:

const createFoo = compose({ foo: 'foo'});

const fooMixin = compose.createMixin(createFoo);

createFoo.mixin(fooMixin);

In this case the mixin won't actually do anything, because we applied it immediately after creating it. Another thing to note in this exapmle, is that passing createFoo to createMixin is optional, but is generally a good idea. This lets the mixin know that it should be mixed into something that provides at least the same functionality as createFoo, so the mixin can automatically include the prototype and options types from createFoo.

In order to create a mixin that's actually useful, we can use any of the ComposeFactory methods discussed above. The mixin will record these calls, and when mixed into a factory will apply them as if they were called directly on the factory.

const createFoo = compose({
	foo: 'foo'
}, (instance, options?: { foo: string }) => {
	instance.foo = (options && options.foo) || 'foo';
});

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

const fooMixin = compose.createMixin(createFoo)
	// Because we passed createFoo, the types of instance and options
	// are both { foo: string }
	.init((instance, options?) => {
		instance.foo = (options && options.foo) + 'bar';
	});
	.extend({ baz: 'baz'});

const createFooBaz = createFoo.mixin(fooMixin);
/* Equivalent to calling
	createFoo
        .init((instance, options?) => {
            instance.foo = (options && options.foo) + 'bar';
        });
        .extend({ baz: 'baz'});
*/

const createFooBarBaz = createFooBar.mixin(fooMixin);
/* Equivalent to calling
	createFooBar
        .init((instance, options?) => {
            instance.foo = (options && options.foo) + 'bar';
        });
        .extend({ baz: 'baz'});
*/

Compose also provides the ability to mixin a factory directly, or a FactoryDescriptor object, but these are allowed only for the backwards compatibility. The createMixin API is the preferred method for creating and applying mixins.

How do I use this package?

The easiest way to use this package is to install it via npm:

$ npm install @dojo/compose

In addition, you can clone this repository and use the Grunt build scripts to manage the package.

Using under TypeScript or ES6 modules, you would generally want to just import the @dojo/compose/compose module:

import compose from '@dojo/compose/compose';

const createFoo = compose({
	foo: 'foo'
}, (instance, options) => {
	/* do some initialization */
});

const foo = createFoo();

How do I contribute?

We appreciate your interest! Please see the Contributing Guidelines and Style Guide.

Installation

To start working with this package, clone the repository and run npm install.

In order to build the project run grunt dev or grunt dist.

Testing

Test cases MUST be written using Intern using the Object test interface and Assert assertion interface.

90% branch coverage MUST be provided for all code submitted to this repository, as reported by Istanbul’s combined coverage results for all supported platforms.

Prior Art and Inspiration

A lot of thinking, talks, publications by Eric Elliott (@ericelliott) inspired @bryanforbes and @kitsonk to take a look at the composition and factory pattern.

@kriszyp helped bring AOP to Dojo 1 and we found a very good fit for those concepts in dojo/compose.

dojo/_base/declare was the starting point for bringing Classes and classical inheritance to Dojo 1 and without @uhop we wouldn't have had Dojo 1's class system.

@pottedmeat and @kitsonk iterated on the original API, trying to figure a way to get types to work well within TypeScript and @maier49 worked with the rest of the dgrid team to make the whole API more usable.

© 2015 - 2017 JS Foundation. New BSD license.

compose's People

Contributors

agubler avatar dylans avatar edhager avatar jdonaghue avatar kitsonk avatar lzhoucs avatar maier49 avatar msssk avatar novemberborn avatar rishson avatar rorticus avatar vansimke avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

compose's Issues

How to properly handle states?

We are in the process of converting a few ES6 classes into ComposeFactory. I ran into the following issue where I couldn't find a proper way to handle instance states.

Suppose I have the following sample class FooBar that I'd like to convert from:

interface Foo {
    getFoo(): string;
}

class FooBar implements Foo {
    protected log: string[];
    private count: number;
    constructor(log: string[]) {
        this.log = log;
        this.count = 0;
    };
    getFoo() {
        this.count++;
        this.log.push(`Have called ${this.count} times at ${new Date()}`);
        return 'foo';
    }
}

When I attempt to to convert it into a ComposeFactory that creates Foo as shown below:

compose<Foo, Object>({
    getFoo(this: Foo) {
        this.count++;
        this.log.push(`Have called ${this.count} times at ${new Date()}`);
        return 'foo';
    }
});

It doesn't compile because count and log are not available in Foo, even though they can be set in a Initializer function. If I try to add them in the object literal:

compose<Foo, Object>({
    count: -1 as number,
    log: null as string[],
    getFoo(this: Foo) {
        this.count++;
        this.log.push(`Have called ${this.count} times at ${new Date()}`);
        return 'foo';
    }
})

I get the expected error Object literal may only specify known properties. There's a few workarounds, but none of them are ideal:

  • Declare them in the interface Foo. This is not ideal since they are internal states of the instance.
  • Remove generic types<Foo, Object>. I lose the benefit of type checking as well. I would like typescript to check to make sure I am implementing the right interface Foo.
  • Use the original ES6 class Foobar as the base. It defeats the purpose of what I am doing - converting ES6 classes.

@maier49 extracted a new state interface along with an external map to maintain the states, in which case the state of the current instance has to be retrieved in every function where it is needed.

Maybe there's a simpler way around.

Import base class interfaces from dojo/interfaces

Items to address:

  • the base classes should be moved from mixins to bases
  • the base classes should import their interfaces from dojo-interfaces
  • these modules should also import other interfaces, like those for Observables and Handles from dojo-interfaces

Support Async Initializers

Enhancement

We could consider supporting async initializers where if the value returned from an initialize function is Thenable that the next initializer.

The only challenge with this is that the instance is returned from the factory sync and therefore if there is an async initializer, additional sync code could run against the instance before the code completes.

Changing compose to return an instance to resolve to a promise is a lot of overhead for what someone could responsibly do within the initializer though, knowing that their code will resolve async and needs to "play nicely".

The other potential solution is that there is a discreet "async" factory for compose which supports async initializers and returns a Promise which resolves with the instance.

Provide Foundational Mixins

We should provide some foundational mixins that might be widely used as part of this package.

I have been working on porting some of those into the feature-mixins branch. The mixins I am thinking of:

  • createDestroyable
  • createEvented
  • createStateful (to do this though, it would require we integrate RxJS Observables into dojo/compose)

Are there any other ones we should have?

We might also then want to consider removing/deprecating any foundational Classes from dojo/core.

Integration of dojo/interfaces

Enhancement

We need to integrate dojo/interfaces into Dojo 2. This is the Epic issue which will own the other issues required to achieve this.

Static Factory Methods

I realised we don't have an API to add static methods to a factory, which makes it difficult to actually specify those when typing factories. I would see something like this:

interface Static {
    <F extends ComposeFactory<T, O>, T, O, S>(factory: F, staticProperties: S): F & S;
}

And integrating that into the compose API like the other methods, and we would be able to do something like this:

interface Foo {
  bar: number;
}

interface FooFactory extends ComposeFactory<Foo, any> {
  foo(): string;
}

const createMy: FooFactory = compose({
    bar: 1
  })
  .static({
    foo() { return 'foo'; }
  });

createFactory.foo();

(also, wouldn't that be our preferred method for overriding the mixin descriptor too? Possibly a more clean way of dealing with other bits we don't want in the prototype)

Types of Composition

In talking about the prototype with @msssk in regards to the dojo/compose prototype, he is suggesting that that versions of composition that he is familiar with vary from the mechanisms we have implemented in the prototype. Specifically, instead of traits being mixed into the base prototype of the resulting class, traits are name spaced on to the class. The advantage is that it means that traits cannot have an dependency on the other and are enforced to be self contained.

For example, current api would be something like this:

const armFactory = compose({ grab() { /* method */ } });
const bodyFactory = compose({ breath() { /* method */ });
const humanFactory = compose(Body).mixin(Arms);

const human = humanFactory();
human.grab();
human.breath();

While what we discussed would be more like this:

const ArmTrait = compose.trait({ grab() { /* method */ } });
const BodyTrait = compose.trait({ breath() { /* method */ });
const humanFactory = compose({
    arm: ArmTrait,
    body: BodyTrait
});

const human = humanFactory();
human.arm.grab();
human.body.breath();

In the second example, clearly arm cannot depend upon body and have to be designed in a way to where only human is aware of what traits it has. What do others think?

Custom toString() is skipped by the factory

Package Version: "2.0.0-beta.13"

Code

interface Foo {
    fooProp: string;
    toString(): string;
}
const fooFactory = compose<Foo, any>({
    fooProp: 'A Foo',
    toString: function(this: Foo) {
        return `[this.fooProp]`;
    }
});
const foo = fooFactory();
assert.strictEqual(foo.toString(), '[A Foo]');

Expected behavior:
The foo.toString() in example code above is expected to return [A Foo], which has been the the case until recently #65 is merged. L107 seems to intentionally skip toString that is defined in the prototype and a toString defined by the library in L58 is used in the factory.

Actual behavior:
foo.toString() returns [object Compose]

Currently a few places in the stores are relying on a custom toString to work properly. I can understand the intention of #65/#63, however I wonder if it makes sense to have the toString (if defined in the prototype) override the default one defined by the library?

compose and prototype.constructor

In conversations with other developers, the question was raised "what happens when you mixin in a method named constructor into a compose class"?

Should we guard against methods named constructor being mixed in and throw?

Complete README

There are several items to be completed in the README:

  • Class Extension
  • Mixing in Traits/State
  • Using Generics
  • Overlaying Functionality

Stateful should self-destroy upon state observation complete

Enhancement

It is logical that if a Stateful instance is observing its state, that if the observation completes (which indicates the underlying state has been removed) that it should attempt to auto-destroy itself.

Also, it should emit a cancellable event of type statecomplete which would allow a listener to disallow this automatic destruction by cancelling the event.

Proposal: Remove `.extend()` and replace with `.override()`

Proposal

We should consider eliminating the .extend() API that is part of dojo-compose and replace it with .override(). .override() would only allow known properties/methods on the base to be replaced and would throw if the overriding prototype contained something that was not part of the base. .override() would also override any mixed array properties. For example:

const createFoo = compose({
        foo: [ 'bar' ]
    })
    .override({
        foo: [ 'foo' ]
    });

const foo = createFoo();
foo.foo; // [ 'foo' ]

Currently, extend would caused these to be "mixed in" resulting in [ 'bar', 'foo' ].

We could also consider throwing on if the the underlying type is changed as well. For example, if the target is a string, but a number is being overridden, we should throw. When TypeScript delivers the partial type operator, we will also be able to guard against this at design time.

Justification

.extend() promotes the concept of traditional inheritance which is not the way we should think about creating classes with dojo-compose. Most of the real world use cases we have for .extend() are actually overriding, and actually adding new functionality or changing the shape of the class is really anti-pattern of how to create a maintainable class.

We are also lacking a mechanism to "overwrite" the array mixin functionality. For example if a default class has an array of functions that provide a level of default behaviour which is not desired, the only option we have at the moment is to add another function which "undoes" what the previous function was doing. By allowing an override, a new behaviour could be accomplished without requiring functionality to "undo" what was done earlier.

RxJS seems to be a peer dependency

dojo/app#3 uses dojo-compose. At the time of writing it only pulls in interfaces from dojo-compose/mixins/createStateful. Without the @reactivex/rxjs dependency installed there are errors in the ts:dev output:

Running "ts:dev" (ts) task
Compiling...
Using tsc v1.8.10
typings/globals/dojo-compose/index.d.ts(348,16): error TS2307: Cannot find module 'node_modules/@reactivex/rxjs/dist/cjs/Rx'.
typings/globals/dojo-compose/index.d.ts(465,11): error TS2305: Module ''rxjs/Rx'' has no exported member 'Observable'.

I think this makes RxJS a peer dependency, not a dev dependency. Which seems undesirable?

Evented and Observable events

Enhancement:

While our Evented events are designed to have an API that mirrors the DOM event API, it causes two major challenges:

  • No concept of backpressure, or pausability of events
  • Events are delivered sync

While more and more of a widgets and other Evented objects have an asynchronous lifecycle, this does make it more difficult to manage this.

We should consider potential solutions to this, for example, utilise ES Observables to deliver events, which would allow a callback to called async serially and allow interesting transformations of the events. For example, the following would work:

const evented = createEvented();
const evented
    .on('foo')
    .forEach((e) => { /* do something with event */ })
    .then(() => { /* do cleanup when the observation is completed (evented destroyed) */ });

The biggest challenge is that there is no concept of backpressure in Observables, which means that any subscriber would need to implement the concept if required (e.g. queue up events until it is ready to process them).

One thought, we could include some helpers that would wrap the subscribers and then providing that backpressure interface.

We should also consider if breaking from the paradigm of "DOM events" would be outweigh the developer familiarity.

Stateful should emit an initialization event

Enhancement

Since the "setup" of an initial state of a Stateful can be complex and occur async, it is not safe to assume instance.state will be correct at initialization. Also, it isn't safe to assume that options.state will be correct either when state is delegated to a store.

Therefore, Stateful shuld emit a single state:initialized that should always occur asynchronously, that can inform a listener that the state has been full "setup" and subsequent state:changed events will then be emitted, until state:completed.

See #79 which is a pre-req due to the name changes for the events.

Allow `id` to be zero for stateful objects

Given the following snippet within createStateful.ts, it prevents the id being 0 as it is a falsy value but is actually a valid id value.

if (id && stateFrom) {
    instance.own(instance.observeState(id, stateFrom));
}
else if (stateFrom) {
    throw new TypeError('When "stateFrom" option is supplied, factory also requires "id" option.');
}

Type inference and conflicts

I don't know if we have worked through all the edge cases with type inference and conflicts... I can't remember what happens at the typescript level with something like this:

const fooFactory = compose({
    foo: 'bar'
});

const fooBarFactory = compose.extend({
    foo: function () {
        console.log('bar');
    }
});

const fooBar = fooBarFactory();

What type is fooBar.foo. From a code perspective, it is last one wins, but what happens with the type? Do we need to do something about that?

Issue with multiple advice being applied

Version and Environment:

Dojo 2 version: beta

Environment(s): all

Code

const createFoo = compose({
    foo(a: string): string {
        return a;
    }
});

const createAspectFoo = createFoo
    .mixin({
        aspectAdvice: {
            after: {
                foo(previousResult: string): string {
                    return previousResult + 'foo';
                }
            }
        }
    });

createAspectFoo
    .mixin({
        aspectAdvice: {
            after: {
                foo(previousResult: string): string {
                    return previousResult + 'bar';
                }
            }
        }
    });

const foo = createAspectFoo();
assert.strictEqual(foo.foo('baz'), 'bazfoo', 'should only apply advice in chain');

Expected behavior:

Only the advice in the constructor function used should be applied.

Actual behavior:

Additional advice downstream is applied upstream. The assert fails with 'bazfoobar' being returned.

Compose Concepts: Bases, Mixins and Final Classes

One of the concepts that has evolved as we have started using dojo-compose is how we should structure code to be the easiest to compose and maintain. To that vein, there are three core concepts of classes that we believe we have identified:

  • Base Classes - These are classes, which are not abstract in nature, but likely are generally not intended to be used directly by an end developer. They provide a base API which is a foundation for things to be built upon. While potentially not intended, they are exposed as compose factories as well as export their instance interface from the module. By convention these should be located in the src/bases path and are named create with the classname appended.
  • Mixin Classes - These are mixin classes, which express modifications to a class prototype, advice applied to methods and initialisation functionality which is intended to be mixed into a base. Mixins can also have the concept of target interfaces that contain APIs that they may depend upon (or apply advice to). By convention these should be locatd in the src/mixins path.
  • Final Classes - These are the classes that are intended to be consumed by a user. They export a final interface for their class and are exposed factories. The

Based on these concepts above, there are likely three pieces of functionality that need to be added to dojo-compose to support these concepts:

  • Mixin Creation API - A more specific API for constructing mixins, that generates the mixin descriptor. Also allowing expression of a target API which has to be present before the mixin can be applied (essentially an abstract interface for the mixin).
  • Better Mixin Logic - Augment the factory creation process so that the factory prototype is created by applying the mixins in order before any mixin advice is applied in order. This would resolve #66 as well and ensure that we don't run into the diamond problem of advice with mixins (or other diamond prototype issues).
  • Enforce Final Classes - This one is debatable, but would ensure that a developer could enforce that the class/factory they are creating is not further compositable. We have been avoiding side-effects by making the factories (and therefore classes) immutable, but would it be desirable to not allow someone to further extend/mixin/augment a class?

I will open issues for these three bits of work as part of this issue.

Ordering of Initialize Functions

When mixin in a factory which has initialize functions and compose already sees that the function is in the initialize array, it will simply ignore it (e.g. not add it again). This causes some surprising behaviour, like having to re-mixin a mixin in order to ensure that it has run before the new initialize function is run. This means that developers have to have explicit knowledge of what mixins contain. We should consider when mixin in that we take all the initialize functions and concat them onto the end of the array but splice out any duplicates further up the stack.

Bubbling and Cancelling of events with Evented

Enhancement

We had discussed if Evented would have built in functionality for bubbling and cancelling (see: dojo/core#78).

We have found use cases where cancelling events is useful (see: createStateful, createCloseableMixin, and createFormFieldMixin).

We also have identified a use case for bubbling where when dealing with widgets asynchronously that emit events, it is difficult to "capture" that instances easily, making it difficult to instrument your code for dealing with errors, where if the event bubbled, in the widget's case, you could quite easily listen on the projector for errors.

I had initially been resistant to over complicate Evented until we had valid use cases, but it is clear that both of those use cases should be supportable in the base class.

Refactor mutations to prototypes

Currently the way compose handles mutating the prototype when constructing the class and mixing in properties is likely not sufficient and needs to be more robust. For example, it is unlikely that enumerability, configurability and accessor properties will be handled as intended.

Also, it should be refactored to re-use as much of core as possible.

Factory versus `new` construction

As we were doing the proposal, it was discussed about doing a full factory implementation or following the constructor function pattern and utilise new for instantiation. I got involved in a side conversation at the request of one of the community and @ericelliott took some time to provide some feedback on why new is limiting and a challenge.

I think it is worth considering adopting a factory function as it does give a level of flexibility to the consumer and actually aligns to some of the patterns we were trying to promote anyways.

Changes to Evented interfaces

dojo-interfaces makes some tweaks to the interfaces of Evented that should be validated in the functional code. This is mostly in the type interfaces, by requiring passing of two generic arguments for an EventedListener.

Additional Diagnostics

* Enhancement*

In complex class compositions, it can be difficult to diagnose how certain methods are resolved and how advice is applied. The getInitFunctionNames() function is useful in providing the initialization function names/ordering, but understanding how the prototype is derived as well could be really useful when certain mixins are overriding behaviour that is already there, since we have "last one wins" resolution order, sometimes it isn't obvious that this can sometimes cause the "diamond" problem.

It might make sense to provide a getComposeMetaData() function that would provide an object which would provide somethings like this:

{
    base: 
        name: 'Base',
        keys:
            {
                foo: 'string',
                bar: 'function'
            },
    },
    mixins: [ { ... }, { ... } ]
    advice: {},
    initFunctions: [ 'initBase', 'mixinA' ]
}

Aspecting Methods Fragile

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().

Debugging Compose

Enhancement

When you have a complex mixin scenario with several init functions, it is difficult at times to figure out exactly what is going on, and therefore difficult to debug compose factories.

We need to come up with a pattern for debugging that allows additional diagnostic information to be available on demand, but also potentially improve the runtime debug information, like stack traces.

Events should not emit on the next turn and `state:initialized` is no longer required

Enhancement

state change events should not emit on the next turn. state:initialized can be removed and reverted back to always emitting state:changed in all cases.

The state:initialized event was implemented to assist with child widget management but this is no longer required with the d widget proposal. Having the event emitted on the next turn causes the render to called twice (in fact n number of times depending on how nested a widgets children are).

Package Version: Latest

Stateful should emit a `state:initialized`

Enhancement

Since the "setup" of an initial state of a Stateful can be complex and occur async, it is not safe to assume instance.state will be correct at initialization. Also, it isn't safe to assume that options.state will be correct either when state is delegated to a store.

Therefore, Stateful shuld emit a single state:initialized that should always occur asynchronously, that can inform a listener that the state has been full "setup" and subsequent state:changed events will then be emitted, until state:completed.

See #79 which is a pre-req due to the name changes for the events.

Using ES6 and TypeScript classes as a base

Currently the create and mixin API can accept ES6 and TypeScript classes. There are a few "challenges" with using these, it can especially get confusing with the type inference.

At the moment, none of the constructor functions are executed. This was mainly to avoid any unintended consequences of the constructors. In addition, you cannot directly call ES6 constructors without the new keyword, meaning that several instances would be created and thrown away during construction.

From a disadvantage point, you do not get any of the affects of the constructor. This is very important though when using non-method properties in TypeScript classes as the type inferrance will abstract those properties from the class, but since then TypeScript moves all of that property setting to the constructor on emit, you don't actually get the values defined. For example, this does not work properly with compose:

import compose from 'dojo-compose/compose';

class Foo {
  foo: string = 'bar';
}

const ComposeFoo = compose(Foo);
const foo = new ComposeFoo();
console.log(foo.bar); // outputs `undefined`

I see two paths forward:

  • Explain this behaviour and why.
  • Change the behaviour, using some sort of intelligence in create which would under ES5 pass the instance to the constructors in the class and in ES6 create new instances for each constructor and then mix in anything that wasn't in the prototype.

Difficulty with generics

This issue has come up before, but I recently ran across it again in working on a prototype for dojo/stores and wanted to see if there are thoughts on what the long term solution or pattern for this should be.

Specifically, in order to use a store, which has generics specifying the type of the underlying data, options for CRUD operations, etc., it's necessary to provide an interface for any individual combination of mixins, in order to be able to specify those generics.
For example:

interface ObservableStoreFactory extends ComposeFactory<ObservableStore<{}, {}, any>, ObservableStoreOptions<{}, {}>> {
    <T, O extends CrudOptions, U extends UpdateResults<T>>(options?: ObservableStoreOptions<T, O>): ObservableStore<T, O, U>;
}
const createObservableStore: ObservableStoreFactory = createStore
    .mixin(createObservableStoreMixin());

When working on a prototype to determine the potential needs of dgrid2 with respect to compose, I came up with this pattern, which is ugly internally, but hid the need for an interface like this from the end user of the library.

/**
 * The BaseGridFactory and GridFactory methods provide the ability to add mixins to and extend the base Grid factory
 * without losing the ability to use generics.
 */
export interface BaseGridFactory<O> {
    <P>(options?: GridOptions<P>): Grid<P>;
    mixin<U, P>(mixin: ComposeMixinable<U, P>): GridFactory<U, O & P>;
    mixin<U, P>(mixin: ComposeMixinDescriptor<Grid<any>, O, U, P>): GridFactory<U, O & P>;
}

export interface GridFactory<T, O> extends ComposeFactory<T, O> {
    <P>(options?: GridOptions<P>): Grid<P> & T;
    mixin<U, P>(mixin: ComposeMixinable<U, P>): GridFactory<T & U, O & P>;
    mixin<U, P>(mixin: ComposeMixinDescriptor<T, O, U, P>): GridFactory<T & U, O & P>;
}

Unfortunately, this pattern doesn't work for the store use case. It relies on the fact that the Grid interface in this case stays the same, even as mixins are added to it to alter or extend its functionality, but for the store the API is also modified(generally) by the mixins that are added to it.

Changes to Stateful interfaces

There are some changes to the Stateful interface in the dojo-interfaces specfication. Both the state:changed and state:completed event types have been renamed. This should be reflected in the implementation of Stateful.

Improve test coverage

The initial tests not sufficiently covering the code. Additional test coverage should be added.

Allow deep mixin of prototype

Enhancement

Related to comments in dojo/widgets#29, it might make sense to have a feature that allows one level deep mixin of properties. While I am not sure of how this fits the sort of idiomatic patterns that exist, but I can see a utility there.

The current challenge is that if we want to modify the behaviour of methods, the only easy pattern we have is to aspect the method and apply some form of advice to it. In a true mixin situation, it becomes difficult to keep track of what advice is being applied and often requires external knowledge of the mixin, causing fairly hard dependencies. For example the use case of collecting attributes for a virtual node when rendering a widget. We currently have getNodeAttributes(). Currently there are two choices to modify this behaviour:

  1. Overwrite and re-implement the functionality - This is a sort of sledge hammer, obscuring any other sort functionality that may or may not be implemented in another mixin and you can't be certain other mixins won't do the same thing.
  2. Aspect with before/after/around - This means you have to assume that the method is there and while it sort of decouples other dependencies, and advice can easily be layered without explicit knowledge of the other advice, it can be surprising in how the advice is actually applied.

An alternative could be a reduction of methods though, where any mixin could add additional methods to an array and the core mixin that provides the getNodeAttributes() method, would reduce those methods to the final set of node attributes. But in order to support this, we would need compose to provide a mechanism for expressing those methods to be concatenated for each mixin, resulting in a prototype that contains the final array to be reduced.

My current thinking is that during the compose process, anything that is an Array in the destination prototype and the prototype to be mixed in, would perform a dest.array.concat(source.array) and then during instantiation and construction of the prototype, instance.array = proto.array.slice(0) before running the initialize functions.

Clearly, this idea is open for debate and certainly "complicates" compose by adding some features, but we are encountering challenges elsewhere, which I still don't think are best served by something like C3MRO or super() and classical inheritance. This might be a pattern established somewhere else, or we might be inventing something "new", but I would rather find a practical solution, irrespective of it being an established pattern.

Discrete .initialize API

A common use case with compose is the need to simply change the initialization functionality without changing the prototype of the factory. We should consider a discrete API for initialization that allows the addition of an initialize function.

Should we also consider some sort of explicit ordering? For example, allow initialization functions to be inserted "first" or "last" or "before mixin" or "after mixin"?

Terse API for Mixin

The review of dojo/compose by the SitePen/dgrid (@maier49, @kfranqueiro, and @edhager) as highlighted that the mixin API should/could be significantly more terse, especially when dealing with complex type compositing. Ideally the API would work more like this:

const fooFactory = compose({
    mixins: [ aFactory, BConstructorFunction, CClass, dObjectLiteral ],
    aspect: {
        before: { 'foo': aspectBeforeFunction },
        around: { 'bar': aspectAroundFunction },
        after: { 'baz': aspectAfterFunction }
    },
    init: initFunction
});

const foo = foo({ foo: 'bar' });

I expect the interface would need to be something like this:

interface ComposeAspectDefinition {
    before?: { [ method: string ]: AspectBeforeMethod };
    around?: { [ method: string ]: AspectAroundMethod };
    after?: { [ method: string ]: AspectAfterMethod };
}

interface ComposeBaseFactoryDefinition {
    aspect?: AspectDefinition;
    init?: InitFunction | InitFunction[];
}

interface ComposeSingleFactoryDefinition<A extends Object, O> extends ComposeBaseFactoryDefinition {
    mixins: ComposeFactory<A, O> | GenericClass<A, O> | A | [ComposeFactory<A, O> | GenericClass<A, O> | A];
}


interface ComposeTwoFactoryDefinition<A extends Object, B extends Object, O, P> extends ComposeBaseFactoryDefinition {
    mixins: [ComposeFactory<A, O> | GenericClass<A, O> | A, ComposeFactory<B, P> | GenericClass<B, P> | B];
}
// etc...

interface Compose {
    <A extends Object, O>(ComposeSingleFactoryDefinition<A, O>): ComposeFactory<A, O>;
    <A extends Object, B extends Object, O, P>(ComposeTwoFactoryDefinition<A, B, O, P>): ComposeFactory<A & B, O & P>;
    // etc...
}

Freezing of the factory

Originally, I thought it was wise to freeze the returned factory, basically to keep factories immutable, and limit the amount unanticipated consequences. The challenge is that it makes it impossible to add static methods/properties to the factory.

There are three approaches I can see to resolving this:

  • Don't recommend/support static properties or methods. If the default export of a module is a factory, it is "better" from a potential code optimisations mechanism, to export static methods related to the factory as part of the module. Of course, this may not gracefully handle typing with downstream composites.
  • Don't freeze the factories
  • Add an API to allowing the addition of static properties. The challenge will how to handle the type inference to return an extension of the ComposeFactory. Likely we would need TS 1.8 with F bound polymorphism (see: microsoft/TypeScript#5949).

If we choose either of the last two, we will have to change the factory cloning logic to perpetuate these static method/properties, which it currently does not do.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.