Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add react-is package #12199

Merged
merged 19 commits into from
Feb 11, 2018
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
83 changes: 83 additions & 0 deletions packages/react-is/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# `react-is`

This package allows you to test arbitrary values and see if they're a particular React type, e.g. React Elements.

## Installation

```sh
# Yarn
yarn add react-is

# NPM
npm install react-is --save
```

## Usage

### AsyncMode

```js
import React from "react";
import * as ReactIs from 'react-is';

ReactIs.isAsyncMode(<React.unstable_AsyncMode />); // true
ReactIs.typeOf(<React.unstable_AsyncMode />) === ReactIs.AsyncMode; // true
```

### Context

```js
import React from "react";
import * as ReactIs from 'react-is';

const ThemeContext = React.createContext("blue");

ReactIs.isContextConsumer(<ThemeContext.Consumer />); // true
ReactIs.isContextProvider(<ThemeContext.Provider />); // true
ReactIs.typeOf(<ThemeContext.Provider />) === ReactIs.ContextProvider; // true
ReactIs.typeOf(<ThemeContext.Consumer />) === ReactIs.ContextConsumer; // true
```

### Element

```js
import React from "react";
import * as ReactIs from 'react-is';

ReactIs.isElement(<div />); // true
ReactIs.typeOf(<div />) === ReactIs.Element; // true
```

### Fragment

```js
import React from "react";
import * as ReactIs from 'react-is';

ReactIs.isFragment(<></>); // true
ReactIs.typeOf(<></>) === ReactIs.Fragment; // true
```

### Portal

```js
import React from "react";
import ReactDOM from "react-dom";
import * as ReactIs from 'react-is';

const div = document.createElement("div");
const portal = ReactDOM.createPortal(<div />, div);

ReactIs.isPortal(portal); // true
ReactIs.typeOf(portal) === ReactIs.Portal; // true
```

### StrictMode

```js
import React from "react";
import * as ReactIs from 'react-is';

ReactIs.isStrictMode(<React.StrictMode />); // true
ReactIs.typeOf(<React.StrictMode />) === ReactIs.StrictMode; // true
```
12 changes: 12 additions & 0 deletions packages/react-is/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

'use strict';

export * from './src/ReactIs';
7 changes: 7 additions & 0 deletions packages/react-is/npm/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

if (process.env.NODE_ENV === 'production') {
module.exports = require('./cjs/react-is.production.min.js');
} else {
module.exports = require('./cjs/react-is.development.js');
}
23 changes: 23 additions & 0 deletions packages/react-is/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"name": "react-is",
"version": "16.3.0-alpha.0",
"description": "Brand checking of React Elements.",
"main": "index.js",
"repository": "facebook/react",
"keywords": ["react"],
"license": "MIT",
"bugs": {
"url": "https://github.com/facebook/react/issues"
},
"homepage": "https://reactjs.org/",
"peerDependencies": {
"react": "^16.0.0 || 16.3.0-alpha.0"
},
"files": [
"LICENSE",
"README.md",
"index.js",
"cjs/",
"umd/"
]
}
105 changes: 105 additions & 0 deletions packages/react-is/src/ReactIs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

'use strict';

import {
REACT_ASYNC_MODE_TYPE,
REACT_CONTEXT_TYPE,
REACT_ELEMENT_TYPE,
REACT_FRAGMENT_TYPE,
REACT_PORTAL_TYPE,
REACT_PROVIDER_TYPE,
REACT_STRICT_MODE_TYPE,
} from 'shared/ReactSymbols';

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

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

// 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;
}

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;
}

maybeType = getTypeOf(object);
switch (maybeType) {
case REACT_ELEMENT_TYPE:
case REACT_PORTAL_TYPE:
return maybeType;
}
}
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;
export const ContextConsumer = REACT_CONTEXT_TYPE;
export const ContextProvider = REACT_PROVIDER_TYPE;
export const Element = REACT_ELEMENT_TYPE;
export const Fragment = REACT_FRAGMENT_TYPE;
export const Portal = REACT_PORTAL_TYPE;
export const StrictMode = REACT_STRICT_MODE_TYPE;

