Skip to content

Commit d7e06b6

Browse files
kswensonkswenson
committed
Use type guards to simplify code and reduce type assertions; add stricter linting option (#912)
- use type assertions to simplify code and reduce type casting - add lint:types script which enables additional type-checking rules
1 parent a34b1b2 commit d7e06b6

32 files changed

+217
-181
lines changed

‎.eslintrc.types.js

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/* eslint-env node */
2+
// build/production configuration extends default/development configuration
3+
module.exports = {
4+
parserOptions: {
5+
tsconfigRootDir: __dirname,
6+
project: ['./tsconfig.json']
7+
},
8+
extends: [
9+
"./.eslintrc.js",
10+
"plugin:@typescript-eslint/recommended-requiring-type-checking"
11+
],
12+
rules: {
13+
"@typescript-eslint/no-floating-promises": "off", // 67 as of 2021-02-08
14+
"@typescript-eslint/no-misused-promises": "off", // 11 as of 2021-02-08
15+
"@typescript-eslint/no-unsafe-assignment": "off", // 450 as of 2021-02-08
16+
"@typescript-eslint/no-unsafe-call": "off", // 72 as of 2021-02-08
17+
"@typescript-eslint/no-unsafe-member-access": "off", // 388 as of 2021-02-08
18+
"@typescript-eslint/no-unsafe-return": "off", // 80 as of 2021-02-08
19+
"@typescript-eslint/restrict-template-expressions": "off" // 31 as of 2021-02-08
20+
},
21+
overrides: [
22+
{ // some rules can be relaxed in tests
23+
files: ["**/*.test.*"],
24+
rules: {
25+
"@typescript-eslint/require-await": "off"
26+
}
27+
}
28+
]
29+
};

‎package.json

+1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
"lint": "eslint \"./src/**/*.{js,json,jsx,ts,tsx}\" \"./cypress/**/*.{js,json,jsx,ts,tsx}\"",
4949
"lint:build": "eslint -c \".eslintrc.build.js\" \"./src/**/*.{js,json,jsx,ts,tsx}\"",
5050
"lint:fix": "eslint --fix \"./src/**/*.{js,json,jsx,ts,tsx}\" \"./cypress/**/*.{js,json,jsx,ts,tsx}\"",
51+
"lint:types": "eslint -c \".eslintrc.types.js\" \"./src/**/*.{js,jsx,ts,tsx}\"",
5152
"lint:unused": "tsc --noUnusedLocals --project .",
5253
"migrate": "node ./migrations/migrate",
5354
"migrate:debug": "node --inspect-brk ./migrations/migrate",

‎src/components/app.tsx

+3-3
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ function resolveAppMode(
4949
const { appMode, db, ui} = stores;
5050
if (appMode === "authed") {
5151
if (rawFirebaseJWT) {
52-
db.connect({appMode, stores, rawFirebaseJWT}).catch(ui.setError);
52+
db.connect({appMode, stores, rawFirebaseJWT}).catch(error => ui.setError(error));
5353
}
5454
else {
5555
ui.setError("No firebase token available to connect to db!");
@@ -75,7 +75,7 @@ function resolveAppMode(
7575
}
7676
}
7777
})
78-
.catch(ui.setError);
78+
.catch(error => ui.setError(error));
7979
}
8080
}
8181

@@ -210,7 +210,7 @@ export class AppComponent extends BaseComponent<IProps, IState> {
210210
);
211211
}
212212

213-
private handlePortalLoginRedirect() {
213+
private handlePortalLoginRedirect = () => {
214214
window.location.href = urlParams.domain || "https://learn.concord.org";
215215
}
216216
}

‎src/components/document/document-workspace.tsx

+12-12
Original file line numberDiff line numberDiff line change
@@ -277,24 +277,24 @@ export class DocumentWorkspaceComponent extends BaseComponent<IProps> {
277277
}
278278

