Skip to content

Commit

Permalink
Bugfix: RefObject was derferenced in Provider render instead of Resou…
Browse files Browse the repository at this point in the history
…rce read, but Provider does not re-render when Suspense awaits thrown promise
  • Loading branch information
patrickroberts committed Oct 1, 2020
1 parent 333c2f1 commit 4f716b7
Show file tree
Hide file tree
Showing 14 changed files with 51 additions and 29 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ jobs:
- name: Install and Test
run: |
npm install
npm run rebuild
npm test
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
2 changes: 1 addition & 1 deletion features/support/context/background.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { Given } from '@cucumber/cucumber';
import World from '../world';
import { Context, createContext, useContext } from '../../../src';
import { Context, createContext, useContext } from '../../..';

const createComponent = (Context: Context<string>, Element: keyof JSX.IntrinsicElements) => (
(props: { id?: string | null }) => {
Expand Down
2 changes: 1 addition & 1 deletion features/support/context/declarations.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { DataTable, Given } from '@cucumber/cucumber';
import World from '../world';
import { Context } from '../../../src';
import { Context } from '../../..';

Given(
'I declare a {string} consumer named {string} with the props',
Expand Down
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "suspense-service",
"version": "0.0.2-rc",
"version": "0.0.3-rc",
"description": "Suspense integration library for React",
"repository": "github:patrickroberts/suspense-service",
"main": "dst/cjs/suspense-service.js",
Expand All @@ -9,14 +9,14 @@
"types": "dst/esm/suspense-service.d.ts",
"scripts": {
"clean": "rimraf dst docs",
"test": "cucumber-js",
"build": "rollup --config --environment NODE_ENV:production",
"rebuild": "npm run clean && npm run build",
"test": "cucumber-js",
"docs": "typedoc src/index.ts",
"watch:test": "npm test -- --watch",
"watch:build": "npm run build -- --environment NODE_ENV:development --watch",
"watch:test": "npm test -- --watch",
"watch": "concurrently 'npm:watch:*'",
"prepublishOnly": "npm test && npm run rebuild && npm run docs"
"prepublishOnly": "npm run rebuild && npm test && npm run docs"
},
"author": "Patrick Roberts",
"license": "MIT",
Expand Down
5 changes: 3 additions & 2 deletions src/Context/Consumer.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import React, { Context, FunctionComponent, ReactNode, memo, useCallback, useMemo } from 'react';
import PropTypes from 'prop-types';
import Id, { PropTypesId } from './Id';
import Environment, { unwrap } from './Environment';

interface ContextConsumerProps<T> {
id?: string | null;
id?: Id;
children: (value: T) => ReactNode;
}

Expand All @@ -13,7 +14,7 @@ export default ContextConsumer;

/** @ignore */
const propTypes = {
id: PropTypes.string,
id: PropTypesId,
children: PropTypes.func.isRequired
};

Expand Down
14 changes: 8 additions & 6 deletions src/Context/Environment.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
import Id from './Id';

/** @internal */
export default interface Environment<T> {
has (key: string | null): boolean;
get (key: string | null): T | undefined;
[Symbol.iterator] (): IterableIterator<[string | null, T]>;
has (key: Id): boolean;
get (key: Id): T | undefined;
[Symbol.iterator] (): IterableIterator<[Id, T]>;
};

/** @ignore */
export function wrap<T>(
env: Environment<T>,
value: T,
id: string | null
id: Id
): Environment<T> {
return new Map([...env, [id, value], [null, value]]);
}

/** @ignore */
export function unwrap<T>(
env: Environment<T>,
id: string | null
id: Id
): T {
if (!env.has(id)) {
throw new Error(`Provider with id ${id} is not in scope`);
throw new Error(`Provider with id ${String(id)} is not in scope`);
}

return env.get(id)!;
Expand Down
8 changes: 8 additions & 0 deletions src/Context/Id.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { oneOfType, string, number, symbol } from 'prop-types';

type Id = string | number | symbol | null;

export default Id;

/** @ignore */
export const PropTypesId = oneOfType([string, number, symbol]);
5 changes: 3 additions & 2 deletions src/Context/Provider.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React, { Context, FunctionComponent, ReactNode, memo, useContext, useMemo } from 'react';
import PropTypes from 'prop-types';
import Id, { PropTypesId } from './Id';
import Environment, { wrap } from './Environment';

interface ContextProviderProps<T> {
value: T;
id?: string | null;
id?: Id;
children?: ReactNode;
}

Expand All @@ -17,7 +18,7 @@ const propTypes = {
// typechecking PropTypes.any against T fails due to
// the way WeakValidationMap<T> works in @types/react
value: PropTypes.any.isRequired as any,
id: PropTypes.string,
id: PropTypesId,
children: PropTypes.node
};

Expand Down
3 changes: 2 additions & 1 deletion src/Context/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
createContext as createReactContext,
useContext as useReactContext
} from 'react';
import Id from './Id';
import Environment, { unwrap } from './Environment';
import ContextConsumer, { createConsumer } from './Consumer';
import ContextProvider, { createProvider } from './Provider';
Expand Down Expand Up @@ -40,7 +41,7 @@ export function createContext<T>(defaultValue: T): Context<T> {
*/
export function useContext<T>(
Context: Context<T>,
id: string | null = null
id: Id = null
): T {
const env = useReactContext(Context[kContext]);

Expand Down
5 changes: 3 additions & 2 deletions src/Service/Consumer.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React, { FunctionComponent, ReactNode, memo, useCallback, useMemo } from 'react';
import PropTypes from 'prop-types';
import Id, { PropTypesId } from '../Context/Id';
import Context from '../Context/index';
import Resource from './Resource';

interface ServiceConsumerProps<TResponse> {
id?: string | null;
id?: Id;
children: (value: TResponse) => ReactNode;
}

Expand All @@ -14,7 +15,7 @@ export default ServiceConsumer;

/** @ignore */
const propTypes = {
id: PropTypes.string,
id: PropTypesId,
children: PropTypes.func.isRequired
};

Expand Down
7 changes: 4 additions & 3 deletions src/Service/Provider.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React, { Fragment, FunctionComponent, ReactNode, Suspense, isValidElement, memo, useMemo } from 'react';
import PropTypes from 'prop-types';
import Id, { PropTypesId } from '../Context/Id';
import Context from '../Context/index';
import Resource, { useResource } from './Resource';

interface ServiceProviderProps<TRequest> {
value: TRequest;
id?: string | null;
id?: Id;
children?: ReactNode;
fallback?: NonNullable<ReactNode> | null;
}
Expand All @@ -19,7 +20,7 @@ const propTypes = {
// typechecking PropTypes.any against TRequest fails due to
// the way WeakValidationMap<TRequest> works in @types/react
value: PropTypes.any.isRequired as any,
id: PropTypes.string,
id: PropTypesId,
children: PropTypes.node,
fallback: PropTypes.node
};
Expand All @@ -34,7 +35,7 @@ const defaultProps = {
/** @ignore */
export function createProvider<TRequest, TResponse>(
Context: Context<Resource<TResponse>>,
useHandler: (request: TRequest, id: string | null) => PromiseLike<TResponse>
useHandler: (request: TRequest, id: Id) => PromiseLike<TResponse>
): ServiceProvider<TRequest> {
const Provider: ServiceProvider<TRequest> = ({ value, id = null, children, fallback }) => {
const thenable = useHandler(value, id);
Expand Down
6 changes: 4 additions & 2 deletions src/Service/Resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,17 @@ export default interface Resource<TResponse> {
export function useResource<TResponse>(
thenable: PromiseLike<TResponse>
): Resource<TResponse> {
const state = useThenable(thenable);
const ref = useThenable(thenable);

return useMemo(() => ({
read () {
const state = ref.current!;

switch (state.status) {
case 'pending': throw state.promise;
case 'rejected': throw state.reason;
case 'fulfilled': return state.value;
}
}
}), [state]);
}), [ref]);
}
9 changes: 6 additions & 3 deletions src/Service/Thenable.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useMemo, useRef } from 'react';
import { MutableRefObject, useMemo, useRef } from 'react';

/** @ignore */
interface PromiseStatePending {
Expand All @@ -24,10 +24,13 @@ export type PromiseState<TResponse> =
PromiseStateFulfilled<TResponse> |
PromiseStateRejected;

/** @ignore */
type RefObject<T> = MutableRefObject<T | undefined>;

/** @ignore */
export function useThenable<TResponse>(
thenable: PromiseLike<TResponse>
): PromiseState<TResponse> {
): RefObject<PromiseState<TResponse>> {
const ref = useRef<PromiseState<TResponse>>();

useMemo(() => {
Expand All @@ -51,5 +54,5 @@ export function useThenable<TResponse>(
ref.current = { promise, status: 'pending' };
}, [thenable, ref]);

return ref.current!;
return ref;
}
5 changes: 3 additions & 2 deletions src/Service/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Id from '../Context/Id';
import Context, { createContext, useContext } from '../Context/index';
import Resource from './Resource';
import ServiceConsumer, { createConsumer } from './Consumer';
Expand All @@ -14,7 +15,7 @@ export default interface Service<TRequest, TResponse> {
}

export function createService<TRequest, TResponse>(
useHandler: (request: TRequest) => PromiseLike<TResponse>
useHandler: (request: TRequest, id: Id) => PromiseLike<TResponse>
): Service<TRequest, TResponse> {
const Context = createContext<Resource<TResponse>>({
read () {
Expand All @@ -31,7 +32,7 @@ export function createService<TRequest, TResponse>(

export function useService<TResponse>(
Service: Service<any, TResponse>,
id: string | null = null
id: Id = null
): TResponse {
const resource = useContext(Service[kContext], id);

Expand Down

0 comments on commit 4f716b7

Please sign in to comment.