Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

Commit

Permalink
Fix Notion migrations not working in when served by Netlify. (#8960)
Browse files Browse the repository at this point in the history
Found why the redirections are not working in prod. In local, NextJS
handles the redirections (logical), but in prod, we let Netlify do it
(logical, as it's serving static content).

But, the semantics in between the two are not the same: 

- NextJS sets up the redirection "normally" 
- Netlify instead, will flat out ignore the redirection if the page
exists.

So the fix was to patch the code that generates the `_redirects` file
that Netlify needs, to add the `301!` in `/source https://where 301!`
required for it to stop ignoring those.

I really didn't see coming the different semantics here :/

---------

Co-authored-by: jhchabran <[email protected]>
  • Loading branch information
jhchabran and jhchabran authored Jun 20, 2024
1 parent 111ca71 commit 49bc1de
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 7 deletions.
2 changes: 1 addition & 1 deletion data/notion_migration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# - 'source': URL path to redirect, as currently visible on handbook.sourcegraph.com
# - 'destination': New URL top direct to. If a public 'sourcegraph.notion.site' page is available, prefer that instead of the internal Notion URL.
redirections:
- source: /departments/engineering/teams/devinfra
- source: /departments/engineering/teams/devinfra/
destination: https://sourcegraph.notion.site/Developer-Infrastructure-Team-a46433b93bb2445abc1966c93a570a26
# Core Services
- source: /departments/engineering/teams/core-services/
Expand Down
4 changes: 3 additions & 1 deletion src/scripts/generate-redirects.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import fs from 'fs/promises'

import redirects from './redirects.mjs'

const redirectLines = (await redirects()).map(({ source, destination }) => `${source} ${destination}`).join('\n')
const redirectLines = (await redirects())
.map(entry => `${entry.source} ${entry.destination}${entry.force ? ' 301!' : ''}`)
.join('\n')
const lines = `\n# Generated by generate-redirects.mjs from Git history\n${redirectLines}\n`

console.log('Redirects:\n', lines)
Expand Down
19 changes: 14 additions & 5 deletions src/scripts/redirects.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,21 @@ export function cleanupRedirects(movedPages) {
/**
* Reads Notion specific redirections from data/notion_migration.yaml.
*
* @returns {redirections: {source: string, destination: string}[]}
* @returns {redirections: {source: string, destination: string, force: true}[]}
*/
async function readNotionMigrationRedirects() {
return load(await readFile('data/notion_migration.yaml', 'utf8'))
const data = load(await readFile('data/notion_migration.yaml', 'utf8'))

// While NextJS is okay if content exists when setting up a redirection,
// Netlify doesn't: it will skip the redirection entirely if it finds
// existing content.
// To go around that, we add a new field 'force' that the script that
// generate the final _redirects file used by Netlify uses to append
// 301! on that entry, effectively forcing the redirection.
for (const entry of data.redirections) {
entry.force = true
}
return data
}

/**
Expand All @@ -47,12 +58,10 @@ export default async function redirects() {
const notionRedirections = await readNotionMigrationRedirects()
return cleanupRedirects([
...movedPages,
...notionRedirections.redirections,

// Add custom redirects
{
source: '/careers',
destination: 'https://about.sourcegraph.com/jobs',
},
])
]).concat(notionRedirections.redirections) // we skip the cleanup because notion redirects are always going outside.
}

0 comments on commit 49bc1de

Please sign in to comment.