From b8daaceacec9a5ff90111a8da1369e86ef436bcd Mon Sep 17 00:00:00 2001 From: Tom Dawes Date: Thu, 3 Oct 2019 16:03:45 +0100 Subject: [PATCH 1/6] Connect as a functional component --- packages/core/src/connect.tsx | 335 +++++++++++++++------------------- packages/core/src/types.ts | 2 +- packages/core/src/watch.ts | 6 +- 3 files changed, 150 insertions(+), 193 deletions(-) diff --git a/packages/core/src/connect.tsx b/packages/core/src/connect.tsx index 91f8562a..0297a79c 100644 --- a/packages/core/src/connect.tsx +++ b/packages/core/src/connect.tsx @@ -1,8 +1,11 @@ import * as React from "react"; import logger from "./logger"; -import { Comp, Connect, Dispatch, Node, Store, Watch } from "./types"; +import { Comp, Connect, Node, Store } from "./types"; import { joinPath, splitPath } from "./utils"; -import { subscribe, unsubscribe } from "./watch"; +import { + subscribe as subscribeInner, + unsubscribe as unsubscribeInner, +} from "./watch"; export const ProdoContext = React.createContext>(null as any); @@ -20,6 +23,15 @@ export const createUniverseWatcher = (universePath: string) => { return readProxy([universePath]); }; +const state = createUniverseWatcher("state"); + +const useForceUpdate = () => { + const [, setCounter] = React.useState(0); + return React.useCallback(() => { + setCounter(tick => tick + 1); + }, []); +}; + const getValue = (path: string[], obj: any): any => path.reduce((x: any, y: any) => x && x[y], obj); @@ -75,203 +87,148 @@ export type Func = (viewCtx: V) => React.ComponentType

; export const connect: Connect =

( func: Func, - name: string = "(anonymous)", -): React.ComponentType

=> - class ConnectComponent

