Skip to content

Commit ee415b3

Browse files
authored
Add safeguards for future Resource addition (#55)
When adding a new Resource correctly (see comments in the resources.ts file) the type system demands an entry for every components that does type-based pattern matching on the Resource discriminated union members Inspiration: https://medium.com/@fillopeter/pattern-matching-with-typescript-done-right-94049ddd671c#809c Limitations: - the solution doesn't check for excessive declarations / code handling a non-existing resource - casting, casting everywhere, but we'll deal with this later - two additional sets with Resource addition (aside from actually adding the Resource) I'm accepting the tradeof as adding Resources shouldn't be that common and there is a comment in place to facilitate doing that correctly. To the best of my knowledge this is the best that could be done It actually allowed to simplify many mechanisms either directly (moving the statically reflected type properties to the core domain) or indirectly (I've realised that initial Formik fields can be simply the resource object itself). As a side note: I'm having second thoughts about choosing TypeScript for this, as it clearly lacks the expressive power to implement such features properly
1 parent fe5e134 commit ee415b3

File tree

4 files changed

+56
-69
lines changed

4 files changed

+56
-69
lines changed

src/App.tsx

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { Text, Resource } from "./core/resources";
1+
// TODO: how to limit importing scope?
2+
import { Resource, Link, Text } from "./core/resources";
23
import { Tree, makeTree, TreeNode, NodeID, serializeTree, deserializeTree } from "./core/tree";
34

45
import ResourceEditor from "./ResourceEditor";
@@ -26,7 +27,8 @@ export default class App extends Component<{
2627
{
2728
super(props);
2829

29-
const ourTree = makeTree({content: "korzen", is_done: false});
30+
// TODO: get rid of the casting
31+
const ourTree = makeTree({__typename: "Text", content: "korzen", is_done: false} as Text);
3032

3133
this.state= {
3234
chosenNode: ourTree.root.id,
@@ -181,28 +183,17 @@ function displayTree(t: Tree<Resource>, chosenNode: NodeID): ReactTreeGraphNode
181183
{
182184
function _displayTree(t: TreeNode<Resource>): ReactTreeGraphNode
183185
{
184-
const resource_to_display = t.data;
185-
186-
let fuj;
187-
// TODO: make this properly extendiable - maybe delegate?
188-
if ("address" in resource_to_display)
189-
{
190-
fuj = resource_to_display.address;
191-
}
192-
else
193-
{
194-
fuj = (resource_to_display as Text).content;
195-
}
196-
197186
const selected = t.id === chosenNode ? {className: "selected"} : {}
187+
198188
const result: ReactTreeGraphNode = {
199-
name: fuj,
189+
name: displayResource(t.data),
200190
id: t.id,
201191
children: [],
202192
textProps: selected,
203193
circleProps: selected,
204194
}
205195

196+
// terminate recursion
206197
if (t.children.length !== 0)
207198
{
208199
result.children = t.children.map(_displayTree)
@@ -213,3 +204,13 @@ function displayTree(t: Tree<Resource>, chosenNode: NodeID): ReactTreeGraphNode
213204

214205
return _displayTree(t.root);
215206
}
207+
208+
209+
// TODO: get rid of the casting
210+
function displayResource(r: Resource): string
211+
{
212+
return {
213+
"Link": (r as unknown as Link).address,
214+
"Text": (r as Text).content,
215+
}[r.__typename];
216+
}

src/ResourceAdder.tsx

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { Resource } from "./core/resources";
1+
// TODO: how to limit importing scope?
2+
import { Resource, ResourceTypeString, ResourceTypeStringValues, Link, Text } from "./core/resources";
23

34
import React, { FunctionComponent } from "react";
45
import { Formik, Form, Field } from "formik";
@@ -7,15 +8,12 @@ import { Formik, Form, Field } from "formik";
78
import "./ResourceAdder.css";
89

910

10-
// TODO: is is possible to reflect this from Resource union given proper typenames on interfaces?
11-
type TypeString = "Link" | "Text";
12-
1311
const ResourceAdder: React.FunctionComponent<{
1412
onAdd: (resource: Resource) => void,
1513
}> = (props) =>
1614
<Formik
1715
initialValues={{
18-
typestring: "Link"
16+
typestring: ResourceTypeStringValues[0]
1917
}}
2018
onSubmit={(values, _) =>
2119
{
@@ -30,22 +28,23 @@ const ResourceAdder: React.FunctionComponent<{
3028
}
3129
<Form translate="yes">
3230
<Field name="typestring" component="select">
33-
<option value="Link">Link</option>
34-
<option value="Text">Text</option>
31+
{
32+
ResourceTypeStringValues
33+
.map(t => <option value={t}>{t}</option>)
34+
}
3535
</Field>
36-
3736
<button type="submit"> Dodaj </button>
3837
</Form>
3938
</Formik>
4039

4140
export default ResourceAdder;
4241

4342

44-
// TODO: make this a field on a resource
45-
function default_for_typestring(t: TypeString): Resource
43+
// TODO: get rid of the casting
44+
function default_for_typestring(t: ResourceTypeString): Resource
4645
{
4746
return {
48-
"Link": { address: "", is_done: false },
49-
"Text": { content: "", is_done: false },
47+
"Link": { __typename: "Link", address: "", is_done: false } as Link,
48+
"Text": { __typename: "Text", content: "", is_done: false } as Text,
5049
}[t];
5150
}

src/ResourceEditor.tsx

Lines changed: 15 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ const ResourceEditor: React.FunctionComponent<{
1616
}> = (props) =>
1717
<Formik
1818
initialValues={
19-
initial_fields_for_resource(props.resource)
19+
props.resource
2020
}
2121
onSubmit={(values, actions) =>
2222
{
2323
const resource = {
2424
...values,
25-
is_done: false,
25+
// TODO: remove this after #35
26+
is_done: props.resource.is_done,
27+
__typename: props.resource.__typename,
2628
};
2729

2830
props.onEditonCommit(resource as any);
@@ -43,45 +45,18 @@ const ResourceEditor: React.FunctionComponent<{
4345
export default ResourceEditor;
4446

4547

46-
// TODO: use reflection instead of hardcode
4748
// TODO: is the filedset a good idea here?
49+
// TODO: dry-ify this a bit
4850
function resource_to_fields(r: Resource): Array<JSX.Element>
4951
{
50-
return [
51-
{
52-
property: "address",
53-
jsx: <fieldset>
54-
<label className="file-uploader-label">address</label>
55-
<Field name="address" />
56-
</fieldset>
57-
},
58-
{
59-
property: "content",
60-
jsx: <fieldset>
61-
<label className="file-uploader-label">content</label>
62-
<Field name="content" />
63-
</fieldset>
64-
},
65-
]
66-
.filter(x => x.property in r)
67-
.map(x => x.jsx)
68-
;
69-
}
70-
71-
// TODO: use reflection instead of hardcode
72-
// TODO: return type will actually be a Resource 0.o
73-
function initial_fields_for_resource(r: Resource): object
74-
{
75-
return [
76-
"address",
77-
"content",
78-
]
79-
.filter(x => x in r)
80-
.reduce((obj, prop) =>
81-
{
82-
// TODO: do something about the types
83-
(obj as any)[prop] = (r as any)[prop];
84-
return obj;
85-
}, {})
86-
;
52+
return {
53+
"Link": [<fieldset>
54+
<label className="file-uploader-label">address</label>
55+
<Field name="address" />
56+
</fieldset>],
57+
"Text": [<fieldset>
58+
<label className="file-uploader-label">content</label>
59+
<Field name="content" />
60+
</fieldset>],
61+
}[r.__typename];
8762
}

src/core/resources.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,28 @@
11
export interface Link {
2+
__typename: "Link";
23
address: string;
34
is_done: boolean;
45
}
56

67
export interface Text {
8+
__typename: "Text";
79
content: string;
810
is_done: boolean;
9-
// TODO: this is an enum, not a boolean
11+
// TODO: is_done => status ; boolean => enum
1012
}
13+
// when adding a new Resource please remember to add it to Resource union
14+
// and ResourceTypeStringValues array
1115

1216
// TODO: add a null resource - for the root
1317

18+
1419
export type Resource = Link | Text;
20+
export type ResourceTypeString = Resource["__typename"];
21+
22+
export const ResourceTypeStringValues: Array<ResourceTypeString> = [
23+
"Link",
24+
"Text",
25+
];
26+
1527

1628
export default {};

0 commit comments

Comments
 (0)