Typescript 2.2 mixin conversion part 2

[maier49]

Type: feature

Description:

Converts the rest of the code base to use TypeScript 2.2 instead of dojo/compose

Related Issue: #108

Please review this checklist before submitting your PR:

• There is a related issue
• All contributors have signed a CLA
• All code matches the style guide
• The code passes the CI tests
• Unit or Functional tests are included in the PR
• The PR increases or maintains the overall unit test coverage percentage
• The code is ready to be merged

[codecov-io]

Codecov Report

Merging #110 into master will decrease coverage by -0.22%.
The diff coverage is 99.52%.

@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
- Coverage     100%   99.78%   -0.22%     
==========================================
  Files          17       16       -1     
  Lines        1393     1368      -25     
  Branches      253      251       -2     
==========================================
- Hits         1393     1365      -28     
- Misses          0        1       +1     
- Partials        0        2       +2

Impacted Files	Coverage Δ
src/store/createStoreObservable.ts	100% <ø> (ø)	✅
src/query/createSort.ts	100% <ø> (ø)	✅
src/query/CompoundQuery.ts	100% <ø> (ø)	✅
src/query/createFilter.ts	100% <ø> (ø)	✅
src/patch/Patch.ts	100% <ø> (ø)	✅
src/storage/InMemoryStorage.ts	100% <ø> (ø)	✅
src/query/createStoreRange.ts	100% <ø> (ø)	✅
src/store/QueryableStore.ts	100% <100%> (ø)
src/store/materialize.ts	100% <100%> (ø)	✅
src/storage/IndexedDBStorage.ts	100% <100%> (ø)	✅
... and 7 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1f2fef...97c3271. Read the comment docs.

[agubler]

I think this will get hit by the issue that incorrect code is emitted in the d.ts, you'll need to declare the mixed type in a const then use it in for extends. Something like this:

export const DefaultObservableStoreBase = ObservableStoreMixin<T, CrudOptions, UpdateResults<T>, typeof StoreBase>(StoreBase)

export class DefaultObservableStore<T> extends DefaultObservableStoreBase<T, ObservableStoreMixinOptions> { }

[maier49]

I think this is going to be problematic.

type Constructor<T> = new (...args: any[]) => T;

class GenericClass<T> {
}

interface GenericTrait<T> {
	newGenericValue: T;
}

function MixinGeneric<T, C extends Constructor<GenericClass<T>>>(base: C): C & Constructor<GenericTrait<T>> {
	return class extends base {
		newGenericValue: T;
		constructor(...args: any[]) {
			super(...args);
		}
	};
}

class Child<T> extends GenericClass<T> {
	child: T;
	constructor(options: { childValue: string }) {
		super();
	}
}

// const ClassyMixinBase = MixinGeneric(Child);
// class ClassyMixin<T> extends ClassyMixinBase<T> {}
class ClassyMixin<T> extends MixinGeneric<T, typeof Child>(Child)<T> {}

// maintains signature
let classMixin: Child<string> & GenericTrait<string> = new ClassyMixin<string>({ childValue: 'string' });

// is a number
classMixin.child = 'abc';

// is not a number (is string)
classMixin.newGenericValue = 'abc';

This was the approach that we had to take to get the generic being mixed in to have the same type as the generic on the base class. If you comment out the existing class declaration and uncomment the workaround approach, this fails to compile, because the generic for the mixin defaults to {}.

Passing any as the first generic to MixinGeneric fixes it, but then if classMixin is not explicitly typed classMixin.newGenericValue just becomes any.

[maier49]

This has now been updated to work around this by providing the Queryable store and the Observable store as classes instead of mixins.

[agubler]

lots of code! but generally looks good. Could do with a follow up PR to add _ on private variables, clean up any formatting inconsistencies and check the visibility of class methods.