From f27947262f08dc38d5c9247ade3156b27f3fcdc3 Mon Sep 17 00:00:00 2001 From: Alberto Gasparin Date: Thu, 4 Jan 2024 16:22:57 +1100 Subject: [PATCH 1/3] Fix provider global clenup on error --- src/eslint/rules/no-extraneous.js | 4 +- src/react/__tests__/provider.test.js | 48 +++++++++++++++++++- src/react/provider.js | 66 ++++++++++++++++++---------- 3 files changed, 92 insertions(+), 26 deletions(-) diff --git a/src/eslint/rules/no-extraneous.js b/src/eslint/rules/no-extraneous.js index 578021d..cb6c0ba 100644 --- a/src/eslint/rules/no-extraneous.js +++ b/src/eslint/rules/no-extraneous.js @@ -62,7 +62,9 @@ module.exports = { blockReferences.set(node, { di: diVars, - through: context.getScope().through.map((v) => v.identifier), + through: context.sourceCode + .getScope(node) + .through.map((v) => v.identifier), }); }, diff --git a/src/react/__tests__/provider.test.js b/src/react/__tests__/provider.test.js index 4afd230..b25f291 100644 --- a/src/react/__tests__/provider.test.js +++ b/src/react/__tests__/provider.test.js @@ -3,7 +3,7 @@ */ /* eslint-env jest */ -import React, { forwardRef } from 'react'; +import React, { Component, forwardRef } from 'react'; import { render } from '@testing-library/react'; import { Context } from '../context'; @@ -27,6 +27,20 @@ describe('DiProvider', () => { }); }); + it.only('should expose stable context', () => { + const children = jest.fn(); + const App = () => ( + + {children} + + ); + + const { rerender } = render(); + rerender(); + + expect(children.mock.calls[0][0]).toBe(children.mock.calls[1][0]); + }); + describe('getDependencies()', () => { const Text = () => 'text'; const Button = forwardRef(() => 'button'); @@ -161,6 +175,38 @@ describe('DiProvider', () => { expect(globalDi._remove).toHaveBeenCalledWith([TextDi2]); }); + it('should remove global injection on error', () => { + jest.spyOn(globalDi, '_remove'); + const cSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + const children = jest.fn().mockImplementation(() => { + throw new Error('Oops!'); + }); + const TextDi = injectable(Text, () => '', { global: true }); + const TextDi2 = injectable(Text, () => ''); + class ErrorBoundary extends Component { + componentDidCatch() { + cSpy.mockRestore(); + } + render() { + return this.props.children; + } + } + + render( + + + + {children} + + + + ); + + expect(globalDi._remove).toHaveBeenCalledWith([TextDi]); + expect(globalDi._remove).toHaveBeenCalledWith([TextDi2]); + }); + it('should error when a non injectable is used', () => { const cSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); expect(() => { diff --git a/src/react/provider.js b/src/react/provider.js index 9ea9d85..a6c8596 100644 --- a/src/react/provider.js +++ b/src/react/provider.js @@ -1,4 +1,4 @@ -import React, { useContext, useMemo, forwardRef, useEffect } from 'react'; +import React, { forwardRef, Component } from 'react'; import PropTypes from 'prop-types'; import { diRegistry } from './constants'; @@ -6,11 +6,37 @@ import { Context } from './context'; import { addInjectableToMap, getDisplayName, findInjectable } from './utils'; import { globalDi } from './global'; -export const DiProvider = ({ children, use, target, global }) => { - const { getDependencies } = useContext(Context); +export class DiProvider extends Component { + static contextType = Context; + + static propTypes = { + children: PropTypes.oneOfType([PropTypes.func, PropTypes.node]), + global: PropTypes.bool, + target: PropTypes.oneOfType([ + PropTypes.func, + PropTypes.arrayOf(PropTypes.func), + ]), + use: PropTypes.arrayOf( + PropTypes.oneOfType([PropTypes.func, PropTypes.object]) + ).isRequired, + }; + + componentDidCatch(err) { + globalDi._remove(this.props.use); + throw err; + } + + componentWillUnmount() { + globalDi._remove(this.props.use); + } + + value = undefined; + getValue() { + if (this.value) return this.value; + + const { use, target, global } = this.props; + const { getDependencies } = this.context; - // memo provider value so gets computed only once - const value = useMemo(() => { // create a map of dependency real -> replacements for fast lookup const replacementMap = use.reduce((acc, inj) => { addInjectableToMap(acc, inj); @@ -21,7 +47,7 @@ export const DiProvider = ({ children, use, target, global }) => { // support single or multiple targets const targets = target && (Array.isArray(target) ? target : [target]); - return { + this.value = { getDependencies(realDeps, targetChild) { // First we collect dependencies from parent provider(s) (if any) const dependencies = getDependencies(realDeps, targetChild); @@ -44,25 +70,17 @@ export const DiProvider = ({ children, use, target, global }) => { return dependencies; }, }; - }, [getDependencies]); // ignore use & target props - - // on unmount - useEffect(() => () => globalDi._remove(use), []); // ignore use prop + return this.value; + } - return {children}; -}; - -DiProvider.propTypes = { - children: PropTypes.oneOfType([PropTypes.func, PropTypes.node]), - global: PropTypes.bool, - target: PropTypes.oneOfType([ - PropTypes.func, - PropTypes.arrayOf(PropTypes.func), - ]), - use: PropTypes.arrayOf( - PropTypes.oneOfType([PropTypes.func, PropTypes.object]) - ).isRequired, -}; + render() { + return ( + + {this.props.children} + + ); + } +} export function withDi(Comp, deps, target = null) { const WrappedComponent = forwardRef((props, ref) => ( From 119659801ee0fee3d3e4ed6525ca1fa94e355c40 Mon Sep 17 00:00:00 2001 From: Alberto Gasparin Date: Thu, 4 Jan 2024 17:07:10 +1100 Subject: [PATCH 2/3] Remove test only --- src/react/__tests__/provider.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/react/__tests__/provider.test.js b/src/react/__tests__/provider.test.js index b25f291..a0f3b13 100644 --- a/src/react/__tests__/provider.test.js +++ b/src/react/__tests__/provider.test.js @@ -27,7 +27,7 @@ describe('DiProvider', () => { }); }); - it.only('should expose stable context', () => { + it('should expose stable context', () => { const children = jest.fn(); const App = () => ( From 78f492cdd7937118d8dab94ae4ea5f71a530ff1c Mon Sep 17 00:00:00 2001 From: Alberto Gasparin Date: Fri, 5 Jan 2024 15:56:02 +1100 Subject: [PATCH 3/3] Fix examples --- examples/.babelrc.js | 5 ++++- examples/flow/components/input.js | 2 -- examples/flow/components/label.js | 2 -- examples/flow/components/section.js | 3 --- examples/global/index.ts | 4 ---- examples/typescript/components/input.tsx | 2 -- examples/typescript/components/label.tsx | 3 --- examples/typescript/components/section.tsx | 4 ---- src/index.js | 2 +- 9 files changed, 5 insertions(+), 22 deletions(-) diff --git a/examples/.babelrc.js b/examples/.babelrc.js index 3d1bc60..0c2cc09 100644 --- a/examples/.babelrc.js +++ b/examples/.babelrc.js @@ -3,5 +3,8 @@ module.exports = { extends: '../babel.config.js', presets: ['@babel/preset-env', '@babel/preset-react', '@babel/preset-flow'], - plugins: ['@babel/plugin-proposal-class-properties', '../src/babel'], + plugins: [ + '@babel/plugin-proposal-class-properties', + ['../src/babel', { enabledEnvs: ['development', 'test', undefined] }], + ], }; diff --git a/examples/flow/components/input.js b/examples/flow/components/input.js index 69c0afa..7d90974 100644 --- a/examples/flow/components/input.js +++ b/examples/flow/components/input.js @@ -1,6 +1,5 @@ // @flow import React, { useState, type Node } from 'react'; -import { di } from 'react-magnetic-di'; type UseThemeState = { color: string }; export function useTheme(): $Call { @@ -12,7 +11,6 @@ export type InputProps = {| |}; export function Input(props: InputProps): Node { - di(useTheme); const [style] = useTheme(); return ( diff --git a/examples/flow/components/label.js b/examples/flow/components/label.js index e40ac8e..25dfdc0 100644 --- a/examples/flow/components/label.js +++ b/examples/flow/components/label.js @@ -1,6 +1,5 @@ // @flow import React, { useState, type Node } from 'react'; -import { di } from 'react-magnetic-di'; type UseThemeState = { color: string }; export function useTheme(): $Call { @@ -12,7 +11,6 @@ type LabelProps = {| |}; export function Label({ children = 'Label' }: LabelProps): Node { - di(useTheme); const [style] = useTheme(); return
{children}
; } diff --git a/examples/flow/components/section.js b/examples/flow/components/section.js index 5ab976d..5c2787d 100644 --- a/examples/flow/components/section.js +++ b/examples/flow/components/section.js @@ -1,6 +1,5 @@ // @flow import React, { Component, type Node } from 'react'; -import { di } from 'react-magnetic-di'; import { Label } from './label'; import { Input } from './input'; @@ -11,8 +10,6 @@ type Props = {| export class Section extends Component { render(): Node { - di(Input, Label); - const { title } = this.props; return (
diff --git a/examples/global/index.ts b/examples/global/index.ts index e772ae6..3269b62 100644 --- a/examples/global/index.ts +++ b/examples/global/index.ts @@ -1,17 +1,13 @@ -import { di } from 'react-magnetic-di'; - type Data = { data: number[] }; export const fetchApi = async (): Promise => ({ data: [1, 2] }); export const processApiData = (data: Data['data']) => data.map((v) => v * v); export function transformer(response: Data) { - di(processApiData); return processApiData(response.data); } export async function apiHandler() { - di(fetchApi, transformer); const data = await fetchApi(); return transformer(data); } diff --git a/examples/typescript/components/input.tsx b/examples/typescript/components/input.tsx index 831f3ca..1db9d75 100644 --- a/examples/typescript/components/input.tsx +++ b/examples/typescript/components/input.tsx @@ -1,6 +1,5 @@ // @flow import React, { useState } from 'react'; -import { di } from 'react-magnetic-di'; type UseThemeState = { color: string }; export function useTheme() { @@ -12,7 +11,6 @@ export type InputProps = { }; export function Input(props: InputProps) { - di(useTheme); const [style] = useTheme(); return ( diff --git a/examples/typescript/components/label.tsx b/examples/typescript/components/label.tsx index 363c6cb..9065148 100644 --- a/examples/typescript/components/label.tsx +++ b/examples/typescript/components/label.tsx @@ -1,6 +1,4 @@ -// @flow import React, { type ReactNode, useState } from 'react'; -import { di } from 'react-magnetic-di'; type UseThemeState = { color: string }; export function useTheme() { @@ -12,7 +10,6 @@ type LabelProps = { }; export function Label({ children = 'Label' }: LabelProps) { - di(useTheme); const [style] = useTheme(); return
{children}
; } diff --git a/examples/typescript/components/section.tsx b/examples/typescript/components/section.tsx index f661868..5f85039 100644 --- a/examples/typescript/components/section.tsx +++ b/examples/typescript/components/section.tsx @@ -1,6 +1,4 @@ -// @flow import React, { Component } from 'react'; -import { di } from 'react-magnetic-di'; import { Label } from './label'; import { Input } from './input'; @@ -11,8 +9,6 @@ type Props = { export class Section extends Component { render() { - di(Input, Label); - const { title } = this.props; return (
diff --git a/src/index.js b/src/index.js index 2f1f42a..3529022 100644 --- a/src/index.js +++ b/src/index.js @@ -1,5 +1,5 @@ export { di } from './react/consumer'; -export { runWithDi } from './react/global'; +export { runWithDi, globalDi as unstable_globalDi } from './react/global'; export { DiProvider, withDi } from './react/provider'; export { injectable } from './react/injectable'; export { debug } from './react/utils';