Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Adjusted typeOf() to be more efficient
  • Loading branch information
bvaughn committed Feb 11, 2018
commit ef0a90099eb8304028d0564312791de10a5d89af
94 changes: 39 additions & 55 deletions packages/react-is/src/ReactIs.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,50 +19,39 @@ import {
REACT_STRICT_MODE_TYPE,
} from 'shared/ReactSymbols';

// e.g. Fragments, StrictMode
function getType(object: any) {
return typeof object === 'object' && object !== null ? object.type : null;
}
export function typeOf(object: any) {
if (typeof object === 'object' && object !== null) {
const $$typeof = object.$$typeof;

// e.g. Elements, Portals
function getTypeOf(object: any) {
return typeof object === 'object' && object !== null ? object.$$typeof : null;
}
switch ($$typeof) {
case REACT_ELEMENT_TYPE:
const type = object.type;

// e.g. Context provider and consumer
function getTypeTypeOf(object: any) {
return typeof object === 'object' &&
object !== null &&
typeof object.type === 'object' &&
object.type !== null
? object.type.$$typeof
: null;
}
switch (type) {
case REACT_ASYNC_MODE_TYPE:
case REACT_FRAGMENT_TYPE:
case REACT_STRICT_MODE_TYPE:
return type;
default:
const $$typeofType =
typeof type === 'object' && type !== null
? type.$$typeof
: undefined;

export function typeOf(object: any) {
let maybeType = getType(object);
switch (maybeType) {
case REACT_ASYNC_MODE_TYPE:
case REACT_FRAGMENT_TYPE:
case REACT_STRICT_MODE_TYPE:
if (getTypeOf(object) === REACT_ELEMENT_TYPE) {
return maybeType;
}
}

maybeType = getTypeTypeOf(object);
switch (maybeType) {
case REACT_CONTEXT_TYPE:
case REACT_PROVIDER_TYPE:
return maybeType;
switch ($$typeofType) {
case REACT_CONTEXT_TYPE:
case REACT_PROVIDER_TYPE:
return $$typeofType;
default:
return $$typeof;
}
}
case REACT_PORTAL_TYPE:
return $$typeof;
}
}

maybeType = getTypeOf(object);
switch (maybeType) {
case REACT_ELEMENT_TYPE:
case REACT_PORTAL_TYPE:
return maybeType;
}
return undefined;
}
Copy link
Collaborator

@gaearon gaearon Feb 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the structure of this function a bit hard to follow. Is there any specific reason it is written this way?

It also seems like we're reading .type and compare it many times in pretty common cases like when the input is an element.

I would prefer to write it inline:

export function typeOf(object: any) {
  const $$typeof = getTypeOf(object);
  switch ($$typeof) {
    case REACT_ELEMENT_TYPE:
      const type = object.type;
      switch (type) {
        case REACT_ASYNC_MODE_TYPE:
        case REACT_FRAGMENT_TYPE:
        case REACT_STRICT_MODE_TYPE:
          return type;
        default:
          const $$typeofType = type.$$typeof;
          switch ($$typeofType) {
            case REACT_CONTEXT_TYPE:
            case REACT_PROVIDER_TYPE:
              return $$typeofType;
            default:
              return $$typeof;
          }
      }
    case REACT_PORTAL_TYPE:
      return $$typeof;
    default:
      return null;
  }
}

What do you think? IMO this makes the object structure a bit clearer, and avoids unnecessary property reads.

It also mirrors how we use it in practice (both Fiber and SSR) so it is helpful as a guide for "how do I handle all the cases".


export const AsyncMode = REACT_ASYNC_MODE_TYPE;
Expand All @@ -74,32 +63,27 @@ export const Portal = REACT_PORTAL_TYPE;
export const StrictMode = REACT_STRICT_MODE_TYPE;

export function isAsyncMode(object: any) {
return (
getType(object) === REACT_ASYNC_MODE_TYPE &&
getTypeOf(object) === REACT_ELEMENT_TYPE
);
return typeOf(object) === REACT_ASYNC_MODE_TYPE;
}
export function isContextConsumer(object: any) {
return getTypeTypeOf(object) === REACT_CONTEXT_TYPE;
return typeOf(object) === REACT_CONTEXT_TYPE;
}
export function isContextProvider(object: any) {
return getTypeTypeOf(object) === REACT_PROVIDER_TYPE;
return typeOf(object) === REACT_PROVIDER_TYPE;
}
export function isElement(object: any) {
return getTypeOf(object) === REACT_ELEMENT_TYPE;
}
export function isFragment(object: any) {
return (
getType(object) === REACT_FRAGMENT_TYPE &&
getTypeOf(object) === REACT_ELEMENT_TYPE
typeof object === 'object' &&
object !== null &&
object.$$typeof === REACT_ELEMENT_TYPE
);
}
export function isFragment(object: any) {
return typeOf(object) === REACT_FRAGMENT_TYPE;
}
export function isPortal(object: any) {
return getTypeOf(object) === REACT_PORTAL_TYPE;
return typeOf(object) === REACT_PORTAL_TYPE;
}
export function isStrictMode(object: any) {
return (
getType(object) === REACT_STRICT_MODE_TYPE &&
getTypeOf(object) === REACT_ELEMENT_TYPE
);
return typeOf(object) === REACT_STRICT_MODE_TYPE;
}
1 change: 1 addition & 0 deletions packages/react-is/src/__tests__/ReactIs-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ let ReactIs;
describe('ReactIs', () => {
beforeEach(() => {
jest.resetModules();

React = require('react');
ReactDOM = require('react-dom');
ReactIs = require('react-is');
Expand Down
16 changes: 8 additions & 8 deletions scripts/rollup/results.json
Original file line number Diff line number Diff line change
Expand Up @@ -403,29 +403,29 @@
"filename": "react-is.development.js",
"bundleType": "NODE_DEV",
"packageName": "react-is",
"size": 3571,
"gzip": 1030
"size": 3414,
"gzip": 1029
},
{
"filename": "react-is.production.min.js",
"bundleType": "NODE_PROD",
"packageName": "react-is",
"size": 1559,
"gzip": 624
"size": 1470,
"gzip": 616
},
{
"filename": "react-is.development.js",
"bundleType": "UMD_DEV",
"packageName": "react-is",
"size": 3760,
"gzip": 1086
"size": 3603,
"gzip": 1084
},
{
"filename": "react-is.production.min.js",
"bundleType": "UMD_PROD",
"packageName": "react-is",
"size": 1641,
"gzip": 687
"size": 1552,
"gzip": 680
}
]
}