279279
private handleNewDocument = (type: string) => {
280-
const { appConfig, user } = this.stores;
280+
const { appConfig, documents, ui, user } = this.stores;
281281
const isLearningLog = type === LearningLogDocument;
282282
const docType = isLearningLog ? LearningLogDocument : PersonalDocument;
283283
const defaultDocTitle = isLearningLog
284284
? appConfig.defaultLearningLogTitle
285285
: appConfig.defaultDocumentTitle;
286286
const docTypeString = appConfig.getDocumentLabel(docType, 1);
287287
const docTypeStringL = appConfig.getDocumentLabel(docType, 1, true);
288-
const nextTitle = this.stores.documents.getNextOtherDocumentTitle(user, docType, defaultDocTitle);
289-
this.stores.ui.prompt({
288+
const nextTitle = documents.getNextOtherDocumentTitle(user, docType, defaultDocTitle);
289+
ui.prompt({
290290
className: `create-${type}`,
291291
title: `Create ${docTypeString}`,
292292
text: `Name your new ${docTypeStringL}:`,
293293
defaultValue: `${nextTitle}`,
294294
})
295295
.then((title: string) => {
296296
this.handleNewDocumentOpen(docType, title)
297-
.catch(this.stores.ui.setError);
297+
.catch(error => ui.setError(error));
298298
});
299299
}
300300

@@ -309,18 +309,18 @@ export class DocumentWorkspaceComponent extends BaseComponent<IProps> {
309309
}
310310

311311
private handleCopyDocument = (document: DocumentModelType) => {
312-
const { appConfig } = this.stores;
312+
const { appConfig, ui } = this.stores;
313313
const docTypeString = document.getLabel(appConfig, 1);
314314
const docTypeStringL = document.getLabel(appConfig, 1, true);
315315
const originTitle = document?.properties?.get("originTitle");
316316
const baseTitle = appConfig.copyPreferOriginTitle && originTitle
317317
? originTitle
318318
: document.title || this.stores.problem.title;
319-
this.stores.ui.prompt(`Give your ${docTypeStringL} copy a new name:`,
320-
`Copy of ${baseTitle}`, `Copy ${docTypeString}`)
319+
ui.prompt(`Give your ${docTypeStringL} copy a new name:`,
320+
`Copy of ${baseTitle}`, `Copy ${docTypeString}`)
321321
.then((title: string) => {
322322
this.handleCopyDocumentOpen(document, title)
323-
.catch(this.stores.ui.setError);
323+
.catch(error => ui.setError(error));
324324
});
325325
}
326326

@@ -365,7 +365,7 @@ export class DocumentWorkspaceComponent extends BaseComponent<IProps> {
365365
: document.title;
366366
}
367367

368-
private handlePublishSupport = async (document: DocumentModelType) => {
368+
private handlePublishSupport = (document: DocumentModelType) => {
369369
const { db, problemPath, ui, user } = this.stores;
370370
const caption = this.getSupportDocumentBaseCaption(document) || "Untitled";
371371
// TODO: Disable publish button while publishing
@@ -386,9 +386,9 @@ export class DocumentWorkspaceComponent extends BaseComponent<IProps> {
386386
.then((confirm: boolean) => {
387387
if (confirm) {
388388
const dbPublishDocumentFunc = document.type === ProblemDocument
389-
? db.publishProblemDocument
390-
: db.publishOtherDocument;
391-
dbPublishDocumentFunc.call(db, document)
389+
? () => db.publishProblemDocument(document)
390+
: () => db.publishOtherDocument(document);
391+
dbPublishDocumentFunc()
392392
.then(() => ui.alert(`Your ${docTypeStringL} was published.`, `${docTypeString} Published`));
393393
}
394394
});

‎src/components/four-up.test.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ describe("Four Up Component", () => {
4949
});
5050

5151
const stores = createStores({ groups, documents });
52-
const comp = mount(<FourUpComponent userId={document.uid} groupId={document.groupId!} stores={stores}/>);
52+
const comp = mount(<FourUpComponent userId={document.uid} groupId={document.groupId} stores={stores}/>);
5353
expect(comp.find(CanvasComponent)).toHaveLength(4);
5454
expect(comp.find(".member")).toHaveLength(1);
5555
});

‎src/components/navigation/problem-tab-content.tsx

+3-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ interface IProps {
1818

1919
export const ProblemTabContent: React.FC<IProps> = observer(({ context, sections, showSolutionsSwitch}: IProps) => {
2020
const { isTeacher } = useUserStore();
21-
const { showTeacherContent, toggleShowTeacherContent } = useUIStore();
21+
const ui = useUIStore();
22+
const { showTeacherContent } = ui;
2223

2324
const handleTabClick = (title: string, type: string) => {
2425
Logger.log(LogEventName.SHOW_TAB_SECTION, {
@@ -28,7 +29,7 @@ export const ProblemTabContent: React.FC<IProps> = observer(({ context, sections
2829
};
2930

3031
const handleToggleSolutions = () => {
31-
toggleShowTeacherContent(!showTeacherContent);
32+
ui.toggleShowTeacherContent(!showTeacherContent);
3233
Logger.log(showTeacherContent ? LogEventName.HIDE_SOLUTIONS : LogEventName.SHOW_SOLUTIONS);
3334
};
3435

‎src/components/tools/drawing-tool/drawing-layer.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,7 @@ export class DrawingLayerView extends React.Component<DrawingLayerViewProps, Dra
850850
if (moved) {
851851
if (objectsToInteract.length > 0) {
852852
const moves: DrawingToolMove = objectsToInteract.map((object) => ({
853-
id: object!.model.id || "",
853+
id: object.model.id || "",
854854
destination: {x: object.model.x, y: object.model.y}
855855
}));
856856
this.sendChange({action: "move", data: moves});

‎src/components/tools/geometry-tool/geometry-content.tsx

+30-35
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ import {
1919
getAssociatedPolygon, getPointsForVertexAngle, getPolygonEdges
2020
} from "../../../models/tools/geometry/jxg-polygon";
2121
import {
22-
isAxis, isAxisLabel, isComment, isFreePoint, isPoint, isPolygon, isVertexAngle, isVisibleEdge, isVisiblePoint
22+
isAxis, isAxisLabel, isComment, isFreePoint, isGeometryElement, isImage, isLine, isMovableLine,
23+
isMovableLineControlPoint, isMovableLineLabel, isPoint, isPolygon, isVertexAngle, isVisibleEdge,
24+
isVisibleMovableLine, isVisiblePoint
2325
} from "../../../models/tools/geometry/jxg-types";
2426
import {
2527
getVertexAngle, updateVertexAngle, updateVertexAnglesFromObjects
@@ -33,9 +35,6 @@ import { getUrlFromImageContent } from "../../../utilities/image-utils";
3335
import { safeJsonParse, uniqueId } from "../../../utilities/js-utils";
3436
import { hasSelectionModifier } from "../../../utilities/event-utils";
3537
import { assign, castArray, debounce, each, filter, find, keys as _keys, throttle, values } from "lodash";
36-
import {
37-
isVisibleMovableLine, isMovableLine, isMovableLineControlPoint, isMovableLineLabel,
38-
} from "../../../models/tools/geometry/jxg-movable-line";
3938
import { Logger, LogEventName, LogEventMethod } from "../../../lib/logger";
4039
import { getDataSetBounds, IDataSet } from "../../../models/data/data-set";
4140
import AxisSettingsDialog from "./axis-settings-dialog";
@@ -109,7 +108,7 @@ function syncBoardChanges(board: JXG.Board, content: GeometryContentModelType,
109108
try {
110109
const change: JXGChange = JSON.parse(content.changes[i]);
111110
const result = content.syncChange(board, change);
112-
const elts = castArray(result).filter(elt => elt instanceof JXG.GeometryElement) as JXG.GeometryElement[];
111+
const elts = castArray(result).filter(isGeometryElement);
113112
newElements.push(...elts);
114113
if (change.operation === "update") {
115114
const ids = castArray(change.targetID);
@@ -578,10 +577,9 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
578577
private getBackgroundImage(_board?: JXG.Board) {
579578
const board = _board || this.state.board;
580579
if (!board) return;
581-
const images = this.getContent()
582-
.findObjects(board, (obj: JXG.GeometryElement) => obj.elType === "image");
580+
const images = this.getContent().findObjects(board, isImage) as JXG.Image[];
583581
return images.length > 0
584-
? images[images.length - 1] as JXG.Image
582+
? images[images.length - 1]
585583
: undefined;
586584
}
587585

@@ -648,8 +646,8 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
648646
private handleToggleVertexAngle = () => {
649647
const { board } = this.state;
650648
const selectedObjects = board && this.getContent().selectedObjects(board);
651-
const selectedPoints = selectedObjects && selectedObjects.filter(isPoint);
652-
const selectedPoint = selectedPoints && selectedPoints[0] as JXG.Point;
649+
const selectedPoints = selectedObjects?.filter(isPoint);
650+
const selectedPoint = selectedPoints?.[0];
653651
if (board && selectedPoint) {
654652
const vertexAngle = getVertexAngle(selectedPoint);
655653
if (!vertexAngle) {
@@ -724,7 +722,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
724722
if (commentAnchor) {
725723
this.applyChange(() => {
726724
const elems = content.addComment(board, commentAnchor.id);
727-
const comment = elems && elems.find((elem: JXG.GeometryElement) => isComment(elem)) as JXG.Text;
725+
const comment = elems?.find(isComment);
728726
if (comment) {
729727
this.handleCreateText(comment);
730728
this.setState({selectedComment: comment});
@@ -1062,7 +1060,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
10621060
});
10631061
this.updateImageUrl(contentUrl);
10641062
if (this.props.size.height && image.height! > this.props.size.height) {
1065-
this.props.onRequestRowHeight(this.props.model.id, image.height!);
1063+
this.props.onRequestRowHeight(this.props.model.id, image.height);
10661064
}
10671065
}
10681066

@@ -1111,22 +1109,22 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
11111109
elt.setAttribute({ fixed: true });
11121110
}
11131111
if (isPoint(elt)) {
1114-
this.handleCreatePoint(elt as JXG.Point);
1112+
this.handleCreatePoint(elt);
11151113
}
11161114
else if (isPolygon(elt)) {
1117-
this.handleCreatePolygon(elt as JXG.Polygon);
1115+
this.handleCreatePolygon(elt);
11181116
}
11191117
else if (isVertexAngle(elt)) {
1120-
this.handleCreateVertexAngle(elt as JXG.Angle);
1118+
this.handleCreateVertexAngle(elt);
11211119
}
11221120
else if (isMovableLine(elt)) {
1123-
this.handleCreateLine(elt as JXG.Line);
1121+
this.handleCreateLine(elt);
11241122
}
11251123
else if (isComment(elt) || isMovableLineLabel(elt)) {
1126-
this.handleCreateText(elt as JXG.Text);
1124+
this.handleCreateText(elt);
11271125
}
11281126
else if (isAxis(elt)) {
1129-
this.handleCreateAxis(elt as JXG.Line);
1127+
this.handleCreateAxis(elt);
11301128
}
11311129
});
11321130
}
@@ -1212,7 +1210,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
12121210
if (board && !hasSelectionModifier(evt || {})) {
12131211
content.metadata.selection.forEach((isSelected: boolean, id: string) => {
12141212
const obj = board.objects[id];
1215-
const pt = isPoint(obj) ? obj as JXG.Point : undefined;
1213+
const pt = isPoint(obj) ? obj : undefined;
12161214
if (pt && isSelected && !pt.getAttribute("fixed")) {
12171215
this.dragPts[id] = {
12181216
initial: copyCoords(pt.coords),
@@ -1230,7 +1228,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
12301228
each(this.dragPts, (entry, id) => {
12311229
if (entry) {
12321230
const obj = board.objects[id];
1233-
const pt = isPoint(obj) ? obj as JXG.Point : undefined;
1231+
const pt = isPoint(obj) ? obj : undefined;
12341232
// move the points not dragged by JSXGraph
12351233
if (pt && !isDragTargetOrAncestor(pt, dragTarget)) {
12361234
const newUsrCoords = JXG.Math.Statistics.add(entry.initial.usrCoords, usrDiff) as number[];
@@ -1267,7 +1265,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
12671265
each(this.dragPts, (entry, id) => {
12681266
if (entry && content) {
12691267
const obj = board.objects[id];
1270-
const pt = isPoint(obj) ? obj as JXG.Point : undefined;
1268+
const pt = isPoint(obj) ? obj : undefined;
12711269
if (pt) {
12721270
const newUsrCoords = JXG.Math.Statistics.add(entry.initial.usrCoords, usrDiff) as number[];
12731271
ids.push(id);
@@ -1295,7 +1293,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
12951293
(dragEntry.final.usrCoords[2] === dragEntry.initial.usrCoords[2])) return;
12961294

12971295
const position = dragEntry.final.usrCoords.slice(1) as JXGCoordPair;
1298-
this.applyChange(() => content.updateObjects(board!, dragTarget.id, { position }));
1296+
this.applyChange(() => content.updateObjects(board, dragTarget.id, { position }));
12991297
}
13001298

13011299
private handleCreateBoard = (board: JXG.Board) => {
@@ -1359,7 +1357,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
13591357
if (!hasSelectionModifier(evt)) {
13601358
const props = { snapToGrid: true, snapSizeX: kSnapUnit, snapSizeY: kSnapUnit };
13611359
this.applyChange(() => {
1362-
const point = geometryContent.addPoint(board, [x, y], props) as JXG.Point;
1360+
const point = geometryContent.addPoint(board, [x, y], props);
13631361
if (point) {
13641362
this.handleCreatePoint(point);
13651363
}
@@ -1428,7 +1426,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
14281426
if (isFreePoint(point) && this.isDoubleClick(this.lastPointDown, { evt, coords })) {
14291427
if (board) {
14301428
this.applyChange(() => {
1431-
const polygon = geometryContent.createPolygonFromFreePoints(board, tableId, columnId) as JXG.Polygon;
1429+
const polygon = geometryContent.createPolygonFromFreePoints(board, tableId, columnId);
14321430
if (polygon) {
14331431
this.handleCreatePolygon(polygon);
14341432
this.props.onContentChange();
@@ -1449,7 +1447,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
14491447

14501448
if (isMovableLineControlPoint(point)) {
14511449
// When a control point is clicked, deselect the rest of the line so the line slope can be changed
1452-
const line = find(point.descendants, el => isMovableLine(el));
1450+
const line = find(point.descendants, isMovableLine);
14531451
if (line) {
14541452
geometryContent.deselectElement(undefined, line.id);
14551453
each(line.ancestors, (parentPoint, parentId) => {
@@ -1517,7 +1515,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
15171515
private handleCreateLine = (line: JXG.Line) => {
15181516

15191517
function getVertices() {
1520-
return filter(line.ancestors, ancestor => isPoint(ancestor)) as JXG.Point[];
1518+
return filter(line.ancestors, isPoint);
15211519
}
15221520

15231521
const isInVertex = (evt: any) => {
@@ -1605,8 +1603,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
16051603
const coords = getEventCoords(board, evt, scale);
16061604
let inVertex = false;
16071605
each(polygon.ancestors, point => {
1608-
const pt = point as JXG.Point;
1609-
if (pt.hasPoint(coords.scrCoords[1], coords.scrCoords[2])) {
1606+
if (isPoint(point) && point.hasPoint(coords.scrCoords[1], coords.scrCoords[2])) {
16101607
inVertex = true;
16111608
}
16121609
});
@@ -1617,8 +1614,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
16171614
const geometryContent = this.props.model.content as GeometryContentModelType;
16181615
let allSelected = true;
16191616
each(polygon.ancestors, point => {
1620-
const pt = point as JXG.Point;
1621-
if (!geometryContent.isSelected(pt.id)) {
1617+
if (isPoint(point) && !geometryContent.isSelected(point.id)) {
16221618
allSelected = false;
16231619
}
16241620
});
@@ -1651,9 +1647,8 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
16511647
if (selectPolygon) {
16521648
geometryContent.selectElement(board, polygon.id);
16531649
each(polygon.ancestors, point => {
1654-
const pt = point as JXG.Point;
1655-
if (board && !inVertex) {
1656-
geometryContent.selectElement(board, pt.id);
1650+
if (board && isPoint(point) && !inVertex) {
1651+
geometryContent.selectElement(board, point.id);
16571652
}
16581653
});
16591654
}
@@ -1751,8 +1746,8 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
17511746
// Extended clicks/drags don't open the movable line dialog
17521747
const clickTimeThreshold = 500;
17531748
if (evt.timeStamp - this.lastBoardDown.evt.timeStamp < clickTimeThreshold) {
1754-
const parentLine = values(text.ancestors)[0] as JXG.Line;
1755-
if (parentLine && !readOnly) {
1749+
const parentLine = values(text.ancestors)[0];
1750+
if (isLine(parentLine) && !readOnly) {
17561751
this.setState({selectedLine: parentLine});
17571752
}
17581753
}

0 commit comments

Comments
 (0)