Skip to content

Commit

Permalink
fix: store methods context
Browse files Browse the repository at this point in the history
  • Loading branch information
amirqasemi74 authored and Amir Hossein Qasemi Moqaddam committed Aug 27, 2022
1 parent 1c3ab80 commit 51ca2ed
Show file tree
Hide file tree
Showing 11 changed files with 128 additions and 115 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@react-store/core",
"version": "0.0.32",
"version": "0.0.33",
"main": "dist/index.js",
"repository": "https://github.com/sahabpardaz/react-store.git",
"author": "Amir Hossein Qasemi Moqaddam <[email protected]>",
Expand Down
4 changes: 0 additions & 4 deletions src/proxy/storeForConsumerComponentProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ export class StoreForConsumerComponentProxy implements ProxyHandler<object> {
);
}

if (storeAdmin?.methodsManager.methods.has(propertyKey)) {
return storeAdmin.methodsManager.methods.get(propertyKey)?.storeBound;
}

if (storeAdmin?.gettersManager.getters.has(propertyKey)) {
return storeAdmin.gettersManager.getters.get(propertyKey)?.getValue("State");
}
Expand Down
74 changes: 0 additions & 74 deletions src/store/administrator/effects/effectsProxy.ts

This file was deleted.

46 changes: 46 additions & 0 deletions src/store/administrator/methods/methodProxyHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { StoreAdministrator } from "../storeAdministrator";

/**
* Effect context must be proxied to use value from state insteadOf store
* because we have hooks like useDeferredValue or useTransition can others hooks cause
* rerenders but these hooks value not change in that render
* So we must bind effect context to state values.
*/
export class MethodProxyHandler implements ProxyHandler<object> {
directMutatedStoreProperties = new Map<PropertyKey, unknown>();

constructor(private storeAdmin: StoreAdministrator) {}

get(target: object, propertyKey: PropertyKey, receiver: unknown) {
/**
* Because we change effect context to state if we set value it will be
* done async and if we read the value immediately it doesn't work
* so we make trick here only for primitive types
*/
if (this.directMutatedStoreProperties.has(propertyKey)) {
return this.directMutatedStoreProperties.get(propertyKey);
}

if (this.storeAdmin?.propertyKeysManager.propertyKeys.has(propertyKey)) {
return this.storeAdmin?.propertyKeysManager.propertyKeys
.get(propertyKey)
?.getValue("State", false);
}

// Getters
if (this.storeAdmin?.gettersManager.getters.has(propertyKey)) {
return this.storeAdmin.gettersManager.getters
.get(propertyKey)
?.getValue("State");
}

return Reflect.get(target, propertyKey, receiver);
}

set(_: object, propertyKey: PropertyKey, value: unknown) {
if (this.storeAdmin?.propertyKeysManager.onSetPropertyKey(propertyKey, value)) {
this.directMutatedStoreProperties.set(propertyKey, value);
}
return true;
}
}
Original file line number Diff line number Diff line change
@@ -1,20 +1,33 @@
import type { StoreAdministrator } from "./storeAdministrator";
import type { StoreAdministrator } from "../storeAdministrator";
import { MethodProxyHandler } from "./methodProxyHandler";
import { Func } from "src/types";

