Skip to content

Commit

Permalink
Merge a108a82 into 9edba30
Browse files Browse the repository at this point in the history
  • Loading branch information
marvinhagemeister committed Apr 22, 2019
2 parents 9edba30 + a108a82 commit 4dd05e8
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 10 deletions.
21 changes: 11 additions & 10 deletions debug/src/debug.js
Expand Up @@ -130,19 +130,19 @@ export function initDebug() {
options.diffed = (vnode) => {
if (vnode._component && vnode._component.__hooks) {
let hooks = vnode._component.__hooks;
if (hooks._list.length > 0) {
hooks._list.forEach(hook => {
if (hook._callback && (!hook._args || !Array.isArray(hook._args))) {
console.warn(
`In ${vnode.type.name || vnode.type} you are calling useMemo/useCallback without passing arguments.\n` +
`This is a noop since it will not be able to memoize, it will execute it every render.`
);
}
});
}
hooks._list.forEach(hook => {
if (hook._callback && (!hook._args || !Array.isArray(hook._args))) {
/* istanbul ignore next */
console.warn(
`In ${vnode.type.name || vnode.type} you are calling useMemo/useCallback without passing arguments.\n` +
`This is a noop since it will not be able to memoize, it will execute it every render.`
);
}
});
if (hooks._pendingEffects.length > 0) {
hooks._pendingEffects.forEach((effect) => {
if (!effect._args || !Array.isArray(effect._args)) {
/* istanbul ignore next */
throw new Error('You should provide an array of arguments as the second argument to the "useEffect" hook.\n\n' +
'Not doing so will invoke this effect on every render.\n\n' +
'This effect can be found in the render of ' + (vnode.type.name || vnode.type) + '.');
Expand All @@ -152,6 +152,7 @@ export function initDebug() {
if (hooks._pendingLayoutEffects.length > 0) {
hooks._pendingLayoutEffects.forEach((layoutEffect) => {
if (!layoutEffect._args || !Array.isArray(layoutEffect._args)) {
/* istanbul ignore next */
throw new Error('You should provide an array of arguments as the second argument to the "useEffect" hook.\n\n' +
'Not doing so will invoke this effect on every render.\n\n' +
'This effect can be found in the render of ' + (vnode.type.name || vnode.type) + '.');
Expand Down
14 changes: 14 additions & 0 deletions debug/test/browser/debug.test.js
Expand Up @@ -77,6 +77,11 @@ describe('debug', () => {
expect(fn).to.throw(/createElement/);
});

it('should print an error on invalid object component', () => {
let fn = () => render(h({}), scratch);
expect(fn).to.throw(/createElement/);
});

it('should throw an error when using a hook outside a render', () => {
class App extends Component {
componentWillMount() {
Expand Down Expand Up @@ -172,6 +177,15 @@ describe('debug', () => {
expect(warnings.length).to.equal(2);
});

it('should warn when non-array args is passed', () => {
const App = () => {
const foo = useMemo(() => 'foo', 12);
return <p>{foo}</p>;
};
render(<App />, scratch);
expect(warnings[0]).to.match(/without passing arguments/);
});

it('should print an error on invalid refs', () => {
let fn = () => render(<div ref="a" />, scratch);
expect(fn).to.throw(/createRef/);
Expand Down
2 changes: 2 additions & 0 deletions hooks/src/index.js
Expand Up @@ -201,6 +201,7 @@ export function useDebugValue(value, formatter) {
* Invoke a component's pending effects after the next frame renders
* @type {(component: import('./internal').Component) => void}
*/
/* istanbul ignore next */
let afterPaint = () => {};

/**
Expand All @@ -219,6 +220,7 @@ function scheduleFlushAfterPaint() {
setTimeout(flushAfterPaintEffects, 0);
}

/* istanbul ignore else */
if (typeof window !== 'undefined') {
afterPaint = (component) => {
if (!component._afterPaintQueued && (component._afterPaintQueued = true) && afterPaintEffects.push(component) === 1) {
Expand Down
16 changes: 16 additions & 0 deletions hooks/test/browser/useImperativeHandle.test.js
Expand Up @@ -49,4 +49,20 @@ describe('useImperativeHandle', () => {
expect(ref.current).to.have.property('test');
expect(ref.current.test()).to.equal('test1');
});

it('should not update ref when args have not changed', () => {
let ref;

function Comp() {
ref = useRef({});
useImperativeHandle(ref, () => ({ test: () => 'test' }), [1]);
return <p>Test</p>;
}

render(<Comp />, scratch);
expect(ref.current.test()).to.equal('test');

render(<Comp />, scratch);
expect(ref.current.test()).to.equal('test');
});
});
1 change: 1 addition & 0 deletions karma.conf.js
Expand Up @@ -134,6 +134,7 @@ module.exports = function(config) {
exclude: [
// Default config
'coverage/**',
'dist/**',
'test/**',
'test{,-*}.js',
'**/*.test.js',
Expand Down

0 comments on commit 4dd05e8

Please sign in to comment.