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

Simplify Widget Registries #686

Merged
merged 15 commits into from
Sep 20, 2017
Merged

Simplify Widget Registries #686

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e87219c
first set of changes to only support base and local registry
agubler Sep 18, 2017
7b9b304
update registries to registry
agubler Sep 18, 2017
bd8de6b
move logic into projector
agubler Sep 18, 2017
d8137f6
improve get logic in handler
agubler Sep 18, 2017
cf0c527
try and make registry handler a little cleaner
agubler Sep 19, 2017
5dd4cfd
Add defineInjector to registry handler
agubler Sep 19, 2017
6e54089
Unit tests for registry handler
agubler Sep 19, 2017
e672728
Unit tests for projector
agubler Sep 19, 2017
e407fa8
refactor to use tuple for registries and remove destory management logic
agubler Sep 19, 2017
843ab96
emit the item and check before invalidating
agubler Sep 19, 2017
7e6f5d7
tests for for the extra invalidates
agubler Sep 19, 2017
b7356ee
util _get for both get and getHandler
agubler Sep 19, 2017
c8b8f71
do not default projector properties
agubler Sep 19, 2017
1ea5af1
use as for cast
agubler Sep 20, 2017
8bd5fbb
Add generic for getInjector and correct define type
agubler Sep 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/Registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const WIDGET_BASE_TYPE = Symbol('Widget Base');

export interface RegistryEventObject extends EventObject {
action: string;
item: WidgetBaseConstructor | Injector;
}

