From e0a5118f8d97523c60ef424723e0d34d9d6adb93 Mon Sep 17 00:00:00 2001 From: William Chargin Date: Mon, 23 Apr 2018 13:12:44 -0700 Subject: [PATCH] Switch typed dispatch table to `empty` assertion (#133) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: This replaces the implementation of a static check from a somewhat complicated use of higher-order types to a more simple empty-union assertion, as suggested by jez in “Case Exhaustiveness in Flow”: https://blog.jez.io/flow-exhaustiveness/ (I know; we’re not using Reason. One step at a time. :-) ) I adapted the implementation a bit because I prefer explicitly disabling an ESLint warning over a no-op function call; it is not clear from the latter that the purpose is to suppress a lint warning. Test Plan: In `githubPlugin.js`, add `| "ANOTHER"` to the `NodeType` type, and note a compile-time Flow error on the appropriate line, with a very readable error message. Note that all unit tests pass, and running the UI on `sourcecred/sourcecred` yields correct titles for each node type present (namely, all node types except for `ORGANIZATION` and `BOT`). wchargin-branch: empty-union-assertion --- .../editor/adapters/githubPluginAdapter.js | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/plugins/artifact/editor/adapters/githubPluginAdapter.js b/src/plugins/artifact/editor/adapters/githubPluginAdapter.js index 201da21..b76d8a1 100644 --- a/src/plugins/artifact/editor/adapters/githubPluginAdapter.js +++ b/src/plugins/artifact/editor/adapters/githubPluginAdapter.js @@ -7,7 +7,6 @@ import type {Node} from "../../../../core/graph"; import type { NodePayload, NodeType, - NodeTypes, IssueNodePayload, PullRequestNodePayload, CommentNodePayload, @@ -79,26 +78,27 @@ const adapter: PluginAdapter = { function extractAuthorTitle(node: Node) { return node.payload.login; } - type TypedNodeToStringExtractor = >( - T - ) => (node: Node<$ElementType>) => string; - const extractors: $Exact<$ObjMap> = { - ISSUE: extractIssueOrPrTitle, - PULL_REQUEST: extractIssueOrPrTitle, - COMMENT: (node) => extractCommentTitle("comment", node), - PULL_REQUEST_REVIEW_COMMENT: (node) => - extractCommentTitle("review comment", node), - PULL_REQUEST_REVIEW: extractPRReviewTitle, - USER: extractAuthorTitle, - ORGANIZATION: extractAuthorTitle, - BOT: extractAuthorTitle, - }; - function fallbackAccessor(node: Node) { - throw new Error(`unknown node type: ${node.address.type}`); + const anyNode: Node = node; + const type: NodeType = (node.address.type: any); + switch (type) { + case "ISSUE": + case "PULL_REQUEST": + return extractIssueOrPrTitle(anyNode); + case "COMMENT": + return extractCommentTitle("comment", anyNode); + case "PULL_REQUEST_REVIEW_COMMENT": + return extractCommentTitle("review comment", anyNode); + case "PULL_REQUEST_REVIEW": + return extractPRReviewTitle(anyNode); + case "USER": + case "ORGANIZATION": + case "BOT": + return extractAuthorTitle(anyNode); + default: + // eslint-disable-next-line no-unused-expressions + (type: empty); + throw new Error(`unknown node type: ${node.address.type}`); } - return (extractors[node.address.type] || fallbackAccessor)( - (node: Node) - ); }, };