Skip to content

Commit

Permalink
Merge pull request #81 from davidgilbertson/80-downgrade-error
Browse files Browse the repository at this point in the history
80 Downgrade throw Error to console.error
  • Loading branch information
davidgilbertson authored Mar 9, 2020
2 parents 63b0996 + 9edd1c6 commit f942283
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 21 deletions.
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-recollect",
"version": "4.0.2",
"version": "4.0.3",
"description": "Simple state management for react",
"keywords": [
"flux",
Expand Down
25 changes: 15 additions & 10 deletions src/proxyHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,12 @@ export const getHandlerForObject = <T extends Target>(
return {
get(target, prop) {
if (IS_PREV_STORE in target && state.currentComponent) {
throw Error(
`You are trying to read "${prop.toString()}" from the global store
while rendering a component. This could result in subtle bugs.
Instead, read from the store object passed as a prop to your component.`
console.error(
[
`You are trying to read "${prop.toString()}" from the global store `,
`while rendering a component. This could result in subtle bugs. `,
`Instead, read from the store object passed as a prop to your component.`,
].join('')
);
}

Expand Down Expand Up @@ -286,12 +288,15 @@ export const getHandlerForObject = <T extends Target>(

set(target, prop, value) {
if (state.currentComponent) {
throw Error(
`You are modifying the store during a render cycle. Don't do this.
You're setting "${prop.toString()}" to "${value}" somewhere; check the stack
trace below.
If you're changing the store in componentDidMount, wrap your code in a
setTimeout() to allow the render cycle to complete before changing the store.`
console.error(
[
`You are attempting to modify the store during a render cycle. `,
`(You're setting "${prop.toString()}" to "${value}" somewhere)\n`,
`This could result in subtle bugs. `,
`If you're changing the store in componentDidMount, wrap your `,
`code in a setTimeout() to allow the render cycle to complete `,
`before changing the store.`,
].join('')
);
}

Expand Down
4 changes: 2 additions & 2 deletions tests/integration/componentDidMount.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable max-classes-per-file */
import React, { Component } from 'react';
import { render } from '@testing-library/react';
import { expectToThrow } from '../testUtils';
import { expectToLogError } from '../testUtils';
import { collect, WithStoreProp } from '../../src';

const TestComponentBad = collect(
Expand Down Expand Up @@ -43,7 +43,7 @@ const TestComponentGood = collect(
);

it('should fail if setting the state during mounting', () => {
expectToThrow(() => {
expectToLogError(() => {
render(<TestComponentBad />);
});
});
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/readFromTwoStores.test.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import React from 'react';
import { store as globalStore, WithStoreProp } from '../../src';
import { collectAndRender, expectToThrow } from '../testUtils';
import { collectAndRender, expectToLogError } from '../testUtils';

it('should throw an error if I read from the wrong store', () => {
globalStore.meta = { title: 'Hello' };

expectToThrow(() => {
expectToLogError(() => {
collectAndRender(({ store }: WithStoreProp) => (
<div>
<p>{`${store.meta.title} from the store`}</p>
Expand Down
12 changes: 7 additions & 5 deletions tests/testUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,18 @@ export const collectAndRender = (Comp: React.ComponentType<any>) => {
export const propPathChanges = (handleChangeMock: jest.Mock) =>
handleChangeMock.mock.calls.map((call) => call[0].changedProps[0]);

export const expectToThrow = (func: () => void) => {
export const expectToLogError = (func: () => void) => {
// Even though the error is caught, it still gets printed to the console
// so we mock that out to avoid the wall of red text.
jest.spyOn(console, 'error');
const mockedConsole = mocked(console.error, true);
mockedConsole.mockImplementation(() => {});
const mockedConsoleError = mocked(console.error, true);
mockedConsoleError.mockImplementation(() => {});

expect(func).toThrow();
func();

mockedConsole.mockRestore();
expect(mockedConsoleError).toHaveBeenCalled();

mockedConsoleError.mockRestore();
};

export type TaskType = {
Expand Down

0 comments on commit f942283

Please sign in to comment.