Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Features
- Add default child a11y behavior to `Menu` related behaviors @silviuavram ([#1282](https://github.com/stardust-ui/react/pull/1282))
- `Ref` component extracted to a `@stardust-ui/react-component-ref` @layershifter ([#1281](https://github.com/stardust-ui/react/pull/1281))
- added `isRefObject()`, `toRefObject()` utils for React refs @layershifter ([#1281](https://github.com/stardust-ui/react/pull/1281))

<!--------------------------------[ v0.29.1 ]------------------------------- -->
## [v0.29.1](https://github.com/stardust-ui/react/tree/v0.29.1) (2019-05-01)
Expand Down
1 change: 1 addition & 0 deletions packages/internal-tooling/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
},
"files": [
"babel",
"eslint",
"jest"
],
"publishConfig": {
Expand Down
3 changes: 3 additions & 0 deletions packages/react-component-ref/.babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"extends": "@stardust-ui/internal-tooling/babel"
}
5 changes: 5 additions & 0 deletions packages/react-component-ref/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
...require('@stardust-ui/internal-tooling/jest'),
name: 'react-component-ref',
moduleNameMapper: require('lerna-alias').jest(),
}
37 changes: 37 additions & 0 deletions packages/react-component-ref/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"name": "@stardust-ui/react-component-ref",
"description": "A set of components and utils to deal with React refs.",
"version": "0.29.1",
"author": "Oleksandr Fediashov <olfedias@microsoft.com>",
"bugs": "https://github.com/stardust-ui/react/issues",
"dependencies": {
"@stardust-ui/react-proptypes": "^0.29.1",
"prop-types": "^15.6.1",
"react-is": "^16.6.3"
},
"devDependencies": {
"@stardust-ui/internal-tooling": "^0.29.1",
"lerna-alias": "^3.0.3-0"
},
"files": [
"dist"
],
"homepage": "https://github.com/stardust-ui/react/tree/master/packages/react-component-ref",
"jsnext:main": "dist/es/index.js",
"license": "MIT",
"main": "dist/commonjs/index.js",
"module": "dist/es/index.js",
"peerDependencies": {
"react": "^16.8.0",
"react-dom": "^16.8.0"
},
"publishConfig": {
"access": "public"
},
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI @miroslavstastny publishConfig is defined

"repository": "stardust-ui/react.git",
"scripts": {
"build": "cross-env TS_NODE_PROJECT=../../tsconfig.json gulp bundle:package:no-umd --package react-component-ref"
},
"sideEffects": false,
"types": "dist/es/index.d.ts"
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import * as customPropTypes from '@stardust-ui/react-proptypes'
import * as PropTypes from 'prop-types'
import * as React from 'react'
import { isForwardRef } from 'react-is'
import * as ReactIs from 'react-is'
Copy link
Member Author

Choose a reason for hiding this comment

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

nit: To be common with other imports


import { ChildrenComponentProps } from '../../lib'
import { ReactProps } from '../../types'
import RefFindNode from './RefFindNode'
import RefForward from './RefForward'

export interface RefProps extends ChildrenComponentProps<React.ReactElement<any>> {
export interface RefProps {
children: React.ReactElement<any>

/**
* Called when a child component will be mounted or updated.
*
Expand All @@ -17,11 +17,11 @@ export interface RefProps extends ChildrenComponentProps<React.ReactElement<any>
innerRef: React.Ref<any>
}

const Ref: React.FunctionComponent<ReactProps<RefProps>> = props => {
const Ref: React.FunctionComponent<RefProps> = props => {
const { children, innerRef } = props

const child = React.Children.only(children)
const ElementType = isForwardRef(child) ? RefForward : RefFindNode
const ElementType = ReactIs.isForwardRef(child) ? RefForward : RefFindNode

return <ElementType innerRef={innerRef}>{child}</ElementType>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ import * as PropTypes from 'prop-types'
import * as React from 'react'
import * as ReactDOM from 'react-dom'

import { ChildrenComponentProps } from '../../lib/commonPropInterfaces'
import handleRef from '../../lib/handleRef'
import handleRef from './handleRef'

export interface RefFindNodeProps {
children: React.ReactElement<any>

export interface RefFindNodeProps extends ChildrenComponentProps<React.ReactElement<any>> {
/**
* Called when a child component will be mounted or updated.
*
Expand All @@ -20,10 +21,10 @@ export default class RefFindNode extends React.Component<RefFindNodeProps> {

static propTypes = {
children: PropTypes.element.isRequired,
innerRef: customPropTypes.ref,
innerRef: customPropTypes.ref.isRequired as PropTypes.Validator<React.Ref<any>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

}

prevNode: Node = null
prevNode: Node | null = null

componentDidMount() {
this.prevNode = ReactDOM.findDOMNode(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ import * as customPropTypes from '@stardust-ui/react-proptypes'
import * as PropTypes from 'prop-types'
import * as React from 'react'

import { ChildrenComponentProps } from '../../lib/commonPropInterfaces'
import handleRef from '../../lib/handleRef'
import handleRef from './handleRef'

export interface RefForwardProps {
children: React.ReactElement<any>

export interface RefForwardProps extends ChildrenComponentProps<React.ReactElement<any>> {
/**
* Called when a child component will be mounted or updated.
*
Expand All @@ -19,7 +20,7 @@ export default class RefForward extends React.Component<RefForwardProps> {

static propTypes = {
children: PropTypes.element.isRequired,
innerRef: customPropTypes.ref,
innerRef: customPropTypes.ref.isRequired as PropTypes.Validator<React.Ref<any>>,
}

private handleRefOverride = (node: HTMLElement) => {
Expand Down
30 changes: 30 additions & 0 deletions packages/react-component-ref/src/handleRef.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import * as React from 'react'

/**
* The function that correctly handles passing refs.
*
* @param ref An ref object or function
* @param node A node that should be passed by ref
*/
const handleRef = <N>(ref: React.Ref<N> | undefined, node: N) => {
if (process.env.NODE_ENV !== 'production') {
if (typeof ref === 'string') {
throw new Error(
'We do not support refs as string, this is a legacy API and will be likely to be removed in one of the future releases of React.',
)
}
}

if (typeof ref === 'function') {
ref(node)
return
}

if (ref !== null && typeof ref === 'object') {
// The `current` property is defined as readonly, however it's a valid way because
// `ref` is a mutable object
;(ref as React.MutableRefObject<N>).current = node
Copy link
Member Author

Choose a reason for hiding this comment

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

Only this line was changed: we don't need @ts-ignore more because MutableRefObject was introduced to typings

}
}

export default handleRef
7 changes: 7 additions & 0 deletions packages/react-component-ref/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export { default as handleRef } from './handleRef'
export { default as isRefObject } from './isRefObject'
export { default as toRefObject } from './toRefObject'

export { default as Ref, RefProps } from './Ref'
export { default as RefFindNode } from './RefFindNode'
export { default as RefForward } from './RefForward'
10 changes: 10 additions & 0 deletions packages/react-component-ref/src/isRefObject.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import * as React from 'react'

/**
* Check that the passed object is a valid React ref object.
*/
const isRefObject = (ref: any): ref is React.RefObject<any> =>
// https://github.com/facebook/react/blob/v16.8.2/packages/react-reconciler/src/ReactFiberCommitWork.js#L665
ref !== null && typeof ref === 'object' && ref.hasOwnProperty('current')

export default isRefObject
27 changes: 27 additions & 0 deletions packages/react-component-ref/src/toRefObject.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import * as React from 'react'

const nullRefObject: React.RefObject<null> = { current: null }
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the user uses reference like this in the following way: ref.current.focus()? How is it safe to have a nullRefObject?

Copy link
Member Author

Choose a reason for hiding this comment

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

It can be null by typings:

    interface RefObject<T> {
        readonly current: T | null;
    }
    // Mutable it used in hooks an initial value is obviously defined
    // React.useRef(null)
    interface MutableRefObject<T> {
        current: T;
    }


// A map of created ref objects to provide memoization.
const refObjects = new WeakMap<Node, React.RefObject<Node>>()

/**
* Creates a React ref object from existing DOM node.
*/
const toRefObject = <T extends Node>(node: T): React.RefObject<T> => {
// A "null" is not valid key for a WeakMap
if (node === null) {
return nullRefObject
}

if (refObjects.has(node)) {
return refObjects.get(node) as React.RefObject<T>
}

const refObject: React.RefObject<T> = { current: node }
refObjects.set(node, refObject)

return refObject
}

export default toRefObject
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { Ref, RefFindNode, RefForward } from '@stardust-ui/react-component-ref'
import { shallow } from 'enzyme'
import * as React from 'react'

import Ref from 'src/components/Ref/Ref'
import RefFindNode from 'src/components/Ref/RefFindNode'
import RefForward from 'src/components/Ref/RefForward'
import { CompositeClass, ForwardedRef } from './fixtures'

describe('Ref', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { RefFindNode } from '@stardust-ui/react-component-ref'
import { mount } from 'enzyme'
import * as React from 'react'

import RefFindNode from 'src/components/Ref/RefFindNode'
import { CompositeClass, CompositeFunction, DOMClass, DOMFunction } from './fixtures'

const testInnerRef = Component => {
const testInnerRef = (Component: React.ElementType) => {
const innerRef = jest.fn()
const node = mount(
<RefFindNode innerRef={innerRef}>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { RefForward } from '@stardust-ui/react-component-ref'
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like that we have inconsistency between the imports in these tests vs the imports in the components' tests. Can we change this import or is there a reason why it is like this?

Copy link
Member Author

Choose a reason for hiding this comment

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

We want to test public APIs and get rid of src alias: it creates issues with IDE and works only in Jest/Webpack.

/cc @kuzhelov

import { mount } from 'enzyme'
import * as React from 'react'

import RefForward from 'src/components/Ref/RefForward'
import { ForwardedRef } from './fixtures'

describe('RefForward', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import * as React from 'react'

export const DOMFunction = props => <div {...props} id="node" />
export const DOMFunction: React.FunctionComponent = props => <div {...props} id="node" />

export const CompositeFunction = props => <DOMFunction {...props} />
export const CompositeFunction: React.FunctionComponent = props => <DOMFunction {...props} />

export class DOMClass extends React.Component {
render() {
Expand All @@ -18,6 +18,6 @@ export class CompositeClass extends React.Component {

export const ForwardedRef = React.forwardRef<HTMLButtonElement>((props, ref) => (
<div>
<button ref={ref} />
<button {...props} ref={ref} />
</div>
))
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { handleRef } from '@stardust-ui/react-component-ref'
Copy link
Contributor

Choose a reason for hiding this comment

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

See the prev comment for the import

import * as React from 'react'
import handleRef from 'src/lib/handleRef'

describe('handleRef', () => {
it('throws an error when "ref" is string', () => {
Expand Down
11 changes: 11 additions & 0 deletions packages/react-component-ref/test/isRefObject-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { isRefObject } from '@stardust-ui/react-component-ref'

describe('isRefObject', () => {
it('checks for a valid param', () => {
expect(isRefObject(document.createElement('div'))).toBe(false)
expect(isRefObject(null)).toBe(false)

expect(isRefObject({ current: document.createElement('div') })).toBe(true)
expect(isRefObject({ current: null })).toBe(true)
})
})
19 changes: 19 additions & 0 deletions packages/react-component-ref/test/toRefObject-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { toRefObject } from '@stardust-ui/react-component-ref'

describe('toRefObject', () => {
it('creates an ref object from an input', () => {
const node = document.createElement('div')
expect(toRefObject(node)).toHaveProperty('current', node)
})

it('handles "null" as input', () => {
expect(toRefObject(null)).toHaveProperty('current', null)
})

it('returned object is memoized', () => {
const node = document.createElement('div')
const refObject = toRefObject(node)

expect(toRefObject(node)).toBe(refObject)
})
})
11 changes: 11 additions & 0 deletions packages/react-component-ref/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"extends": "../../build/tsconfig.common",
"compilerOptions": {
"noImplicitReturns": true,
"noImplicitThis": true,
"noImplicitAny": true,
"noUnusedParameters": true,
"strictNullChecks": true
},
"include": ["src", "test"]
}
1 change: 1 addition & 0 deletions packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"dependencies": {
"@stardust-ui/react-component-event-listener": "^0.29.1",
"@stardust-ui/react-component-nesting-registry": "^0.29.1",
"@stardust-ui/react-component-ref": "^0.29.1",
"@stardust-ui/react-proptypes": "^0.29.1",
"classnames": "^2.2.5",
"css-shorthand-expand": "^1.2.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Ref } from '@stardust-ui/react-component-ref'
import * as customPropTypes from '@stardust-ui/react-proptypes'
import * as _ from 'lodash'
import * as PropTypes from 'prop-types'
Expand All @@ -20,7 +21,6 @@ import Button, { ButtonProps } from '../Button/Button'
import Box, { BoxProps } from '../Box/Box'
import Header from '../Header/Header'
import Portal from '../Portal/Portal'
import Ref from '../Ref/Ref'
import Flex from '../Flex/Flex'

export interface DialogProps
Expand Down
3 changes: 1 addition & 2 deletions packages/react/src/components/Dropdown/Dropdown.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { handleRef, Ref } from '@stardust-ui/react-component-ref'
import * as customPropTypes from '@stardust-ui/react-proptypes'
import * as React from 'react'
import * as PropTypes from 'prop-types'
Expand Down Expand Up @@ -27,11 +28,9 @@ import {
AutoControlledComponent,
RenderResultConfig,
commonPropTypes,
handleRef,
UIComponentProps,
} from '../../lib'
import List from '../List/List'
import Ref from '../Ref/Ref'
import DropdownItem, { DropdownItemProps } from './DropdownItem'
import DropdownSelectedItem, { DropdownSelectedItemProps } from './DropdownSelectedItem'
import DropdownSearchInput, { DropdownSearchInputProps } from './DropdownSearchInput'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Ref } from '@stardust-ui/react-component-ref'
import * as customPropTypes from '@stardust-ui/react-proptypes'
import * as React from 'react'
import * as PropTypes from 'prop-types'
Expand All @@ -10,7 +11,6 @@ import { createShorthandFactory, UIComponent, RenderResultConfig, commonPropType
import Icon, { IconProps } from '../Icon/Icon'
import Image from '../Image/Image'
import Label from '../Label/Label'
import Ref from '../Ref/Ref'
import Box from '../Box/Box'

export interface DropdownSelectedItemSlotClassNames {
Expand Down
Loading