From 04a4fb1c1b13ef8cbd00db6feb7d2257ea12ccd3 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Fri, 28 Aug 2020 19:28:35 -0400 Subject: [PATCH 01/10] Disable `simpleLinker` in headings and fix complex `details` triggers Code quotes in headers are no longer auto-linked by `simpleLinker`, both for UX and bugfix purposes. Before, any markup like code quotes would break a header as only the first child had its text extracted. Now the trigger uses the whole children value of the header it's made from. --- .../gatsby-remark-dvc-linker/simpleLinker.js | 4 ++- .../Documentation/Markdown/index.tsx | 30 ++++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/plugins/gatsby-remark-dvc-linker/simpleLinker.js b/plugins/gatsby-remark-dvc-linker/simpleLinker.js index 43fc18dd0a..46e2fa8678 100644 --- a/plugins/gatsby-remark-dvc-linker/simpleLinker.js +++ b/plugins/gatsby-remark-dvc-linker/simpleLinker.js @@ -4,6 +4,8 @@ const { createLinkNode } = require('./helpers') const entries = require('../../content/linked-terms') +const excludedParentTypes = ['link', 'heading'] + const useMatcher = (matcher, item) => { switch (typeof matcher) { case 'string': @@ -21,7 +23,7 @@ module.exports = astNode => { const node = astNode[0] const parent = astNode[2] - if (parent.type !== 'link') { + if (!excludedParentTypes.includes(parent.type)) { const entry = entries.find(({ matches }) => useMatcher(matches, node.value)) if (entry) { createLinkNode(entry.url, astNode) diff --git a/src/components/Documentation/Markdown/index.tsx b/src/components/Documentation/Markdown/index.tsx index 5d1a7ef88a..ae9afeaabb 100644 --- a/src/components/Documentation/Markdown/index.tsx +++ b/src/components/Documentation/Markdown/index.tsx @@ -1,4 +1,10 @@ -import React, { useCallback, useEffect, useRef } from 'react' +import React, { + useCallback, + useEffect, + useRef, + ReactNode, + ReactElement +} from 'react' import cn from 'classnames' import { navigate } from '@reach/router' import rehypeReact from 'rehype-react' @@ -13,6 +19,8 @@ import 'github-markdown-css/github-markdown.css' import sharedStyles from '../styles.module.css' import styles from './styles.module.css' +import util from 'util' + const isInsideCodeBlock = (node: Element): boolean => { while (node?.parentNode) { if (node.tagName === 'PRE') { @@ -30,17 +38,31 @@ const isInsideCodeBlock = (node: Element): boolean => { } const Details: React.FC<{ - children: Array<{ props: { children: Array } } | string> + children: Array<{ props: { children: ReactNode } } | string> }> = ({ children }) => { const filteredChildren = children.filter(child => child !== '\n') if (!filteredChildren.length) return null if (typeof filteredChildren[0] === 'string') return null - const text = filteredChildren[0].props.children[0] + const headingChildren: ReactNode[] = filteredChildren[0].props + .children as ReactNode[] + + // Remove header auto-link if present + const finalHeadingChild = headingChildren[headingChildren.length - 1]! as { + props: { + className: string + } + } + + const triggerChildren: unknown = + finalHeadingChild.props !== undefined && + finalHeadingChild.props.className === 'anchor after' + ? headingChildren.slice(0, headingChildren.length - 1) + : headingChildren return ( - + {filteredChildren.slice(1)} ) From 92d31986c6b4eed13e40086efddbcca08bc2eee4 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Fri, 28 Aug 2020 19:54:57 -0400 Subject: [PATCH 02/10] Remove unused util import and unncessesary null assertion --- src/components/Documentation/Markdown/index.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/Documentation/Markdown/index.tsx b/src/components/Documentation/Markdown/index.tsx index ae9afeaabb..345d7f38ec 100644 --- a/src/components/Documentation/Markdown/index.tsx +++ b/src/components/Documentation/Markdown/index.tsx @@ -19,8 +19,6 @@ import 'github-markdown-css/github-markdown.css' import sharedStyles from '../styles.module.css' import styles from './styles.module.css' -import util from 'util' - const isInsideCodeBlock = (node: Element): boolean => { while (node?.parentNode) { if (node.tagName === 'PRE') { @@ -49,7 +47,7 @@ const Details: React.FC<{ .children as ReactNode[] // Remove header auto-link if present - const finalHeadingChild = headingChildren[headingChildren.length - 1]! as { + const finalHeadingChild = headingChildren[headingChildren.length - 1] as { props: { className: string } From 2c69ee67d82c8a79c64a8465307286e17319df83 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Mon, 31 Aug 2020 00:28:42 -0400 Subject: [PATCH 03/10] Change TS cast for triggerChildren to reflect that it's a ReactNode --- src/components/Documentation/Markdown/index.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/components/Documentation/Markdown/index.tsx b/src/components/Documentation/Markdown/index.tsx index 345d7f38ec..0af899390d 100644 --- a/src/components/Documentation/Markdown/index.tsx +++ b/src/components/Documentation/Markdown/index.tsx @@ -53,14 +53,17 @@ const Details: React.FC<{ } } - const triggerChildren: unknown = + const triggerChildren: ReactNode = finalHeadingChild.props !== undefined && finalHeadingChild.props.className === 'anchor after' ? headingChildren.slice(0, headingChildren.length - 1) : headingChildren return ( - + {filteredChildren.slice(1)} ) From 4c3a756b28fbd54078b79bc0eb8f6ad3e3bd855e Mon Sep 17 00:00:00 2001 From: rogermparent Date: Mon, 31 Aug 2020 00:29:43 -0400 Subject: [PATCH 04/10] Remove dead code from GitHub Model This is unrelated to the PR, but while we're ts-linting we may as well --- src/gatsby/models/github/index.js | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/src/gatsby/models/github/index.js b/src/gatsby/models/github/index.js index 9398d821c0..5a7bb74b66 100644 --- a/src/gatsby/models/github/index.js +++ b/src/gatsby/models/github/index.js @@ -7,34 +7,6 @@ alone does. */ -async function createStaticGithubDataNode(api, fieldData = {}) { - const { - actions: { createNode }, - createNodeId, - createContentDigest - } = api - - const fields = { - stars: 8888, - parent: null, - ...fieldData - } - - const node = { - id: createNodeId('DVCGithubStaticData'), - children: [], - ...fields, - internal: { - type: 'StaticGithubData', - contentDigest: createContentDigest(fields) - } - } - - await createNode(node) - - return node -} - module.exports = { async createSchemaCustomization({ actions: { createTypes }, schema }) { createTypes( From b9983b686f1f7ebf677db65048b06839f8b03114 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Mon, 31 Aug 2020 00:53:27 -0400 Subject: [PATCH 05/10] Simplify how details triggers/headings are handled Now it just runs off the first (filtered) child. If that child is a header, it's assumed the last item of the heading is an auto-link and removed. Currently, every `details` first child is a heading, and all headings have auto-links. --- .../Documentation/Markdown/index.tsx | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/components/Documentation/Markdown/index.tsx b/src/components/Documentation/Markdown/index.tsx index 0af899390d..371e573cbf 100644 --- a/src/components/Documentation/Markdown/index.tsx +++ b/src/components/Documentation/Markdown/index.tsx @@ -39,26 +39,19 @@ const Details: React.FC<{ children: Array<{ props: { children: ReactNode } } | string> }> = ({ children }) => { const filteredChildren = children.filter(child => child !== '\n') + const firstChild = filteredChildren[0] - if (!filteredChildren.length) return null - if (typeof filteredChildren[0] === 'string') return null + const triggerChildren: ReactNode[] = firstChild.props.children as ReactNode[] - const headingChildren: ReactNode[] = filteredChildren[0].props - .children as ReactNode[] - - // Remove header auto-link if present - const finalHeadingChild = headingChildren[headingChildren.length - 1] as { - props: { - className: string - } + /* + As an adaptation to auto-linked headings, the last child of any heading + nodes must be removed. The only way around this is the change the + autolinker, which we currently have as an external package + */ + if (/h./.test(firstChild.type)) { + triggerChildren.pop() } - const triggerChildren: ReactNode = - finalHeadingChild.props !== undefined && - finalHeadingChild.props.className === 'anchor after' - ? headingChildren.slice(0, headingChildren.length - 1) - : headingChildren - return ( Date: Mon, 31 Aug 2020 00:57:10 -0400 Subject: [PATCH 06/10] Change details DVC-file to `.dvc` file This is mostly as an example of this PR's fix working. --- content/docs/user-guide/external-dependencies.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/content/docs/user-guide/external-dependencies.md b/content/docs/user-guide/external-dependencies.md index d768bda4ce..0e46718520 100644 --- a/content/docs/user-guide/external-dependencies.md +++ b/content/docs/user-guide/external-dependencies.md @@ -154,7 +154,7 @@ The command above creates the import stage (DVC-file)
-### Expand to see resulting DVC-file +### Expand to see resulting `.dvc` file ```yaml # ... @@ -193,7 +193,7 @@ specified (with the `repo` field).
-### Expand to see resulting DVC-file +### Expand to see resulting `.dvc` file ```yaml # ... From 229bddcdbc2c77e522ff2c618a4c68aa66d6ab62 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Mon, 31 Aug 2020 11:18:46 -0400 Subject: [PATCH 07/10] Re-word comment and change heading type regex to use ^ and $ --- src/components/Documentation/Markdown/index.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/components/Documentation/Markdown/index.tsx b/src/components/Documentation/Markdown/index.tsx index 371e573cbf..b21b47ad67 100644 --- a/src/components/Documentation/Markdown/index.tsx +++ b/src/components/Documentation/Markdown/index.tsx @@ -44,14 +44,18 @@ const Details: React.FC<{ const triggerChildren: ReactNode[] = firstChild.props.children as ReactNode[] /* - As an adaptation to auto-linked headings, the last child of any heading - nodes must be removed. The only way around this is the change the - autolinker, which we currently have as an external package + To work around auto-linked headings, the last child of any heading node + must be removed. The only way around this is the change the autolinker, + which we currently have as an external package. */ - if (/h./.test(firstChild.type)) { + if (/^h.$/.test(firstChild.type)) { triggerChildren.pop() } + /* + Collapsible's trigger type wants ReactElement, so we force a TS cast from + ReactNode here. + */ return ( Date: Mon, 31 Aug 2020 11:34:22 -0400 Subject: [PATCH 08/10] Add JSX.Element cast to firstChild to quell TS Even if the element is a string, the regex test will return false from a non-string property. --- src/components/Documentation/Markdown/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Documentation/Markdown/index.tsx b/src/components/Documentation/Markdown/index.tsx index b21b47ad67..68be406e85 100644 --- a/src/components/Documentation/Markdown/index.tsx +++ b/src/components/Documentation/Markdown/index.tsx @@ -38,8 +38,8 @@ const isInsideCodeBlock = (node: Element): boolean => { const Details: React.FC<{ children: Array<{ props: { children: ReactNode } } | string> }> = ({ children }) => { - const filteredChildren = children.filter(child => child !== '\n') - const firstChild = filteredChildren[0] + const filteredChildren: ReactNode[] = children.filter(child => child !== '\n') + const firstChild = filteredChildren[0] as JSX.Element const triggerChildren: ReactNode[] = firstChild.props.children as ReactNode[] From d86b96e7736230ff2815c257a80094f163c2b414 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Mon, 31 Aug 2020 11:52:33 -0400 Subject: [PATCH 09/10] Refactor to only accept details with headings as the first child --- src/components/Documentation/Markdown/index.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/components/Documentation/Markdown/index.tsx b/src/components/Documentation/Markdown/index.tsx index 68be406e85..ddd4bea2b5 100644 --- a/src/components/Documentation/Markdown/index.tsx +++ b/src/components/Documentation/Markdown/index.tsx @@ -41,16 +41,19 @@ const Details: React.FC<{ const filteredChildren: ReactNode[] = children.filter(child => child !== '\n') const firstChild = filteredChildren[0] as JSX.Element - const triggerChildren: ReactNode[] = firstChild.props.children as ReactNode[] + if (!/^h.$/.test(firstChild.type)) { + throw new Error('The first child of a details element must be a heading!') + } /* - To work around auto-linked headings, the last child of any heading node + To work around auto-linked headings, the last child of the heading node must be removed. The only way around this is the change the autolinker, which we currently have as an external package. */ - if (/^h.$/.test(firstChild.type)) { - triggerChildren.pop() - } + const triggerChildren: ReactNode[] = firstChild.props.children.slice( + 1, + firstChild.props.children.length + ) as ReactNode[] /* Collapsible's trigger type wants ReactElement, so we force a TS cast from From 884dd91b663d3ad239520aecfed32d5ea56c4f93 Mon Sep 17 00:00:00 2001 From: rogermparent Date: Mon, 31 Aug 2020 12:13:13 -0400 Subject: [PATCH 10/10] Fix malformed slice from refactor --- src/components/Documentation/Markdown/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Documentation/Markdown/index.tsx b/src/components/Documentation/Markdown/index.tsx index ddd4bea2b5..625c35c169 100644 --- a/src/components/Documentation/Markdown/index.tsx +++ b/src/components/Documentation/Markdown/index.tsx @@ -51,8 +51,8 @@ const Details: React.FC<{ which we currently have as an external package. */ const triggerChildren: ReactNode[] = firstChild.props.children.slice( - 1, - firstChild.props.children.length + 0, + firstChild.props.children.length - 1 ) as ReactNode[] /*