export interface RegistryListener {
Expand Down Expand Up @@ -108,10 +109,11 @@ export class Registry extends Evented implements RegistryInterface {
/**
* Emit loaded event for registry label
*/
private emitLoadedEvent(widgetLabel: RegistryLabel): void {
private emitLoadedEvent(widgetLabel: RegistryLabel, item: WidgetBaseConstructorFunction | WidgetBaseConstructor | Injector): void {
this.emit({
type: widgetLabel,
action: 'loaded'
action: 'loaded',
item
});
}

Expand All @@ -129,14 +131,14 @@ export class Registry extends Evented implements RegistryInterface {
if (item instanceof Promise) {
item.then((widgetCtor) => {
this._widgetRegistry.set(label, widgetCtor);
this.emitLoadedEvent(label);
this.emitLoadedEvent(label, widgetCtor);
return widgetCtor;
}, (error) => {
throw error;
});
}
else {
this.emitLoadedEvent(label);
this.emitLoadedEvent(label, item);
}
}

Expand All @@ -150,6 +152,7 @@ export class Registry extends Evented implements RegistryInterface {
}

this._injectorRegistry.set(label, item);
this.emitLoadedEvent(label, item);
}

public get<T extends WidgetBaseInterface = WidgetBaseInterface>(label: RegistryLabel): Constructor<T> | null {
Expand All @@ -172,7 +175,7 @@ export class Registry extends Evented implements RegistryInterface {

promise.then((widgetCtor) => {
this._widgetRegistry.set(label, widgetCtor);
this.emitLoadedEvent(label);
this.emitLoadedEvent(label, widgetCtor);
return widgetCtor;
}, (error) => {
throw error;
Expand Down
95 changes: 45 additions & 50 deletions src/RegistryHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,79 +1,74 @@
import { Map } from '@dojo/shim/Map';
import { Evented } from '@dojo/core/Evented';
import { Constructor, RegistryLabel, WidgetBaseInterface } from './interfaces';
import { Registry, RegistryEventObject } from './Registry';
import { Injector } from './Injector';

export default class RegistryHandler extends Evented {
private _registries: { handle?: any, registry: Registry }[] = [];
export class RegistryHandler extends Evented {
private _registry = new Registry();
private _baseRegistry: Registry;
private _registryWidgetLabelMap: Map<Registry, RegistryLabel[]> = new Map();
private _registryInjectorLabelMap: Map<Registry, RegistryLabel[]> = new Map();

public add(registry: Registry, isDefault: boolean = false) {
if (isDefault) {
this._registries.push({ registry });
}
else {
this._registries.unshift({ registry });
}
constructor() {
super();
this.own(this._registry);
}

public remove(registry: Registry): boolean {
return this._registries.some((registryWrapper, i) => {
if (registryWrapper.registry === registry) {
registry.destroy();
this._registries.splice(i, 1);
return true;
}
return false;
});
public set base(baseRegistry: Registry) {
this._registryWidgetLabelMap.delete(this._baseRegistry);
this._registryInjectorLabelMap.delete(this._baseRegistry);
this._baseRegistry = baseRegistry;
}

public replace(original: Registry, replacement: Registry): boolean {
return this._registries.some((registryWrapper, i) => {
if (registryWrapper.registry === original) {
original.destroy();
registryWrapper.registry = replacement;
return true;
}
return false;
});
public define(label: RegistryLabel, widget: Constructor<WidgetBaseInterface>): void {
this._registry.define(label, widget);
}

public get defaultRegistry(): Registry | undefined {
if (this._registries.length) {
return this._registries[this._registries.length - 1].registry;
}
public defineInjector(label: RegistryLabel, injector: Injector): void {
this._registry.defineInjector(label, injector);
}

public has(widgetLabel: RegistryLabel): boolean {
return this._registries.some((registryWrapper) => {
return registryWrapper.registry.has(widgetLabel);
});
public has(label: RegistryLabel): boolean {
return this._registry.has(label) || this._baseRegistry.has(label);
}

public getInjector<T extends Injector>(label: RegistryLabel): T | null {
for (let i = 0; i < this._registries.length; i++) {
const registryWrapper = this._registries[i];
return registryWrapper.registry.getInjector<T>(label);
}
return null;
public hasInjector(label: RegistryLabel): boolean {
return this._registry.hasInjector(label) || this._baseRegistry.hasInjector(label);
}

public get<T extends WidgetBaseInterface = WidgetBaseInterface>(widgetLabel: RegistryLabel): Constructor<T> | null {
for (let i = 0; i < this._registries.length; i++) {
const registryWrapper = this._registries[i];
const item = registryWrapper.registry.get<T>(widgetLabel);
public get<T extends WidgetBaseInterface = WidgetBaseInterface>(label: RegistryLabel, globalPrecedence: boolean = false): Constructor<T> | null {
return this._get(label, globalPrecedence, 'get', this._registryWidgetLabelMap);
}

public getInjector(label: RegistryLabel, globalPrecedence: boolean = false): Injector | null {
return this._get(label, globalPrecedence, 'getInjector', this._registryInjectorLabelMap);
}

private _get(label: RegistryLabel, globalPrecedence: boolean, getFunctionName: 'getInjector' | 'get', labelMap: Map<Registry, RegistryLabel[]>): any {
const registries = globalPrecedence ? [ this._baseRegistry, this._registry ] : [ this._registry, this._baseRegistry ];
for (let i = 0; i < registries.length; i++) {
const registry: any = registries[i];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No other decent type than any?

Copy link
Member Author

@agubler agubler Sep 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's so we can use an index using the string function name that is passed.

If the getFunctionName (which typing I realise is a bad name), is restricted to just 'get' or 'getInjector' then using registry[getFunctionName]('') works but not if is it's typed as 'get' | 'getInjector', any ideas?

private _get(label: RegistryLabel, globalPrecedence: boolean, getFunctionName: 'getInjector' | 'get', labelMap: Map<Registry, RegistryLabel[]>): any {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... Recent versions of TypeScript allow you to access properties by method name just fine, if the variable is a string literal, unless I am missing something here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was fine is I typed the argument is just 'get' or just 'getInjector' but not when I typed it as 'getInjector' | 'get'...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, the playground example types it as 'getInjector' | 'get'... wonder what is wrong.

if (!registry) {
continue;
}
const item = registry[getFunctionName](label);
const registeredLabels = labelMap.get(registry) || [];
if (item) {
return item;
}
else if (!registryWrapper.handle) {
registryWrapper.handle = registryWrapper.registry.on(widgetLabel, (event: RegistryEventObject) => {
if (event.action === 'loaded') {
else if (registeredLabels.indexOf(label) === -1) {
const handle = registry.on(label, (event: RegistryEventObject) => {
if (event.action === 'loaded' && (<any> this)[getFunctionName](label) === event.item) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(this as any) and why the any cast?

this.emit({ type: 'invalidate' });
registryWrapper.handle.destroy();
registryWrapper.handle = undefined;
}
});
this.own(handle);
labelMap.set(registry, [ ...registeredLabels, label]);
}
}
return null;
}
}

export default RegistryHandler;
91 changes: 16 additions & 75 deletions src/WidgetBase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
} from './interfaces';
import RegistryHandler from './RegistryHandler';
import NodeHandler from './NodeHandler';
import { isWidgetBaseConstructor, WIDGET_BASE_TYPE, Registry } from './Registry';
import { isWidgetBaseConstructor, WIDGET_BASE_TYPE } from './Registry';

/**
* Widget cache wrapper for instance management
Expand Down Expand Up @@ -167,7 +167,7 @@ export class WidgetBase<P = WidgetProperties, C extends DNode = DNode> extends E
*/
private _decoratorCache: Map<string, any[]>;

private _registries: RegistryHandler;
private _registry: RegistryHandler;

/**
* Map of functions properties for the bound function
Expand All @@ -182,8 +182,6 @@ export class WidgetBase<P = WidgetProperties, C extends DNode = DNode> extends E

private _boundInvalidate: () => void;

private _defaultRegistry = new Registry();

private _nodeHandler: NodeHandler;

private _projectorAttachEvent: Handle;
Expand All @@ -206,15 +204,14 @@ export class WidgetBase<P = WidgetProperties, C extends DNode = DNode> extends E
this._cachedChildrenMap = new Map<string | Promise<WidgetBaseConstructor> | WidgetBaseConstructor, WidgetCacheWrapper[]>();
this._diffPropertyFunctionMap = new Map<string, string>();
this._bindFunctionPropertyMap = new WeakMap<(...args: any[]) => any, { boundFunc: (...args: any[]) => any, scope: any }>();
this._registries = new RegistryHandler();
this._registry = new RegistryHandler();
this._nodeHandler = new NodeHandler();
this._registries.add(this._defaultRegistry, true);
this.own(this._registries);
this.own(this._registry);
this.own(this._nodeHandler);
this._boundRenderFunc = this.render.bind(this);
this._boundInvalidate = this.invalidate.bind(this);

this.own(this._registries.on('invalidate', this._boundInvalidate));
this.own(this._registry.on('invalidate', this._boundInvalidate));
this._checkOnElementUsage();
}

Expand Down Expand Up @@ -309,50 +306,12 @@ export class WidgetBase<P = WidgetProperties, C extends DNode = DNode> extends E
return this._properties;
}

protected setRegistry(previousRegistry: Registry | undefined, newRegistry: Registry | undefined): void {
const { _registries, _defaultRegistry } = this;

if (_registries.defaultRegistry === _defaultRegistry && newRegistry) {
_registries.remove(_defaultRegistry);
}

if (previousRegistry && newRegistry) {
_registries.replace(previousRegistry, newRegistry);
}
else if (newRegistry) {
_registries.add(newRegistry);
}
else if (previousRegistry) {
_registries.remove(previousRegistry);
}
}

protected setDefaultRegistry(previousRegistry: Registry | undefined, newRegistry: Registry | undefined): void {
const { _registries, _defaultRegistry } = this;
if (newRegistry === undefined && previousRegistry) {
_registries.remove(previousRegistry);
if (_registries.defaultRegistry === undefined) {
_registries.add(_defaultRegistry);
}
}
else if (newRegistry) {
const result = _registries.replace(previousRegistry || _defaultRegistry, newRegistry);
if (!result) {
_registries.add(newRegistry, true);
}
}
}

public __setCoreProperties__(coreProperties: CoreProperties) {
public __setCoreProperties__(coreProperties: CoreProperties): void {
this._renderState = WidgetRenderState.PROPERTIES;
const { registry, defaultRegistry } = coreProperties;
const { baseRegistry } = coreProperties;

if (this._coreProperties.registry !== registry) {
this.setRegistry(this._coreProperties.registry, registry);
this.invalidate();
}
if (this._coreProperties.defaultRegistry !== defaultRegistry) {
this.setDefaultRegistry(this._coreProperties.defaultRegistry, defaultRegistry);
if (this._coreProperties.baseRegistry !== baseRegistry) {
this._registry.base = baseRegistry;
this.invalidate();
}
this._coreProperties = coreProperties;
Expand Down Expand Up @@ -479,34 +438,16 @@ export class WidgetBase<P = WidgetProperties, C extends DNode = DNode> extends E
}
}
else {
const { defaultRegistry, registry } = this.__getCoreProperties__(node.properties);
node.coreProperties = node.coreProperties || {};

if (node.coreProperties.bind === undefined) {
node.coreProperties.bind = this;
}
node.coreProperties.defaultRegistry = defaultRegistry;
node.coreProperties.registry = registry;
node.coreProperties = {
bind: this,
baseRegistry: this._coreProperties.baseRegistry
};
}
nodes = [ ...nodes, ...node.children ];
}
}
}

protected __getCoreProperties__(properties: any): CoreProperties {
const {
registry
} = properties;
const {
defaultRegistry = registry || this._registries.defaultRegistry
} = this._coreProperties;

return {
registry,
defaultRegistry
};
}

protected invalidate(): void {
if (this._renderState === WidgetRenderState.IDLE) {
this._dirty = true;
Expand Down Expand Up @@ -644,8 +585,8 @@ export class WidgetBase<P = WidgetProperties, C extends DNode = DNode> extends E
return property;
}

public get registries(): RegistryHandler {
return this._registries;
public get registry(): RegistryHandler {
return this._registry;
}

private _runBeforeProperties(properties: any) {
Expand Down Expand Up @@ -719,7 +660,7 @@ export class WidgetBase<P = WidgetProperties, C extends DNode = DNode> extends E
let child: WidgetBaseInterface<WidgetProperties>;

if (!isWidgetBaseConstructor(widgetConstructor)) {
const item = this._registries.get(widgetConstructor);
const item = this._registry.get(widgetConstructor);
if (item === null) {
return null;
}
Expand Down
3 changes: 1 addition & 2 deletions src/d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ export function w<W extends WidgetBaseInterface>(widgetConstructor: Constructor<
children,
widgetConstructor,
properties,
type: WNODE,
coreProperties: {}
type: WNODE
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/decorators/inject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export interface InjectConfig {
export function inject({ name, getProperties }: InjectConfig) {
return handleDecorator((target, propertyKey) => {
beforeProperties(function(this: WidgetBase, properties: any) {
const injector = this.registries.getInjector(name);
const injector = this.registry.getInjector(name);
if (injector) {
const registeredInjectors = registeredInjectorsMap.get(this) || [];
if (registeredInjectors.length === 0) {
Expand Down
Loading