export class StoreMethodsManager {
methods = new Map<PropertyKey, { storeBound: Func | null }>();
private methods = new Map<string, Func>();

handler = new MethodProxyHandler(this.storeAdmin);

constructor(private storeAdmin: StoreAdministrator) {}

bindMethods() {
const context = new Proxy(this.storeAdmin.instance, this.handler);

Object.entries(this.getMethodsPropertyDescriptors(this.storeAdmin.instance))
.filter(([key]) => key !== "constructor")
.filter(([, desc]) => desc.value) // only methods not getter or setter
.forEach(([methodKey, descriptor]) => {
this.methods.set(methodKey, {
storeBound: descriptor.value?.bind(this.storeAdmin.instance),
this.methods.set(methodKey, descriptor.value?.bind(context));
Object.defineProperty(this.storeAdmin.instance, methodKey, {
enumerable: false,
configurable: true,
get: () => this.methods.get(methodKey),
});
});

this.storeAdmin.hooksManager.reactHooks.add({
when: "AFTER_INSTANCE",
hook: () => this.handler.directMutatedStoreProperties.clear(),
});
}

private getMethodsPropertyDescriptors(
Expand Down
11 changes: 7 additions & 4 deletions src/store/administrator/propertyKeys/observableProperty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,18 @@ export class ObservableProperty {
}
}

getValue(from: "State" | "Store") {
getValue(from: "State" | "Store", doUnproxy = true) {
switch (from) {
case "State":
return this.isPrimitive
case "State": {
const value = this.isPrimitive
? this.value.state
: // due to performance we return pure values of store properties
// not proxied ones, pure value does not collect access logs
// and this is good
getUnproxiedValue((this.value.state as { $: unknown } | undefined)?.$);
(this.value.state as { $: unknown } | undefined)?.$;

return doUnproxy ? getUnproxiedValue(value) : value;
}
case "Store":
return this.isPrimitive
? this.value.store
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,18 @@ export class StorePropertyKeysManager {
if (info instanceof ObservableProperty) {
info.isSetStatePending = true;
storeValueAndRenderIfNeed();
return true;
}

if (info instanceof ReadonlyProperty) {
if (force) {
storeValueAndRenderIfNeed();
return true;
} else {
this.readonlyPropertyKeys
.find(({ matcher }) => matcher(propertyKey))
?.onSet(propertyKey);
return false;
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/store/administrator/storeAdministrator.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { STORE_ADMINISTRATION } from "../../constant";
import { StoreForConsumerComponentProxy } from "../../proxy/storeForConsumerComponentProxy";
import { StoreEffectsManager } from "./effects/storeEffectsManager";
import { StoreGettersManager } from "./getters/storeGettersManager";
import { HooksManager } from "./hooksManager";
import { StoreMethodsManager } from "./methods/storeMethodsManager";
import { StorePropertyKeysManager } from "./propertyKeys/storePropertyKeysManager";
import { PropsManager } from "./propsManager";
import { StoreMethodsManager } from "./storeMethodsManager";
import { StoreEffectsManager } from "./storeEffectsManager";
import { StoreStorePartsManager } from "./storeStorePartsManager";
import { ClassType } from "src/types";

Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,14 @@
import type { StoreAdministrator } from "../storeAdministrator";
import { EffectProxy } from "./effectsProxy";
import type { StoreAdministrator } from "./storeAdministrator";
import cloneDeep from "clone-deep";
import { dequal } from "dequal";
import isPromise from "is-promise";
import { useEffect, useRef } from "react";
import { EffectsMetadataUtils, ManualEffectOptions } from "src/decorators/effect";
import { ClassType, Func } from "src/types";
import { Func } from "src/types";

export class StoreEffectsManager {
readonly effects = new Map<PropertyKey, { clear?: Func<void> }>();

private effectProxy: EffectProxy;

context: InstanceType<ClassType>;

constructor(private storeAdmin: StoreAdministrator) {}

registerEffects() {
Expand All @@ -28,9 +23,6 @@ export class StoreEffectsManager {
},
});
});

this.effectProxy = new EffectProxy(this.storeAdmin);
this.context = new Proxy(this.storeAdmin.instance, this.effectProxy);
}

private manualEffectHandler(
Expand Down Expand Up @@ -58,17 +50,16 @@ export class StoreEffectsManager {
}

useEffect(() => {
this.effectProxy.directMutatedStoreProperties.clear();
this.runEffect(effectKey, storeAdmin);
return () => effectsManager.getClearEffect(effectKey)?.();
}, [signal.current]);
}

private runEffect(effectKey: PropertyKey, storeAdmin: StoreAdministrator) {
// Run Effect
const clearEffect: Func<void> | undefined = (
storeAdmin.instance[effectKey] as Func
)?.apply(this.context) as undefined;
const clearEffect = (storeAdmin.instance[effectKey] as Func)?.() as
| Func<void>
| undefined;

if (
clearEffect &&
Expand Down
20 changes: 10 additions & 10 deletions tests/effectDecorator.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Effect, Observable, Store, connect, useStore } from "@react-store/core";
import { fireEvent, render, waitFor } from "@testing-library/react";
import React, { ChangeEvent } from "react";
import React, { ChangeEvent, useDeferredValue, useState } from "react";
import { act } from "react-dom/test-utils";
import { clearContainer } from "src/container/container";

Expand Down Expand Up @@ -349,8 +349,8 @@ describe("Effects", () => {
render(<User />);
});

it("should store method bind to effect context if is called from effect", () => {
expect.assertions(1);
it("should store method bind to effect context if is called from effect", async () => {
expect.assertions(2);

@Store()
class UserStore {
Expand All @@ -361,23 +361,23 @@ describe("Effects", () => {
}

@Effect([])
changePassword() {
async changePassword() {
this.password = "pass2";
this.getPassword();
}
}

const User = connect(() => {
const vm = useStore(UserStore);
return (
<>
<p>{vm.password}</p>
</>
);

return <p>{vm.password}</p>;
}, UserStore);

render(<User />);
const { getByText } = render(<User />);

await waitFor(() => expect(getByText("pass2")).toBeInTheDocument());
});

});

describe("Parent Class", () => {
Expand Down
39 changes: 37 additions & 2 deletions tests/methodsAutoBind.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { AutoWire, Store, StorePart, connect, useStore } from "@react-store/core";
import { render } from "@testing-library/react";
import {
AutoWire,
Effect,
Store,
StorePart,
connect,
useStore,
} from "@react-store/core";
import { render, waitFor } from "@testing-library/react";
import React from "react";

describe("Methods Auto Bind", () => {
Expand Down Expand Up @@ -67,4 +74,32 @@ describe("Methods Auto Bind", () => {

expect(store.method()).toBe(6);
});

it("should bind methods inside store class", async () => {
@Store()
class UserStore {
username = "amir";

changeUsername() {
this.username = "reza";
}

@Effect([])
onMount() {
setTimeout(this.changeUsername, 0);
}
}

const User = connect(() => {
const st = useStore(UserStore);

return <>{st.username}</>;
}, UserStore);

const { getByText } = render(<User />);

expect(getByText("amir")).toBeInTheDocument();

await waitFor(() => expect(getByText("reza")).toBeInTheDocument());
});
});

0 comments on commit 51ca2ed

Please sign in to comment.