From 666b79989e9e687b3c2ca60a7311d716cf562167 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 21 Aug 2019 13:48:26 +0200 Subject: [PATCH] (chore) - remove useDevtoolsContext (#387) * (chore) - remove usage of useDevtoolsContext * (chore) - remove tests relate'd to useDevtoolsContext * (chore) - save size by not spreading context * (fix) - fix test by defaulting to an empty object * (fix) - ensure useMutation executes with an empty object as context to prevent potential crashes --- src/hooks/index.ts | 1 - src/hooks/useDevtoolsContext.test.ts | 117 --------------------------- src/hooks/useDevtoolsContext.ts | 50 ------------ src/hooks/useMutation.test.tsx | 10 --- src/hooks/useMutation.ts | 6 +- src/hooks/useQuery.spec.ts | 1 - src/hooks/useQuery.test.tsx | 13 --- src/hooks/useQuery.ts | 4 - src/hooks/useSubscription.test.tsx | 9 --- src/hooks/useSubscription.ts | 9 +-- 10 files changed, 4 insertions(+), 216 deletions(-) delete mode 100644 src/hooks/useDevtoolsContext.test.ts delete mode 100644 src/hooks/useDevtoolsContext.ts diff --git a/src/hooks/index.ts b/src/hooks/index.ts index f7c86247c3..58b57faae0 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -1,4 +1,3 @@ export * from './useMutation'; export * from './useQuery'; export * from './useSubscription'; -export * from './useDevtoolsContext'; diff --git a/src/hooks/useDevtoolsContext.test.ts b/src/hooks/useDevtoolsContext.test.ts deleted file mode 100644 index 6d2eebca90..0000000000 --- a/src/hooks/useDevtoolsContext.test.ts +++ /dev/null @@ -1,117 +0,0 @@ -import React, { Component } from 'react'; -import renderer from 'react-test-renderer'; -import { useDevtoolsContext } from './useDevtoolsContext'; - -it('retrieves the component name for function components', () => { - let hookResult; - - const TestFunComponent = () => { - hookResult = useDevtoolsContext(); - return null; - }; - - renderer.create(React.createElement(TestFunComponent)); - - expect(hookResult).toEqual({ - meta: { - source: 'TestFunComponent', - }, - }); -}); - -it('prefers displayNames', () => { - let hookResult; - - const TestFunComponent = () => { - hookResult = useDevtoolsContext(); - return null; - }; - - TestFunComponent.displayName = 'TestFun'; - - renderer.create(React.createElement(TestFunComponent)); - - expect(hookResult).toEqual({ - meta: { - source: 'TestFun', - }, - }); -}); - -it('retrieves the component name for function components using Query', () => { - let hookResult; - - // We're not using the actual query component here but one with the _same name_ - const Query = () => { - hookResult = useDevtoolsContext(); - return null; - }; - - const OuterFunComponent = () => { - return React.createElement(Query); - }; - - renderer.create(React.createElement(OuterFunComponent)); - - expect(hookResult).toEqual({ - meta: { - source: 'OuterFunComponent', - }, - }); -}); - -it('retrieves the component name for class components using Query', () => { - let hookResult; - - // We're not using the actual query component here but one with the _same name_ - const Query = () => { - hookResult = useDevtoolsContext(); - return null; - }; - - class OuterClassComponent extends Component { - render() { - return React.createElement(Query); - } - } - - renderer.create(React.createElement(OuterClassComponent)); - - expect(hookResult).toEqual({ - meta: { - source: 'OuterClassComponent', - }, - }); -}); - -it('looks at the parent component, not the parent fiber', () => { - let hookResult; - - // A context consumer/provider has a different Fiber type - const TestContext = React.createContext(''); - - // We're not using the actual query component here but one with the _same name_ - const Query = () => { - hookResult = useDevtoolsContext(); - return null; - }; - - class OuterClassComponent extends Component { - render() { - // Basically put one element "in the middle" of OuterClassComponent and Query - return React.createElement( - TestContext.Provider, - { value: 'test' }, - React.createElement(Query) - ); - } - } - - renderer.create(React.createElement(OuterClassComponent)); - - expect(hookResult).toEqual({ - meta: { - source: 'OuterClassComponent', - }, - }); -}); diff --git a/src/hooks/useDevtoolsContext.ts b/src/hooks/useDevtoolsContext.ts deleted file mode 100644 index dc257953c5..0000000000 --- a/src/hooks/useDevtoolsContext.ts +++ /dev/null @@ -1,50 +0,0 @@ -import * as React from 'react'; -import { OperationContext } from '../types'; - -const { - ReactCurrentOwner: CurrentOwner, -} = (React as any).__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED; - -// Is the Fiber a FunctionComponent, ClassComponent, or IndeterminateComponent -const isComponentFiber = (fiber: void | { tag: number }) => - fiber && (fiber.tag === 0 || fiber.tag === 1 || fiber.tag === 2); - -// Is the component one of ours (just a heuristic to avoid circular dependencies or flags) -const isInternalComponent = (Component: { name: string }) => - Component.name === 'Query' || - Component.name === 'Mutation' || - Component.name === 'Subscription'; - -const useDevtoolsContextImpl = (): Partial => { - return React.useMemo(() => { - let source = 'Component'; - - // Check whether the CurrentOwner is set - const owner = CurrentOwner.current; - if (owner !== null && isComponentFiber(owner)) { - let Component = owner.type; - - // If this is one of our own components then check the parent - if ( - isInternalComponent(Component) && - isComponentFiber(owner._debugOwner) - ) { - Component = owner._debugOwner.type; - } - - // Get the Component's name if it has one - if (typeof Component === 'function') { - source = Component.displayName || Component.name || source; - } - } - - return { meta: { source } }; - }, []); -}; - -/** Creates additional context values for serving metadata to devtools. */ -export const useDevtoolsContext: () => Partial | undefined = - // NOTE: We check for CurrentOwner in case it'll be unexpectedly changed in React's source - process.env.NODE_ENV !== 'production' && !!CurrentOwner - ? useDevtoolsContextImpl - : () => undefined; diff --git a/src/hooks/useMutation.test.tsx b/src/hooks/useMutation.test.tsx index 235add9d6e..f6732d851f 100644 --- a/src/hooks/useMutation.test.tsx +++ b/src/hooks/useMutation.test.tsx @@ -107,16 +107,6 @@ describe('on execute', () => { ); }); - it('calls executeMutation with source component info', () => { - renderer.create(); - act(() => { - execute(vars); - }); - expect(client.executeMutation.mock.calls[0][1]).toHaveProperty('meta', { - source: 'MutationUser', - }); - }); - it('can adjust context in executeMutation', () => { renderer.create(); act(() => { diff --git a/src/hooks/useMutation.ts b/src/hooks/useMutation.ts index 63f2abeaf9..b4c83cb71d 100644 --- a/src/hooks/useMutation.ts +++ b/src/hooks/useMutation.ts @@ -5,7 +5,6 @@ import { Context } from '../context'; import { OperationResult, OperationContext } from '../types'; import { CombinedError, createRequest } from '../utils'; import { useImmediateState } from './useImmediateState'; -import { useDevtoolsContext } from './useDevtoolsContext'; export interface UseMutationState { fetching: boolean; @@ -22,7 +21,6 @@ export type UseMutationResponse = [ export const useMutation = ( query: DocumentNode | string ): UseMutationResponse => { - const devtoolsContext = useDevtoolsContext(); const client = useContext(Context); const [state, setState] = useImmediateState>({ fetching: false, @@ -43,7 +41,7 @@ export const useMutation = ( const request = createRequest(query, variables as any); return pipe( - client.executeMutation(request, { ...devtoolsContext, ...context }), + client.executeMutation(request, context || {}), toPromise ).then(result => { const { data, error, extensions } = result; @@ -51,7 +49,7 @@ export const useMutation = ( return result; }); }, - [client, devtoolsContext, query, setState] + [client, query, setState] ); return [state, executeMutation]; diff --git a/src/hooks/useQuery.spec.ts b/src/hooks/useQuery.spec.ts index 8236254ded..00f3442efd 100644 --- a/src/hooks/useQuery.spec.ts +++ b/src/hooks/useQuery.spec.ts @@ -80,7 +80,6 @@ describe('useQuery', () => { variables: mockVariables, }, { - meta: { source: 'TestHook' }, requestPolicy: undefined, url: 'test', } diff --git a/src/hooks/useQuery.test.tsx b/src/hooks/useQuery.test.tsx index cd03726be0..2740be19f1 100644 --- a/src/hooks/useQuery.test.tsx +++ b/src/hooks/useQuery.test.tsx @@ -88,19 +88,6 @@ describe('on initial useEffect', () => { }) ); }); - - it('passes source component name to executeQuery', () => { - renderer.create(); - - expect(client.executeQuery).toBeCalledWith( - expect.any(Object), - expect.objectContaining({ - meta: { - source: 'QueryUser', - }, - }) - ); - }); }); describe('on subscription', () => { diff --git a/src/hooks/useQuery.ts b/src/hooks/useQuery.ts index ddd80b4e09..03bb45ddc9 100644 --- a/src/hooks/useQuery.ts +++ b/src/hooks/useQuery.ts @@ -4,7 +4,6 @@ import { pipe, subscribe } from 'wonka'; import { Context } from '../context'; import { OperationContext, RequestPolicy } from '../types'; import { CombinedError, noop } from '../utils'; -import { useDevtoolsContext } from './useDevtoolsContext'; import { useRequest } from './useRequest'; import { useImmediateEffect } from './useImmediateEffect'; import { useImmediateState } from './useImmediateState'; @@ -33,7 +32,6 @@ export type UseQueryResponse = [ export const useQuery = ( args: UseQueryArgs ): UseQueryResponse => { - const devtoolsContext = useDevtoolsContext(); const unsubscribe = useRef(noop); const client = useContext(Context); @@ -62,7 +60,6 @@ export const useQuery = ( pollInterval: args.pollInterval, ...args.context, ...opts, - ...devtoolsContext, }), subscribe(({ data, error, extensions }) => { setState({ fetching: false, data, error, extensions }); @@ -74,7 +71,6 @@ export const useQuery = ( args.requestPolicy, args.pollInterval, client, - devtoolsContext, request, setState, ] diff --git a/src/hooks/useSubscription.test.tsx b/src/hooks/useSubscription.test.tsx index 6d04306928..2a1db21ff7 100644 --- a/src/hooks/useSubscription.test.tsx +++ b/src/hooks/useSubscription.test.tsx @@ -62,14 +62,6 @@ describe('on initial useEffect', () => { expect.any(Object) ); }); - - it('passes source component info to executeSubscription', () => { - renderer.create(); - expect(client.executeSubscription).toBeCalledWith( - expect.any(Object), - expect.objectContaining({ meta: { source: 'SubscriptionUser' } }) - ); - }); }); it('should support setting context in useSubscription params', () => { @@ -85,7 +77,6 @@ it('should support setting context in useSubscription params', () => { }, { url: 'test', - meta: { source: 'SubscriptionUser' }, } ); }); diff --git a/src/hooks/useSubscription.ts b/src/hooks/useSubscription.ts index 25ea6843ea..8562c4148d 100644 --- a/src/hooks/useSubscription.ts +++ b/src/hooks/useSubscription.ts @@ -3,7 +3,6 @@ import { useCallback, useContext, useEffect, useRef } from 'react'; import { pipe, subscribe } from 'wonka'; import { Context } from '../context'; import { CombinedError, noop } from '../utils'; -import { useDevtoolsContext } from './useDevtoolsContext'; import { useRequest } from './useRequest'; import { useImmediateState } from './useImmediateState'; import { OperationContext } from '../types'; @@ -29,7 +28,6 @@ export const useSubscription = ( args: UseSubscriptionArgs, handler?: SubscriptionHandler ): UseSubscriptionResponse => { - const devtoolsContext = useDevtoolsContext(); const unsubscribe = useRef(noop); const client = useContext(Context); @@ -48,10 +46,7 @@ export const useSubscription = ( unsubscribe.current(); [unsubscribe.current] = pipe( - client.executeSubscription(request, { - ...devtoolsContext, - ...args.context, - }), + client.executeSubscription(request, args.context || {}), subscribe(({ data, error, extensions }) => { setState(s => ({ fetching: true, @@ -61,7 +56,7 @@ export const useSubscription = ( })); }) ); - }, [client, devtoolsContext, handler, request, setState, args.context]); + }, [client, handler, request, setState, args.context]); // Trigger subscription on query change // We don't use useImmediateEffect here as we have no way of