export function isAsyncMode(object: any) {
return (
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.

Seems like all of these could be rewritten as typeof() comparisons to ensure 1:1 check parity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a lot of back and forth on a PR, and the latest round seems entirely like nits. Seems a bit inefficient.

WRT using typeOf here- Either way should be fine. It's just a subjective preference, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PS using typeOf in isElement didn't work, because the return type was overly specific. I didn't feel strongly about it, but I opted out of using typeOf director for that reason.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't agree that repeating the same checks many times is a nit. Generally we avoid this everywhere else in the codebase. (If you're referring to the previous comment.)

Yes, they probably don't make a difference in a large scheme of things, but if we can make it more efficient, why not do it?

For this comment, I'm not sure. The issue I noted in #12199 (comment) was a correctness issue. It was fixed in b22a7b7. The fix required changes in two places. My concern is that it's confusing to remember both those places need to contain this logic. When we add a new type it won't be obvious which ones of those get* functions need to be called for is* to work consistently. Expressing this via typeof() solves this problem by consolidating that logic in one place.

Copy link
Collaborator

Choose a reason for hiding this comment

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

PS using typeOf in isElement didn't work, because the return type was overly specific. I didn't feel strongly about it, but I opted out of using typeOf director for that reason.

You probably considered it but another way to express this would be

switch (typeof(obj)) {
  case Element:
  case ContextConsumer:
  case ContextProvider:
    return true;
  default:
    return false;
}

I agree there's no clear winner in this particular case though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, they probably don't make a difference in a large scheme of things, but if we can make it more efficient, why not do it?

I don't think the specific comment this is in response to is really more efficient one way or another, in terms of runtime efficiency. And the complexity/code-size difference is minimal. This is why I said it felt like a nit.

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.

See my response below in #12199 (comment). Again I agree it probably doesn't matter in practice. But since it's a core utility and it's easy for us to minimize property access I don't see a reason not to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, the comment we're responding to is the implementation of isAsyncMode, so the runtime path of typeOf didn't seem relevant since it wasn't being used (just the helper getter functions).

It's fine. I'm about to push the change you suggested.

getType(object) === REACT_ASYNC_MODE_TYPE &&
getTypeOf(object) === REACT_ELEMENT_TYPE
);
}
export function isContextConsumer(object: any) {
return getTypeTypeOf(object) === REACT_CONTEXT_TYPE;
}
export function isContextProvider(object: any) {
return getTypeTypeOf(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
);
}
export function isPortal(object: any) {
return getTypeOf(object) === REACT_PORTAL_TYPE;
}
export function isStrictMode(object: any) {
return (
getType(object) === REACT_STRICT_MODE_TYPE &&
getTypeOf(object) === REACT_ELEMENT_TYPE
);
}
90 changes: 90 additions & 0 deletions packages/react-is/src/__tests__/ReactIs-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
*/

'use strict';

let React;
let ReactDOM;
let ReactIs;

describe('ReactIs', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
ReactIs = require('react-is');
});

it('should identify async mode', () => {
expect(ReactIs.typeOf(<React.unstable_AsyncMode />)).toBe(
ReactIs.AsyncMode,
);
expect(ReactIs.isAsyncMode(<React.unstable_AsyncMode />)).toBe(true);
expect(ReactIs.isAsyncMode({type: ReactIs.AsyncMode})).toBe(false);
expect(ReactIs.isAsyncMode(<React.StrictMode />)).toBe(false);
expect(ReactIs.isAsyncMode(<div />)).toBe(false);
});

it('should identify context consumers', () => {
const Context = React.createContext(false);
expect(ReactIs.typeOf(<Context.Consumer />)).toBe(ReactIs.ContextConsumer);
expect(ReactIs.isContextConsumer(<Context.Consumer />)).toBe(true);
expect(ReactIs.isContextConsumer(<Context.Provider />)).toBe(false);
expect(ReactIs.isContextConsumer(<div />)).toBe(false);
});

