Skip to content

Commit 4419434

Browse files
committed
Reset on empty raw data
1 parent fc484f1 commit 4419434

File tree

6 files changed

+105
-4
lines changed

6 files changed

+105
-4
lines changed

docs/callback-handling.review.md

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
For learning purposes I refactored a component into it's own NPM package. So it becomes reusable and the idea of React is fulfilled. You can find the NPM package here: https://github.com/openscript/react-dsv-import
2+
3+
When I tried to use the component I ran into a problem:
4+
5+
Cannot update a component (`SomeComponent`) while rendering a different component (`DSVImport$1`). To locate the bad setState() call inside `DSVImport$1`, follow the stack trace as described in https://fb.me/setstate-in-render
6+
7+
The problem is that my `DSVImport` component, which I split into its own NPM package, uses a context and if the data within this context changes the `onChange` callback is triggered. If the user of the `DSVImport` component sets another state within the callback, the error stated above arises.
8+
9+
The good news is, that I could fix the problem, but I would like you to review my [fix](https://github.com/openscript/react-dsv-import/commit/5ddb0daad7984123b2e51d7d96471c365d1957ed).
10+
11+
Before I was calling the `onChange` callback during the context update in the middleware:
12+
13+
export const createSimpleParserMiddleware = <T>(onChange?: (value: T[]) => void) => {
14+
return (state: State<T>, action: Actions<T>) => {
15+
let newState = reducer<T>(state, action);
16+
17+
if (action.type === 'setRaw') {
18+
const delimiter = detectDelimiterFromValue(action.raw);
19+
const parsed = parseData<T>(action.raw, state.columns, delimiter);
20+
if (onChange) {
21+
onChange(parsed);
22+
}
23+
newState = reducer<T>(state, { type: 'setParsed', parsed });
24+
}
25+
26+
return newState;
27+
};
28+
};
29+
30+
I've removed all this stuff and created a context consumer component, which calls the `onChange` callback within an `effect` (`useEffect`):
31+
32+
33+
interface EventListenerProps<T> {
34+
onChange?: (value: T[]) => void;
35+
}
36+
37+
const EventListener = <T extends { [key: string]: string }>(props: EventListenerProps<T>) => {
38+
const [context] = useDSVImport<T>();
39+
40+
useEffect(() => {
41+
if (context.parsed && props.onChange) {
42+
props.onChange(context.parsed);
43+
}
44+
});
45+
46+
return null;
47+
};
48+
49+
export interface Props<T> {
50+
onChange?: (value: T[]) => void;
51+
columns: ColumnsType<T>;
52+
}
53+
54+
export const DSVImport = <T extends { [key: string]: string }>(props: PropsWithChildren<Props<T>>) => {
55+
const DSVImportContext = getDSVImportContext<T>();
56+
const middleware = createSimpleParserMiddleware<T>();
57+
const initialValues: State<T> = { columns: props.columns };
58+
59+
return (
60+
<DSVImportContext.Provider value={useReducer(middleware, initialValues)}>
61+
<EventListener<T> onChange={props.onChange} />
62+
{props.children}
63+
</DSVImportContext.Provider>
64+
);
65+
};
66+
67+
68+
What do you think about this?

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"react"
88
],
99
"homepage": "https://openscript.github.io/react-dsv-import/",
10-
"version": "0.1.4",
10+
"version": "0.1.5",
1111
"main": "dist/index.js",
1212
"module": "dist/es/index.js",
1313
"types": "dist/index.d.ts",

src/DSVImport.stories.tsx

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React from 'react';
1+
import React, { useState } from 'react';
22
import { DSVImport, ColumnsType } from './';
33
import { action } from '@storybook/addon-actions';
44

@@ -23,3 +23,24 @@ export const BasicUsage = () => {
2323
);
2424
};
2525
BasicUsage.story = { name: 'Basic usage' };
26+
27+
export const UsingCallbacks = () => {
28+
const [state, setState] = useState<BasicType[]>([]);
29+
30+
const handleOnChange = (value: BasicType[]) => {
31+
onChangeAction(value);
32+
setState(value);
33+
};
34+
35+
return (
36+
<>
37+
<DSVImport<BasicType> columns={columns} onChange={handleOnChange}>
38+
<DSVImport.TextareaInput />
39+
<DSVImport.TablePreview />
40+
</DSVImport>
41+
<h2>Current state as JSON:</h2>
42+
{JSON.stringify(state)}
43+
</>
44+
);
45+
};
46+
UsingCallbacks.story = { name: 'Using callbacks with states' };

src/DSVImport.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const EventListener = <T extends { [key: string]: string }>(props: EventListener
1515
if (context.parsed && props.onChange) {
1616
props.onChange(context.parsed);
1717
}
18-
});
18+
}, [context.parsed]);
1919

2020
return null;
2121
};

src/middlewares/simpleParserMiddleware.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@ describe('simpleParserMiddleware', () => {
2121
expect(newState.parsed).toStrictEqual([{ forename: 'Max', surname: undefined, email: undefined }]);
2222
});
2323

24+
it('should set parsed data to an empty array if there is no raw data', () => {
25+
middleware(defaultState, { type: 'setRaw', raw: 'Max' });
26+
const newState = middleware(defaultState, { type: 'setRaw', raw: '' });
27+
28+
expect(newState.parsed).toStrictEqual([]);
29+
});
30+
2431
it('should detect the correct delimiter from raw data', () => {
2532
Object.values(Delimiter).forEach((d) => {
2633
const newState = middleware(defaultState, { type: 'setRaw', raw: rawData.replace(/!/g, d) });

src/middlewares/simpleParserMiddleware.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,12 @@ export const createSimpleParserMiddleware = <T>() => {
3434

3535
if (action.type === 'setRaw') {
3636
const delimiter = detectDelimiterFromValue(action.raw);
37-
const parsed = parseData<T>(action.raw, state.columns, delimiter);
37+
38+
let parsed: T[] = [];
39+
if (action.raw !== '') {
40+
parsed = parseData<T>(action.raw, state.columns, delimiter);
41+
}
42+
3843
newState = reducer<T>(state, { type: 'setParsed', parsed });
3944
}
4045

0 commit comments

Comments
 (0)