Redirect checks ported to JavaScript + unit tests (#1761)

* create safe_rename in JS

* Adding JS safe redirects commit.

Across the two checks (Python & JS) one should find not redirects based on the work the other has done.

We can run these in parallel for a while to ensure they both work in the same way.

JS checks include some unit tests.

Co-authored-by: Sam Winslow <sammywinslow@gmail.com>

.github/workflows/checks.yml

@@ -4,7 +4,7 @@ on:
     - pull_request
 
 jobs:
-    spell-check:
+    standard_checks:
         name: Standard Checks
         runs-on: ubuntu-latest
         if: github.event.pull_request.head.repo.full_name == github.repository
@@ -52,3 +52,30 @@ jobs:
             - name: Push changes
               run: |
                   git push
+
+    js_checks:
+        name: JS Checks
+        runs-on: ubuntu-latest
+        steps:
+            - uses: actions/checkout@v2
+            - uses: actions/setup-node@v2
+              with:
+                  node-version: '16'
+            - name: Install dependencies
+              run: yarn
+            - name: Run tests
+              run: yarn test-generate-redirects
+            - name: Configure Git
+              run: |
+                  git fetch origin master:master
+                  git config user.name PostHog Bot
+                  git config user.email hey@posthog.com
+            - name: Add safe redirects
+              if: ${{ github.base_ref == 'master' }}
+              run: yarn generate-redirects-js
+            - name: Cleanup and commit
+              if: ${{ github.base_ref == 'master' }}
+              run: |
+                  rm pr_diff
+                  git add .
+                  [ "$(git status -s)" = "" ] && exit 0 || git commit -m "Add safe redirects (JS)"

package.json

@@ -18,7 +18,9 @@
         "test": "echo \"Error: no test specified\" && exit 1",
         "typegen": "kea-typegen write .",
         "update-sprite": "svg-sprite -s --symbol-dest src/components/productFeature/images/icons --symbol-sprite sprited-icons.svg src/components/productFeature/images/icons/*.svg",
-        "generate-redirects": "rm pr_diff && git fetch && git diff origin/master $(git branch --show-current) > pr_diff && python3 ./scripts/safe_rename.py"
+        "generate-redirects": "rm pr_diff && git fetch && git diff origin/master $(git branch --show-current) > pr_diff && python3 ./scripts/safe_rename.py",
+        "generate-redirects-js": "git diff origin/master $(git branch --show-current) > pr_diff && node ./scripts/safe_rename.js RUN_AS_SCRIPT=true",
+        "test-generate-redirects": "jest"
     },
     "dependencies": {
         "@ant-design/icons": "^4.1.0",
@@ -104,6 +106,7 @@
         "eslint": "^7.9.0",
         "eslint-plugin-react": "^7.20.6",
         "husky": "^4.2.5",
+        "jest": "^27.0.6",
         "lint-staged": "^10.2.12",
         "mem": "^5.1.1",
         "prettier": "^2.1.0",

scripts/safe_rename.js

@@ -0,0 +1,142 @@
+/* eslint-disable @typescript-eslint/no-var-requires */
+const fs = require('fs')
+const fetch = require('node-fetch')
+
+let DEBUG = false
+
+const log = (...args) => {
+    if (DEBUG) {
+        console.log.apply(null, args)
+    }
+}
+
+const formatDate = (date) => {
+    return `${date.getFullYear()}-${date.getMonth() + 1}-${date.getDate()}`
+}
+
+const redirectText = (from, to) => {
+    return `
+[[redirects]]
+    from = "${from}"
+    to = "${to}"
+`
+}
+
+const redirectWithDateCommentText = (date, redirect) => {
+    return `
+# Added: ${formatDate(date)}${redirect}`
+}
+
+// Rules
+const samePath = ({ fromPath, toPath }) => {
+    return fromPath === toPath
+}
+
+const mdToMdx = ({ fromPath, toPath }) => {
+    // # old rule
+    // # return '.mdx' not in from_paths[i] and '.mdx' in to_paths[i]
+    return fromPath.endsWith('.md') && toPath.endsWith('.mdx')
+}
+
+const redirectExists = ({ redirect, localConfig, remoteConfig }) => {
+    return localConfig.indexOf(redirect.trim()) !== -1 || remoteConfig.indexOf(redirect.trim()) !== -1
+}
+
+const fromDotStar = ({ fromPath }) => {
+    return fromPath == '(.*)'
+}
+
+const isSnippetsRename = ({ fromPath, toPath }) => {
+    return fromPath.indexOf('/snippets/') !== -1 || toPath.indexOf('/snippets') !== -1
+}
+
+const skipRules = {
+    samePath,
+    mdToMdx,
+    redirectExists,
+    fromDotStar,
+    isSnippetsRename,
+}
+
+// # Load existing remote redirect config
+const getRemoteConfig = async () => {
+    const response = await fetch('https://raw.githubusercontent.com/PostHog/posthog.com/master/netlify.toml')
+    return await response.text()
+}
+
+// # Load existing redirect file to be used to avoid duplicates
+const getLocalConfig = async () => {
+    return await fs.promises.readFile('./netlify.toml', 'utf8')
+}
+
+const appendToLocalConfig = async (newRedirects) => {
+    await fs.promises.appendFile('./netlify.toml', newRedirects)
+}
+
+const getRedirects = async ({ gitDiff, localConfig, remoteConfig, debug = false }) => {
+    DEBUG = debug
+    log(gitDiff)
+
+    const renameFromRegex = new RegExp('rename from contents(.*).md', 'g')
+    const renameToRegex = new RegExp('rename to contents(.*).md', 'g')
+
+    const fromPaths = Array.from(gitDiff.matchAll(renameFromRegex), (m) => m[1])
+    const toPaths = Array.from(gitDiff.matchAll(renameToRegex), (m) => m[1])
+
+    log(fromPaths, toPaths)
+
+    const newRedirects = []
+    if (fromPaths.length > 0 && fromPaths.length === toPaths.length) {
+        for (let i = 0; i < fromPaths.length; ++i) {
+            // handle index default directory files. /path/index will become /path
+            const fromPath = fromPaths[i].replace(/\/index$/, '', fromPaths[i])
+            const toPath = toPaths[i].replace(/\/index$/, '', toPaths[i])
+
+            const redirect = redirectText(fromPath, toPath)
+            // console.log(redirect)
+
+            log(`Testing if redirects are required for: "${fromPath}" to "${toPath}"`)
+
+            let skipRedirect = false
+            for (const [skipRuleName, skipRuleFunction] of Object.entries(skipRules)) {
+                if (skipRedirect === false) {
+                    skipRedirect = skipRuleFunction({ fromPath, toPath, redirect, localConfig, remoteConfig })
+                    log(`Rule: "${skipRuleName}"', 'skipRedirect?', ${skipRedirect}`)
+                }
+            }
+
+            if (skipRedirect === false) {
+                newRedirects.push(redirectWithDateCommentText(new Date(), redirect))
+            } else {
+                log('Not including redirect', redirect)
+            }
+        }
+    }
+
+    log(newRedirects)
+    return newRedirects
+}
+
+const main = async () => {
+    try {
+        const gitDiff = await fs.promises.readFile('./pr_diff', 'utf8')
+
+        const remoteConfig = await getRemoteConfig()
+        const localConfig = await getLocalConfig()
+        const redirects = getRedirects({ gitDiff, localConfig, remoteConfig })
+
+        if (redirects.length > 0) {
+            appendToLocalConfig(redirects.join(''))
+        }
+    } catch (error) {
+        console.error(error)
+    }
+}
+
+if (process.env.RUN_AS_SCRIPT === 'true') {
+    main()
+}
+
+module.exports = {
+    getRedirects,
+}

scripts/safe_rename.test.js

@@ -0,0 +1,144 @@
+/* eslint-disable @typescript-eslint/no-var-requires */
+const getRedirects = require('./safe_rename').getRedirects
+
+const debug = false
+
+const singleRenameGitDiff = `
+rename from contents/docs/one.md
+rename to contents/docs/two.md`
+
+const multiRenameGitDiff = `
+rename from contents/docs/one.md
+rename to contents/docs/two.md
+
+rename from contents/docs/cheese.md
+rename to contents/docs/mushroom.md
+
+rename from contents/docs/fish.md
+rename to contents/docs/monkey.md`
+
+const emptyConfig = ``
+const existingRedirectConfig = `
+# Added: 2021-08-03
+[[redirects]]
+    from = "/docs/one"
+    to = "/docs/two"`
+
+test('redirect will be added for single file rename', async () => {
+    const gitDiff = singleRenameGitDiff
+
+    const localConfig = emptyConfig
+    const remoteConfig = emptyConfig
+
+    const redirects = await getRedirects({ gitDiff, localConfig, remoteConfig, debug })
+
+    expect(redirects.length).toBe(1)
+    expect(redirects[0]).toContain('from = "/docs/one"')
+    expect(redirects[0]).toContain('to = "/docs/two"')
+})
+
+test('redirect will be added for multi file rename', async () => {
+    const gitDiff = multiRenameGitDiff
+
+    const localConfig = emptyConfig
+    const remoteConfig = emptyConfig
+
+    const redirects = await getRedirects({ gitDiff, localConfig, remoteConfig, debug })
+
+    expect(redirects.length).toBe(3)
+    expect(redirects[0]).toContain('from = "/docs/one"')
+    expect(redirects[0]).toContain('to = "/docs/two"')
+    expect(redirects[1]).toContain('from = "/docs/cheese"')
+    expect(redirects[1]).toContain('to = "/docs/mushroom"')
+    expect(redirects[2]).toContain('from = "/docs/fish"')
+    expect(redirects[2]).toContain('to = "/docs/monkey"')
+})
+
+test('redirect will only be added for non-existing redirects in multi file rename', async () => {
+    const gitDiff = multiRenameGitDiff
+
+    const localConfig = emptyConfig
+    const remoteConfig = existingRedirectConfig
+
+    const redirects = await getRedirects({ gitDiff, localConfig, remoteConfig, debug })
+
+    expect(redirects.length).toBe(2)
+    expect(redirects[0]).toContain('from = "/docs/cheese"')
+    expect(redirects[0]).toContain('to = "/docs/mushroom"')
+    expect(redirects[1]).toContain('from = "/docs/fish"')
+    expect(redirects[1]).toContain('to = "/docs/monkey"')
+})
+
+test('redirect will not be added if it already exists in the remote config', async () => {
+    const gitDiff = singleRenameGitDiff
+
+    const localConfig = emptyConfig
+    const remoteConfig = existingRedirectConfig
+
+    const redirects = await getRedirects({ gitDiff, localConfig, remoteConfig, debug })
+
+    expect(redirects.length).toBe(0)
+})
+
+test('redirect will not be added if it already exists in the local config', async () => {
+    const gitDiff = singleRenameGitDiff
+
+    const localConfig = existingRedirectConfig
+    const remoteConfig = emptyConfig
+
+    const redirects = await getRedirects({ gitDiff, localConfig, remoteConfig, debug })
+
+    expect(redirects.length).toBe(0)
+})
+
+test('redirect will not be added if rename from /name to /name/index.mdx', async () => {
+    const gitDiff = `
+    rename from contents/docs/filename
+    rename to contents/docs/name/index.mdx`
+
+    const localConfig = emptyConfig
+    const remoteConfig = emptyConfig
+
+    const redirects = await getRedirects({ gitDiff, localConfig, remoteConfig, debug })
+
+    expect(redirects.length).toBe(0)
+})
+
+test('redirect will not be added if rename from /name to /name/index.md', async () => {
+    const gitDiff = `
+    rename from contents/docs/name
+    rename to contents/docs/name/index.md`
+
+    const localConfig = emptyConfig
+    const remoteConfig = emptyConfig
+
+    const redirects = await getRedirects({ gitDiff, localConfig, remoteConfig, debug })
+
+    expect(redirects.length).toBe(0)
+})
+
+test('redirect will not be added if rename from .md to .mdx', async () => {
+    const gitDiff = `
+    rename from contents/docs/filename.md
+    rename to contents/docs/filename.mdx`
+
+    const localConfig = emptyConfig
+    const remoteConfig = emptyConfig
+
+    const redirects = await getRedirects({ gitDiff, localConfig, remoteConfig, debug })
+
+    expect(redirects.length).toBe(0)
+})
+
+test('redirect will not be added when renaming snippets', async () => {
+    const gitDiff = `
+    rename from contents/docs/snippets/js/capture.mdx
+    rename to contents/docs/snippets/js/special-capture.mdx`
+
+    const localConfig = emptyConfig
+    const remoteConfig = emptyConfig
+
+    const redirects = await getRedirects({ gitDiff, localConfig, remoteConfig, debug })
+
+    expect(redirects.length).toBe(0)
+})