it('should identify context providers', () => {
const Context = React.createContext(false);
expect(ReactIs.typeOf(<Context.Provider />)).toBe(ReactIs.ContextProvider);
expect(ReactIs.isContextProvider(<Context.Provider />)).toBe(true);
expect(ReactIs.isContextProvider(<Context.Consumer />)).toBe(false);
expect(ReactIs.isContextProvider(<div />)).toBe(false);
});

it('should identify elements', () => {
expect(ReactIs.typeOf(<div />)).toBe(ReactIs.Element);
expect(ReactIs.isElement(<div />)).toBe(true);
expect(ReactIs.isElement('div')).toBe(false);
expect(ReactIs.isElement(true)).toBe(false);
expect(ReactIs.isElement(123)).toBe(false);

// It should also identify more specific types as elements
const Context = React.createContext(false);
expect(ReactIs.isElement(<Context.Provider />)).toBe(true);
expect(ReactIs.isElement(<Context.Consumer />)).toBe(true);
expect(ReactIs.isElement(<React.Fragment />)).toBe(true);
expect(ReactIs.isElement(<React.unstable_AsyncMode />)).toBe(true);
expect(ReactIs.isElement(<React.StrictMode />)).toBe(true);
});

it('should identify fragments', () => {
expect(ReactIs.typeOf(<React.Fragment />)).toBe(ReactIs.Fragment);
expect(ReactIs.isFragment(<React.Fragment />)).toBe(true);
expect(ReactIs.isFragment({type: ReactIs.Fragment})).toBe(false);
expect(ReactIs.isFragment('React.Fragment')).toBe(false);
expect(ReactIs.isFragment(<div />)).toBe(false);
expect(ReactIs.isFragment([])).toBe(false);
});

it('should identify portals', () => {
const div = document.createElement('div');
const portal = ReactDOM.createPortal(<div />, div);
expect(ReactIs.typeOf(portal)).toBe(ReactIs.Portal);
expect(ReactIs.isPortal(portal)).toBe(true);
expect(ReactIs.isPortal(div)).toBe(false);
});

it('should identify strict mode', () => {
expect(ReactIs.typeOf(<React.StrictMode />)).toBe(ReactIs.StrictMode);
expect(ReactIs.isStrictMode(<React.StrictMode />)).toBe(true);
expect(ReactIs.isStrictMode({type: ReactIs.StrictMode})).toBe(false);
expect(ReactIs.isStrictMode(<React.unstable_AsyncMode />)).toBe(false);
expect(ReactIs.isStrictMode(<div />)).toBe(false);
});
});
10 changes: 10 additions & 0 deletions scripts/rollup/bundles.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@ const bundles = [
global: 'ReactCallReturn',
externals: [],
},

/******* React Is *******/
{
label: 'react-is',
bundleTypes: [NODE_DEV, NODE_PROD, UMD_DEV, UMD_PROD],
moduleType: ISOMORPHIC,
entry: 'react-is',
global: 'ReactIs',
externals: [],
},
];

// Based on deep-freeze by substack (public domain)
Expand Down
28 changes: 28 additions & 0 deletions scripts/rollup/results.json
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,34 @@
"packageName": "react-reconciler",
"size": 41327,
"gzip": 13133
},
{
"filename": "react-is.development.js",
"bundleType": "NODE_DEV",
"packageName": "react-is",
"size": 3571,
"gzip": 1030
},
{
"filename": "react-is.production.min.js",
"bundleType": "NODE_PROD",
"packageName": "react-is",
"size": 1559,
"gzip": 624
},
{
"filename": "react-is.development.js",
"bundleType": "UMD_DEV",
"packageName": "react-is",
"size": 3760,
"gzip": 1086
},
{
"filename": "react-is.production.min.js",
"bundleType": "UMD_PROD",
"packageName": "react-is",
"size": 1641,
"gzip": 687
}
]
}
1 change: 1 addition & 0 deletions scripts/rollup/validate/eslintrc.umd.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ module.exports = {
// UMD wrapper code
// TODO: this is too permissive.
// Ideally we should only allow these *inside* the UMD wrapper.
exports: true,
module: true,
define: true,
require: true,
Expand Down