extends React.Component { - public static contextType = ProdoContext; - - public state: any; - private watched: { [key: string]: any }; - private prevWatched: { [key: string]: any }; - private pathNodes: { [key: string]: Node }; - private compId: number; - private comp: Comp; - private name: string; - private eventIdCnt: number; - private subscribe: ( - path: string[], - unsubscribe?: (comp: Comp) => void, - ) => void; - private unsubscribe: (path: string[]) => void; - private firstTime: boolean; - private status: { unmounted: boolean }; - private store: Store; - private _renderFunc: any; - private _watch: Watch; - private _dispatch: Dispatch; - private _state: any; - - private _viewCtx: any; - - constructor(props: P) { - super(props); - - this.state = {}; - this.watched = {}; - this.prevWatched = {}; - this.pathNodes = {}; - this.firstTime = true; - this.compId = _compIdCnt++; - this.name = name + "." + this.compId; - this.store = this.context; - - const setState = this.setState.bind(this); - this.status = { unmounted: false }; - - this.comp = { - name: this.name, - compId: this.compId, - }; - - this.eventIdCnt = 0; - - this.subscribe = (path: string[], unsubscribe?: (comp: Comp) => void) => { - const pathKey = joinPath(path); - - const node: Node = this.pathNodes[pathKey] || { - pathKey, - status: this.status, - setState, - unsubscribe, - ...this.comp, - }; - - this.pathNodes[pathKey] = node; - subscribe(this.store, path, node); - }; - - this.unsubscribe = (path: string[]) => { - const pathKey = joinPath(path); - const node = this.pathNodes[pathKey]; - if (node != null) { - unsubscribe(this.store, path, node); - delete this.pathNodes[pathKey]; - } - }; - - this._watch = x => x; - - this._dispatch = func => (...args) => - this.store.exec( - { - id: `${this.comp.name}/event.${this.eventIdCnt++}`, - parentId: this.comp.name, - }, - func, - ...args, - ); - - this._renderFunc = (props: any): any => { - return (func as ((args: any) => (props: any) => any))(this._viewCtx)( - props, - ); - }; - - logger.info(`[constructing] ${this.name}`); - } - - public componentDidMount() { - logger.info(`[did mount] ${this.name}`); - - Object.keys(this.watched).forEach(pathKey => { - logger.info(`[start watching] ${this.name}: < ${pathKey} >`); - this.subscribe(splitPath(pathKey)); + baseName: string = "(anonymous)", +): React.ComponentType

=> (props: P) => { + const compId = React.useRef(_compIdCnt++); + const name = baseName + "." + compId.current; + + logger.info(`[rendering] ${name}`); + // First render only + React.useMemo(() => logger.info(`[constructing] ${name}`), []); + + const store = React.useContext(ProdoContext); + const forceUpdate = useForceUpdate(); + + // Subscribing to part of the state + const status = React.useRef({ unmounted: true }); + React.useEffect(() => { + logger.info(`[did mount] ${name}`); + status.current.unmounted = false; + logger.debug("store", store); + }, []); + + const pathNodes = React.useRef({}); + const subscribe = (path: string[], unsubscribe?: (comp: Comp) => void) => { + const pathKey = joinPath(path); + + const node: Node = + pathNodes.current[pathKey] || + (pathNodes.current[pathKey] = { + pathKey, + status: status.current, + forceUpdate, + unsubscribe, + name, + compId: compId.current, }); - logger.debug("store", this.store); - - this.prevWatched = { ...this.watched }; - this.firstTime = true; - this.setState(this.watched); - this.watched = {}; - } - - public shouldComponentUpdate(nextProps: P, nextState: any) { - const test = - !shallowEqual(this.props, nextProps) || - (!this.firstTime && !shallowEqual(this.state, nextState)); - - logger.info(`[should update] ${this.name}`, test); - this.firstTime = false; - - return test; + subscribeInner(store, path, node); + }; + const unsubscribe = (path: string[]) => { + const pathKey = joinPath(path); + const node = pathNodes.current[pathKey]; + if (node != null) { + unsubscribeInner(store, path, node); + delete pathNodes.current[pathKey]; } + }; - public componentDidUpdate() { - logger.info(`[did update] ${this.name}`); - - Object.keys(this.watched).forEach(pathKey => { - const keyExisted = this.prevWatched.hasOwnProperty(pathKey); - if (!keyExisted) { - logger.info(`[update] ${this.name}: now watching < ${pathKey} >`); - this.subscribe(splitPath(pathKey)); - } - }); + const eventIdCnt = React.useRef(0); + const dispatch = func => (...args) => + store.exec( + { + id: `${name}/event.${eventIdCnt.current++}`, + parentId: name, + }, + func, + ...args, + ); - Object.keys(this.prevWatched).forEach(pathKey => { - const keyDeleted = !this.watched.hasOwnProperty(pathKey); - if (keyDeleted) { - logger.info(`[update] ${this.name}: stop watching < ${pathKey} >`); - this.unsubscribe(splitPath(pathKey)); - } + const watched = React.useRef({}); + const prevWatched = React.useRef({}); + + // On update, update subscriptions + React.useEffect(() => { + logger.info(`[updating watch] ${name}`); + + Object.keys(watched.current).forEach(pathKey => { + const keyExisted = prevWatched.current.hasOwnProperty(pathKey); + if (!keyExisted) { + logger.info(`[update] ${name}: now watching < ${pathKey} >`); + subscribe(splitPath(pathKey)); + } + }); + + Object.keys(prevWatched).forEach(pathKey => { + const keyDeleted = watched.current.hasOwnProperty(pathKey); + if (keyDeleted) { + logger.info(`[update] ${name}: stop watching < ${pathKey} >`); + unsubscribe(splitPath(pathKey)); + } + }); + + prevWatched.current = { ...watched.current }; + watched.current = {}; + }); + + // On unmount, unsubscribe from everything + React.useEffect(() => { + return () => { + logger.info(`[unmounting]: ${name}`, watched.current); + + Object.keys(prevWatched.current).forEach(pathKey => { + logger.info(`[unmount] ${name}: stop watching < ${pathKey} >`); + unsubscribe(splitPath(pathKey)); }); + logger.debug("store", store); + status.current = { unmounted: true }; + }; + }, []); - this.prevWatched = { ...this.watched }; - this.watched = {}; - } - - public componentWillUnmount() { - logger.info(`[will unmount]: ${this.name}`, this.state); - Object.keys(this.state).forEach(pathKey => { - logger.info(`[unmount] ${this.name}: stop watching < ${pathKey} >`); - this.unsubscribe(splitPath(pathKey)); - }); + const watch = valueExtractor(store, watched.current); - logger.debug("store", this.store); + const _subscribe = (path: string[], unsubscribe?: () => void): void => { + const pathKey = joinPath(path); + watched.current[pathKey] = getValue(path, store.universe); + subscribe(path, unsubscribe); + }; - this.status.unmounted = true; - } + const ctx = React.useMemo(() => { + const ctx = { + dispatch, + state, + watch, + subscribe: _subscribe, + }; + store.plugins.forEach(p => { + if (p._internals.viewCtx) { + ctx.dispatch = dispatch; + p._internals.viewCtx( + { + ctx, + universe: store.universe, + comp: { + name, + compId: compId.current, + }, + }, + store.config, + ); + } + }); + return ctx; + }, [store.universe]); - public render() { - this.createViewCtx(); + return (func as ((args: any) => (props: any) => any))(ctx)(props); +}; - const Comp = this._renderFunc; - return ; - } +// public shouldComponentUpdate(nextProps: P, nextState: any) { +// const test = +// !shallowEqual(this.props, nextProps) || +// (!this.firstTime && !shallowEqual(this.state, nextState)); - private createViewCtx() { - this.store = this.context; - this._state = createUniverseWatcher("state"); - this._watch = valueExtractor(this.store, this.watched); - - const subscribe = (path: string[], unsubscribe?: () => void): void => { - const pathKey = joinPath(path); - this.watched[pathKey] = getValue(path, this.store.universe); - this.subscribe(path, unsubscribe); - }; - - const ctx = { - dispatch: this._dispatch, - state: this._state, - watch: this._watch, - subscribe, - }; - - this.store.plugins.forEach(p => { - if (p._internals.viewCtx) { - (ctx as any).dispatch = this._dispatch; - - p._internals.viewCtx( - { - ctx, - universe: this.store.universe, - comp: this.comp, - }, - this.store.config, - ); - } - }); +// logger.info(`[should update] ${this.name}`, test); +// this.firstTime = false; - this._viewCtx = ctx; - } - }; +// return test; +// } diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 56063beb..e8589d55 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -72,7 +72,7 @@ export interface Comp { export interface Node extends Comp { pathKey: string; status: { unmounted: boolean }; - setState: (state: any) => void; + forceUpdate: () => void; unsubscribe?: (comp: Comp) => void; } diff --git a/packages/core/src/watch.ts b/packages/core/src/watch.ts index d0a8709b..d8f4f33b 100644 --- a/packages/core/src/watch.ts +++ b/packages/core/src/watch.ts @@ -108,7 +108,7 @@ export const submitPatches = ( if (!comps[x.compId]) { compIds.push(x.compId); comps[x.compId] = { - setState: x.setState, + forceUpdate: x.forceUpdate, status: x.status, name: x.name, newValues: {}, @@ -125,11 +125,11 @@ export const submitPatches = ( event.rerender = {}; Object.keys(comps).forEach(compId => { - const { setState, name, newValues, status } = comps[compId]; + const { forceUpdate, name, newValues, status } = comps[compId]; if (!status.unmounted) { event.rerender![comps[compId].name] = true; logger.info(`[upcoming state update] ${name}`, newValues, status); - setState(newValues); + forceUpdate(); } }); }; From 2d3e2962aed607f2406de759699784d1382a92f3 Mon Sep 17 00:00:00 2001 From: Tom Dawes Date: Fri, 4 Oct 2019 16:18:15 +0100 Subject: [PATCH 2/6] Fix tests --- packages/core/tests/watch.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/tests/watch.test.ts b/packages/core/tests/watch.test.ts index 31223da9..4b1f6bfa 100644 --- a/packages/core/tests/watch.test.ts +++ b/packages/core/tests/watch.test.ts @@ -38,7 +38,7 @@ const createNode = (name: string, compId: number, pathKey: string): Node => ({ name, compId, pathKey, - setState: () => {}, // tslint:disable-line:no-empty + forceUpdate: () => void {}, status: { unmounted: false }, }); From 9ac35afe286caf85ba05c73da8f79de50c960ac7 Mon Sep 17 00:00:00 2001 From: Tom Dawes Date: Fri, 4 Oct 2019 16:59:01 +0100 Subject: [PATCH 3/6] Use correct negation --- packages/core/src/connect.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/core/src/connect.tsx b/packages/core/src/connect.tsx index 0297a79c..2f8fa7f1 100644 --- a/packages/core/src/connect.tsx +++ b/packages/core/src/connect.tsx @@ -160,7 +160,7 @@ export const connect: Connect =

( }); Object.keys(prevWatched).forEach(pathKey => { - const keyDeleted = watched.current.hasOwnProperty(pathKey); + const keyDeleted = !watched.current.hasOwnProperty(pathKey); if (keyDeleted) { logger.info(`[update] ${name}: stop watching < ${pathKey} >`); unsubscribe(splitPath(pathKey)); @@ -222,6 +222,9 @@ export const connect: Connect =

( return (func as ((args: any) => (props: any) => any))(ctx)(props); }; +// We're not applying any kind of `shouldComponentUpdate` any more. +// Previous code was: +// // public shouldComponentUpdate(nextProps: P, nextState: any) { // const test = // !shallowEqual(this.props, nextProps) || From adfa4f7e1ea9ac564661bd288ba4640037b8183a Mon Sep 17 00:00:00 2001 From: Tom Dawes Date: Fri, 4 Oct 2019 17:19:19 +0100 Subject: [PATCH 4/6] Fix a couple of errors --- packages/core/src/connect.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/src/connect.tsx b/packages/core/src/connect.tsx index 2f8fa7f1..27dc27a4 100644 --- a/packages/core/src/connect.tsx +++ b/packages/core/src/connect.tsx @@ -37,12 +37,12 @@ const getValue = (path: string[], obj: any): any => const valueExtractor = ( store: Store, - watched: { [key: string]: any }, + watched: React.MutableRefObject<{ [key: string]: any }>, ) => (x: any) => { const path = x[pathSymbol]; const pathKey = joinPath(path); const value = getValue(path, store.universe); - watched[pathKey] = value; + watched.current[pathKey] = value; return value; }; @@ -159,7 +159,7 @@ export const connect: Connect =

( } }); - Object.keys(prevWatched).forEach(pathKey => { + Object.keys(prevWatched.current).forEach(pathKey => { const keyDeleted = !watched.current.hasOwnProperty(pathKey); if (keyDeleted) { logger.info(`[update] ${name}: stop watching < ${pathKey} >`); @@ -185,7 +185,7 @@ export const connect: Connect =

( }; }, []); - const watch = valueExtractor(store, watched.current); + const watch = valueExtractor(store, watched); const _subscribe = (path: string[], unsubscribe?: () => void): void => { const pathKey = joinPath(path); From 80ba338fbf82db440d02006eda374de13d5fc8c5 Mon Sep 17 00:00:00 2001 From: Tom Dawes Date: Fri, 4 Oct 2019 18:20:42 +0100 Subject: [PATCH 5/6] Fix a race condition between rendering and mounting --- packages/core/src/watch.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/core/src/watch.ts b/packages/core/src/watch.ts index d8f4f33b..b685edcb 100644 --- a/packages/core/src/watch.ts +++ b/packages/core/src/watch.ts @@ -2,10 +2,14 @@ import logger from "./logger"; import { Event, Node, Store, WatchTree } from "./types"; import { splitPath } from "./utils"; +const getValue = (path: string[], obj: any): any => + path.reduce((x: any, y: any) => x && x[y], obj); + export const subscribe = ( store: Store, path: string[], node: Node, + seenValue: any = undefined, ) => { // root tree let tree: WatchTree = store.watchTree; @@ -33,6 +37,10 @@ export const subscribe = ( // add node to esubs of exact part of state tree that was subscribed tree.esubs.add(node); + + if (seenValue !== getValue(path, store.universe)) { + node.forceUpdate(); + } }; export const unsubscribe = ( From c762e81886db4cc5d17065b70d6240fea1fef85b Mon Sep 17 00:00:00 2001 From: Tom Dawes Date: Sun, 6 Oct 2019 17:03:14 +0100 Subject: [PATCH 6/6] Update the subscriptions eimmediately --- packages/core/src/connect.tsx | 131 ++++++++++++++++------------------ packages/core/src/watch.ts | 15 ++-- 2 files changed, 71 insertions(+), 75 deletions(-) diff --git a/packages/core/src/connect.tsx b/packages/core/src/connect.tsx index 27dc27a4..a5825092 100644 --- a/packages/core/src/connect.tsx +++ b/packages/core/src/connect.tsx @@ -35,52 +35,6 @@ const useForceUpdate = () => { const getValue = (path: string[], obj: any): any => path.reduce((x: any, y: any) => x && x[y], obj); -const valueExtractor = ( - store: Store, - watched: React.MutableRefObject<{ [key: string]: any }>, -) => (x: any) => { - const path = x[pathSymbol]; - const pathKey = joinPath(path); - const value = getValue(path, store.universe); - watched.current[pathKey] = value; - return value; -}; - -export const shallowEqual = (objA: any, objB: any): boolean => { - if (Object.is(objA, objB)) { - return true; - } - - if ( - typeof objA !== "object" || - objA === null || - typeof objB !== "object" || - objB === null - ) { - return false; - } - - const keysA = Object.keys(objA); - const keysB = Object.keys(objB); - - if (keysA.length !== keysB.length) { - return false; - } - - // Test for A's keys different from B. - // tslint:disable-next-line:prefer-for-of - for (let i = 0; i < keysA.length; i++) { - if ( - !Object.prototype.hasOwnProperty.call(objB, keysA[i]) || - !Object.is(objA[keysA[i]], objB[keysA[i]]) - ) { - return false; - } - } - - return true; -}; - let _compIdCnt = 1; export type Func = (viewCtx: V) => React.ComponentType

; @@ -93,23 +47,21 @@ export const connect: Connect =

( const name = baseName + "." + compId.current; logger.info(`[rendering] ${name}`); - // First render only - React.useMemo(() => logger.info(`[constructing] ${name}`), []); const store = React.useContext(ProdoContext); const forceUpdate = useForceUpdate(); // Subscribing to part of the state - const status = React.useRef({ unmounted: true }); + const status = React.useRef({ unmounted: false }); React.useEffect(() => { logger.info(`[did mount] ${name}`); - status.current.unmounted = false; logger.debug("store", store); }, []); const pathNodes = React.useRef({}); const subscribe = (path: string[], unsubscribe?: (comp: Comp) => void) => { const pathKey = joinPath(path); + logger.info(`[subscribe] ${name} subscribing to ${pathKey}`); const node: Node = pathNodes.current[pathKey] || @@ -145,21 +97,29 @@ export const connect: Connect =

( ); const watched = React.useRef({}); - const prevWatched = React.useRef({}); + const subscriptions = React.useRef({}); + // As we are rendering, create subscriptions + const watch = React.useCallback( + (proxy: any) => { + const path = proxy[pathSymbol]; + const pathKey = joinPath(path); + const value = getValue(path, store.universe); + watched.current[pathKey] = true; + if (!subscriptions.current[pathKey]) { + subscriptions.current[pathKey] = true; + subscribe(path); + } + return value; + }, + [store], + ); - // On update, update subscriptions + // After updating, prune subscriptions React.useEffect(() => { - logger.info(`[updating watch] ${name}`); - Object.keys(watched.current).forEach(pathKey => { - const keyExisted = prevWatched.current.hasOwnProperty(pathKey); - if (!keyExisted) { - logger.info(`[update] ${name}: now watching < ${pathKey} >`); - subscribe(splitPath(pathKey)); - } + logger.info(`[update] ${name}: watching < ${pathKey} >`); }); - - Object.keys(prevWatched.current).forEach(pathKey => { + Object.keys(subscriptions.current).forEach(pathKey => { const keyDeleted = !watched.current.hasOwnProperty(pathKey); if (keyDeleted) { logger.info(`[update] ${name}: stop watching < ${pathKey} >`); @@ -167,7 +127,7 @@ export const connect: Connect =

( } }); - prevWatched.current = { ...watched.current }; + subscriptions.current = { ...watched.current }; watched.current = {}; }); @@ -175,22 +135,21 @@ export const connect: Connect =

( React.useEffect(() => { return () => { logger.info(`[unmounting]: ${name}`, watched.current); - - Object.keys(prevWatched.current).forEach(pathKey => { + Object.keys(subscriptions.current).forEach(pathKey => { logger.info(`[unmount] ${name}: stop watching < ${pathKey} >`); unsubscribe(splitPath(pathKey)); }); - logger.debug("store", store); status.current = { unmounted: true }; }; }, []); - const watch = valueExtractor(store, watched); - const _subscribe = (path: string[], unsubscribe?: () => void): void => { const pathKey = joinPath(path); watched.current[pathKey] = getValue(path, store.universe); - subscribe(path, unsubscribe); + if (!subscriptions.current[pathKey]) { + subscriptions.current[pathKey] = true; + subscribe(path, unsubscribe); + } }; const ctx = React.useMemo(() => { @@ -217,11 +176,45 @@ export const connect: Connect =

( } }); return ctx; - }, [store.universe]); + }, [store]); return (func as ((args: any) => (props: any) => any))(ctx)(props); }; +// export const shallowEqual = (objA: any, objB: any): boolean => { +// if (Object.is(objA, objB)) { +// return true; +// } + +// if ( +// typeof objA !== "object" || +// objA === null || +// typeof objB !== "object" || +// objB === null +// ) { +// return false; +// } + +// const keysA = Object.keys(objA); +// const keysB = Object.keys(objB); + +// if (keysA.length !== keysB.length) { +// return false; +// } + +// // Test for A's keys different from B. +// // tslint:disable-next-line:prefer-for-of +// for (let i = 0; i < keysA.length; i++) { +// if ( +// !Object.prototype.hasOwnProperty.call(objB, keysA[i]) || +// !Object.is(objA[keysA[i]], objB[keysA[i]]) +// ) { +// return false; +// } +// } + +// return true; +// }; // We're not applying any kind of `shouldComponentUpdate` any more. // Previous code was: // diff --git a/packages/core/src/watch.ts b/packages/core/src/watch.ts index b685edcb..1d4a67a9 100644 --- a/packages/core/src/watch.ts +++ b/packages/core/src/watch.ts @@ -2,14 +2,14 @@ import logger from "./logger"; import { Event, Node, Store, WatchTree } from "./types"; import { splitPath } from "./utils"; -const getValue = (path: string[], obj: any): any => - path.reduce((x: any, y: any) => x && x[y], obj); +// const getValue = (path: string[], obj: any): any => +// path.reduce((x: any, y: any) => x && x[y], obj); export const subscribe = ( store: Store, path: string[], node: Node, - seenValue: any = undefined, + // seenValue: any, ) => { // root tree let tree: WatchTree = store.watchTree; @@ -38,9 +38,10 @@ export const subscribe = ( // add node to esubs of exact part of state tree that was subscribed tree.esubs.add(node); - if (seenValue !== getValue(path, store.universe)) { - node.forceUpdate(); - } + // if (seenValue !== getValue(path, store.universe)) { + // console.log(`Not seen ${path} before, so rerendering`); + // node.forceUpdate(); + // } }; export const unsubscribe = ( @@ -138,6 +139,8 @@ export const submitPatches = ( event.rerender![comps[compId].name] = true; logger.info(`[upcoming state update] ${name}`, newValues, status); forceUpdate(); + } else { + logger.info(`[can't update state] ${name}`, status); } }); };