Add tests for internal linking (#808)

* Add tests for internal linking

* Add workflow for unit tests

* Use jest

.github/workflows/test.yml

@@ -0,0 +1,20 @@
+name: Tests
+on: [pull_request]
+
+jobs:
+  unit:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+
+      - name: Setup Node.js
+        uses: actions/setup-node@v3
+        with:
+          node-version: "18.17.0"
+
+      - name: Install
+        run: npm install
+
+      - name: Test
+        run: npm test

components/text.js

@@ -13,7 +13,7 @@ import Thumb from '../svgs/thumb-up-fill.svg'
 import { toString } from 'mdast-util-to-string'
 import copy from 'clipboard-copy'
 import ZoomableImage, { decodeOriginalUrl } from './image'
-import { IMGPROXY_URL_REGEXP } from '../lib/url'
+import { IMGPROXY_URL_REGEXP, parseInternalLinks } from '../lib/url'
 import reactStringReplace from 'react-string-replace'
 import { rehypeInlineCodeProperty } from '../lib/md'
 import { Button } from 'react-bootstrap'
@@ -186,31 +186,12 @@ export default memo(function Text ({ nofollow, imgproxyUrls, children, tab, item
             }
 
             try {
-              // parse internal links and show as #<itemId>
-              const url = new URL(href)
-              const { pathname, searchParams } = url
-              // ignore empty parts which exist due to pathname starting with '/'
-              const emptyPart = part => !!part
-              const parts = pathname.split('/').filter(emptyPart)
-              if (parts[0] === 'items' && /^[0-9]+$/.test(parts[1])) {
-                const itemId = parts[1]
-                // check for valid item page due to referral links like /items/123456/r/ekzyis
-                const itemPages = ['edit', 'ots', 'related']
-                const itemPage = itemPages.includes(parts[2]) ? parts[2] : null
-                if (itemPage) {
-                  // parse   https://stacker.news/items/1/related?commentId=2
-                  // as      #1/related
-                  // and not #2
-                  // since commentId will be ignored anyway
-                  const linkText = `#${itemId}/${itemPage}`
-                  return <a target='_blank' href={href} rel='noreferrer'>{linkText}</a>
-                }
-                const commentId = searchParams.get('commentId')
-                const linkText = `#${commentId || itemId}`
+              const linkText = parseInternalLinks(href)
+              if (linkText) {
                 return <a target='_blank' href={href} rel='noreferrer'>{linkText}</a>
               }
             } catch {
-              // ignore invalid URLs
+              // ignore errors like invalid URLs
             }
 
             // if the link is to a youtube video, render the video

lib/url.js

@@ -23,6 +23,34 @@ export function removeTracking (value) {
   return value
 }
 
+/**
+ * parse links like https://stacker.news/items/123456 as #123456
+ */
+export function parseInternalLinks (href) {
+  const url = new URL(href)
+  const { pathname, searchParams } = url
+  // ignore empty parts which exist due to pathname starting with '/'
+  const emptyPart = part => !!part
+  const parts = pathname.split('/').filter(emptyPart)
+  if (parts[0] === 'items' && /^[0-9]+$/.test(parts[1])) {
+    const itemId = parts[1]
+    // check for valid item page due to referral links like /items/123456/r/ekzyis
+    const itemPages = ['edit', 'ots', 'related']
+    const itemPage = itemPages.includes(parts[2]) ? parts[2] : null
+    if (itemPage) {
+      // parse   https://stacker.news/items/1/related?commentId=2
+      // as      #1/related
+      // and not #2
+      // since commentId will be ignored anyway
+      const linkText = `#${itemId}/${itemPage}`
+      return linkText
+    }
+    const commentId = searchParams.get('commentId')
+    const linkText = `#${commentId || itemId}`
+    return linkText
+  }
+}
+
 export function stripTrailingSlash (uri) {
   return uri.endsWith('/') ? uri.slice(0, -1) : uri
 }

lib/url.spec.js

@@ -0,0 +1,28 @@
+/* eslint-env jest */
+
+import { parseInternalLinks } from './url.js'
+
+const cases = [
+  ['https://stacker.news/items/123', '#123'],
+  ['https://stacker.news/items/123/related', '#123/related'],
+  // invalid links should not be parsed so user can spot error
+  ['https://stacker.news/items/123foobar', undefined],
+  // parse referral links
+  ['https://stacker.news/items/123/r/ekzyis', '#123'],
+  // use comment id if available
+  ['https://stacker.news/items/123?commentId=456', '#456'],
+  // comment id + referral link
+  ['https://stacker.news/items/123/r/ekzyis?commentId=456', '#456'],
+  // multiple params
+  ['https://stacker.news/items/123?commentId=456&parentId=789', '#456']
+]
+
+describe('internal links', () => {
+  test.each(cases)(
+    'parses %p as %p',
+    (href, expected) => {
+      const actual = parseInternalLinks(href)
+      expect(actual).toBe(expected)
+    }
+  )
+})

package.json

@@ -8,6 +8,7 @@
     "migrate": "prisma migrate deploy",
     "start": "NODE_OPTIONS='--trace-warnings' next start -p $PORT --keepAliveTimeout 120000",
     "lint": "standard",
+    "test": "NODE_OPTIONS='--experimental-vm-modules' jest",
     "worker": "tsx --trace-warnings worker/index.js"
   },
   "dependencies": {
@@ -115,6 +116,7 @@
   "devDependencies": {
     "@next/eslint-plugin-next": "^13.5.4",
     "eslint": "^8.51.0",
+    "jest": "^29.7.0",
     "standard": "^17.1.0"
   }
 }