Issue 1498

.github/workflows/ci.yml

@@ -75,7 +75,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        suite: [unit, browser, build]
+        suite: [unit, build]
     env:
       CI: true
     steps:

data/redirects/types.ts

@@ -1 +1,2 @@
-export type Redirects = Array<{ bookId: string, pageId: string, pathname: string }>;
+export type RedirectsData = Array<{ bookId: string, pageId: string, pathname: string }>;
+export type Redirects = Array<{ from: string, to: string }>;

package.json

@@ -64,15 +64,15 @@
     "pretest": "npm run-script build:css",
     "test": "npm run-script test:unit && npm run-script test:browser && npm run-script test:build",
     "test:unit": "REACT_APP_ENV=test jest --coverage --testPathPattern=\"(\\.|/)spec\\.tsx?\" && ./script/push-codecov.bash",
-    "test:browser": "npm run-script pretest:sims && REACT_APP_ENV=test jest --testPathPattern=\"(\\.|/)browserspec\\.tsx?\" -i",
-    "test:screenshots": "npm run-script pretest:sims && REACT_APP_ENV=test jest --testPathPattern=\"(\\.|/)screenshotspec\\.tsx?\" -i",
+    "test:browser": "npm run-script pretest:sims && REACT_APP_ENV=test jest --testPathPattern=\"(\\.|/)browserspec\\.tsx?\" --preset jest-puppeteer -i",
+    "test:screenshots": "npm run-script pretest:sims && REACT_APP_ENV=test jest --testPathPattern=\"(\\.|/)screenshotspec\\.tsx?\" --preset jest-puppeteer -i",
     "test:build": "REACT_APP_ENV=test npm run-script build:clean && npm run-script test:sourcemap && npm run-script test:prerender",
     "test:sourcemap": "REACT_APP_ENV=test node ./script/verifySourcemaps.js",
     "test:prerender": "npm run-script test:prerender:setup && npm run-script test:prerender:specs && npm run-script test:prerender:browser",
     "test:prerender:setup": "REACT_APP_ENV=test npm run-script prerender && npm run-script pretest:sims",
-    "test:prerender:specs": "REACT_APP_ENV=test SERVER_MODE=built jest --testPathPattern=\"(\\.|/)prerenderspec\\.tsx?\"",
-    "test:prerender:browser": "REACT_APP_ENV=test SERVER_MODE=built jest --testPathPattern=\"(\\.|/)browserspec\\.tsx?\" -i",
-    "test:prerender:screenshots": "REACT_APP_ENV=test SERVER_MODE=built jest --testPathPattern=\"(\\.|/)screenshotspec\\.tsx?\"",
+    "test:prerender:specs": "REACT_APP_ENV=test SERVER_MODE=built jest --testPathPattern=\"(\\.|/)prerenderspec\\.tsx?\" --preset jest-puppeteer",
+    "test:prerender:browser": "REACT_APP_ENV=test SERVER_MODE=built jest --testPathPattern=\"(\\.|/)browserspec\\.tsx?\" --preset jest-puppeteer -i",
+    "test:prerender:screenshots": "REACT_APP_ENV=test SERVER_MODE=built jest --testPathPattern=\"(\\.|/)screenshotspec\\.tsx?\" --preset jest-puppeteer",
     "test:lighthouse-manual": "REACT_APP_ENV=test lighthouse --view --config-path=./src/test/audits/index.js",
     "analyze:bundle": "react-scripts build && source-map-explorer 'build/static/js/*.js' -m",
     "analyze:dom": "node ./script/entry.js domVisitor",
@@ -163,7 +163,6 @@
     "@sentry/browser": "5.6.1"
   },
   "jest": {
-    "preset": "jest-puppeteer",
     "testEnvironment": "jsdom",
     "moduleFileExtensions": [
       "ts",

script/entry.js

@@ -1,18 +1,12 @@
 const fs = require('fs');
-const fetch = require('node-fetch');
 const path = require('path');
 const identity = require('lodash/fp/identity');
 const script = process.argv[2];
 const scriptPath = `./${script}`;
 const extensions = ['.ts', '.tsx'];
+const requireBabelConfig = require('../src/babel-config');
 
-// lots of stuff relies on this
-const JSDOM = require('jsdom').JSDOM;
-global.DOMParser = new JSDOM().window.DOMParser;
-
-const URL = require('url');
-global.URL = URL.URL;
-global.fetch = fetch;
+requireBabelConfig(extensions);
 
 if (!script) {
   console.error('script argument is required');
@@ -29,21 +23,4 @@ if (!exists) {
   process.exit(1);
 }
 
-require('@babel/register')({
-  ignore: [/node_modules/],
-  extensions: ['.js', '.jsx', ...extensions],
-  presets: [
-    '@babel/preset-env',
-    '@babel/preset-typescript',
-    '@babel/preset-react',
-  ],
-  plugins: [
-    'macros',
-    '@babel/proposal-class-properties',
-    '@babel/proposal-object-rest-spread',
-    '@babel/transform-runtime',
-    'babel-plugin-transform-dynamic-import',
-  ]
-});
-
 require(scriptPath);

script/prerender/createRedirects.ts

@@ -1,51 +1,8 @@
-import fs from 'fs';
-import path from 'path';
-import { Redirects } from '../../data/redirects/types';
-import { content } from '../../src/app/content/routes';
-import { makeUnifiedBookLoader } from '../../src/app/content/utils';
-import { findArchiveTreeNodeById } from '../../src/app/content/utils/archiveTreeUtils';
 import { AppServices } from '../../src/app/types';
-import config from '../../src/config.books';
+import prepareRedirects from '../utils/prepareRedirects';
 import { writeAssetFile } from './fileUtils';
 
-const redirectsPath = path.resolve(__dirname, '../../data/redirects/');
-
-const createRedirects = async(archiveLoader: AppServices['archiveLoader'], osWebLoader: AppServices['osWebLoader']) => {
-  const bookLoader = makeUnifiedBookLoader(archiveLoader, osWebLoader);
-
-  const books = fs.readdirSync(redirectsPath).filter((name) => name.match('.json'));
-
-  const redirects: Array<{ from: string, to: string }> = [];
-
-  for (const fileName of books) {
-    const bookRedirects: Redirects = await import(`${redirectsPath}/${fileName}`);
-
-    for (const { bookId, pageId, pathname } of bookRedirects) {
-      const configForBook: { defaultVersion: string } | undefined = config[bookId];
-
-      if (!configForBook) {
-        // tslint:disable-next-line: no-console
-        console.log(`Couldn't find version for book: ${bookId}`);
-        continue;
-      }
-
-      const { tree, slug: bookSlug } = await bookLoader(bookId, configForBook.defaultVersion);
-      const page = findArchiveTreeNodeById(tree, pageId);
-
-      if (!page) {
-        // tslint:disable-next-line: no-console
-        console.log(`Couldn't find page ${pageId} in book ${bookId}`);
-        continue;
-      }
-
-      redirects.push({
-        from: pathname,
-        to: content.getUrl({ book: { slug: bookSlug }, page: { slug: page.slug } }),
-      });
-    }
-  }
-
+export default async(archiveLoader: AppServices['archiveLoader'], osWebLoader: AppServices['osWebLoader']) => {
+  const redirects = await prepareRedirects(archiveLoader, osWebLoader);
   writeAssetFile('/rex/redirects.json', JSON.stringify(redirects, undefined, 2));
 };
-
-export default createRedirects;

script/update-content-versions-and-check-for-archived-slugs.ts

@@ -2,7 +2,7 @@ import fs from 'fs';
 import isEqual from 'lodash/fp/isEqual';
 import path from 'path';
 import { argv } from 'yargs';
-import { Redirects } from '../data/redirects/types';
+import { RedirectsData } from '../data/redirects/types';
 import { content } from '../src/app/content/routes';
 import { LinkedArchiveTreeNode } from '../src/app/content/types';
 import { flattenArchiveTree } from '../src/app/content/utils';
@@ -46,7 +46,7 @@ async function updateRedirections(bookId: string, currentVersion: string, newVer
     });
 
   const redirectsBookPath = path.resolve(redirectsPath, bookId + '.json');
-  const redirects: Redirects = fs.existsSync(redirectsBookPath) ? await import(redirectsBookPath) : [];
+  const redirects: RedirectsData = fs.existsSync(redirectsBookPath) ? await import(redirectsBookPath) : [];
 
   const flatCurrentTree = flattenArchiveTree(currentTree);
 

script/utils/prepareRedirects.ts

@@ -0,0 +1,53 @@
+import fs from 'fs';
+import path from 'path';
+import { RedirectsData } from '../../data/redirects/types';
+import { content } from '../../src/app/content/routes';
+import { makeUnifiedBookLoader } from '../../src/app/content/utils';
+import { findArchiveTreeNodeById } from '../../src/app/content/utils/archiveTreeUtils';
+import { AppServices } from '../../src/app/types';
+import config from '../../src/config.books';
+
+const redirectsPath = path.resolve(__dirname, '../../data/redirects/');
+
+const prepareRedirects = async(
+  archiveLoader: AppServices['archiveLoader'],
+  osWebLoader: AppServices['osWebLoader']
+) => {
+  const bookLoader = makeUnifiedBookLoader(archiveLoader, osWebLoader);
+
+  const books = fs.readdirSync(redirectsPath).filter((name) => name.match('.json'));
+
+  const redirects: Array<{ from: string, to: string }> = [];
+
+  for (const fileName of books) {
+    const bookRedirects: RedirectsData = await import(`${redirectsPath}/${fileName}`);
+
+    for (const { bookId, pageId, pathname } of bookRedirects) {
+      const configForBook: { defaultVersion: string } | undefined = config[bookId];
+
+      if (!configForBook) {
+        // tslint:disable-next-line: no-console
+        console.log(`Couldn't find version for book: ${bookId}`);
+        continue;
+      }
+
+      const { tree, slug: bookSlug } = await bookLoader(bookId, configForBook.defaultVersion);
+      const page = findArchiveTreeNodeById(tree, pageId);
+
+      if (!page) {
+        // tslint:disable-next-line: no-console
+        console.log(`Couldn't find page ${pageId} in book ${bookId}`);
+        continue;
+      }
+
+      redirects.push({
+        from: pathname,
+        to: content.getUrl({ book: { slug: bookSlug }, page: { slug: page.slug } }),
+      });
+    }
+  }
+
+  return redirects;
+};
+
+export default prepareRedirects;

src/app/content/hooks/index.ts

@@ -8,12 +8,14 @@ import searchHooks from '../search/hooks';
 import studyGuidesHooks from '../studyGuides/hooks';
 import locationChangeBody from './locationChange';
 import receiveContentBody from './receiveContent';
+import receivePageNotFoundId from './receivePageNotFoundId';
 
 export default [
   ...searchHooks,
   ...highlightHooks,
   ...studyGuidesHooks,
   ...practiceQuestionsHooks,
+  receivePageNotFoundId,
   routeHook(routes.content, locationChangeBody),
   actionHook(actions.receivePage, receiveContentBody),
 ];

src/app/content/hooks/receivePageNotFoundId.spec.ts

@@ -0,0 +1,68 @@
+import createTestServices from '../../../test/createTestServices';
+import createTestStore from '../../../test/createTestStore';
+import { MiddlewareAPI, Store } from '../../types';
+import { receivePageNotFoundId } from '../actions';
+
+const mockFetch = (valueToReturn: any, error?: any) => () => new Promise((resolve, reject) => {
+  if (error) {
+    reject(error);
+  }
+  resolve({ json: () => new Promise((res) => res(valueToReturn)) });
+});
+
+describe('receivePageNotFoundId hook', () => {
+  let hook: ReturnType<typeof import ('./receivePageNotFoundId').receivePageNotFoundIdHookBody>;
+  let store: Store;
+  let helpers: MiddlewareAPI & ReturnType<typeof createTestServices>;
+  let historyReplaceSpy: jest.SpyInstance;
+  let fetchBackup: any;
+
+  beforeEach(() => {
+    store = createTestStore();
+
+    helpers = {
+      ...createTestServices(),
+      dispatch: store.dispatch,
+      getState: store.getState,
+    };
+
+    helpers.history.location = {
+      pathname: '/books/physics/pages/1-introduction301',
+    } as any;
+
+    historyReplaceSpy = jest.spyOn(helpers.history, 'replace')
+      .mockImplementation(jest.fn());
+
+    fetchBackup = (globalThis as any).fetch;
+
+    hook = require('./receivePageNotFoundId').receivePageNotFoundIdHookBody(helpers);
+  });
+
+  afterEach(() => {
+    (globalThis as any).fetch = fetchBackup;
+  });
+
+  it('checks for redirects when receivePageNotFoundId is dispatched and noops if path wasn\'t found', async() => {
+    (globalThis as any).fetch = mockFetch([{ from: 'asd', to: 'asd' }]);
+
+    await hook(receivePageNotFoundId('asdf'));
+
+    expect(historyReplaceSpy).not.toHaveBeenCalled();
+  });
+
+  it('noops if fetch fails', async() => {
+    (globalThis as any).fetch = mockFetch(undefined, 'error');
+
+    await hook(receivePageNotFoundId('asdf'));
+
+    expect(historyReplaceSpy).not.toHaveBeenCalled();
+  });
+
+  it('calls history.replace if redirect is found', async() => {
+    (globalThis as any).fetch = mockFetch([{ from: helpers.history.location.pathname, to: 'redirected' }]);
+
+    await hook(receivePageNotFoundId('asdf'));
+
+    expect(historyReplaceSpy).toHaveBeenCalledWith('redirected');
+  });
+});

src/app/content/hooks/receivePageNotFoundId.ts

@@ -0,0 +1,21 @@
+import { Redirects } from '../../../../data/redirects/types';
+import { ActionHookBody } from '../../types';
+import { actionHook } from '../../utils';
+import { receivePageNotFoundId } from '../actions';
+
+export const receivePageNotFoundIdHookBody: ActionHookBody<typeof receivePageNotFoundId> = (
+  services
+) => async() => {
+  const redirects: Redirects = await fetch('/rex/redirects.json')
+    .then((res) => res.json())
+    .catch(() => []);
+
+  for (const {from, to} of redirects) {
+    if (from === services.history.location.pathname) {
+      services.history.replace(to);
+      return;
+    }
+  }
+};
+
+export default actionHook(receivePageNotFoundId, receivePageNotFoundIdHookBody);

src/babel-config.js

@@ -0,0 +1,28 @@
+module.exports = function(additionalExtensions = ['.ts', '.tsx']) {
+  const fetch = require('node-fetch');
+
+  // lots of stuff relies on this
+  const JSDOM = require('jsdom').JSDOM;
+  global.DOMParser = new JSDOM().window.DOMParser;
+
+  const URL = require('url');
+  global.URL = URL.URL;
+  global.fetch = fetch;
+
+  require('@babel/register')({
+    ignore: [/node_modules/],
+    extensions: ['.js', '.jsx', ...additionalExtensions],
+    presets: [
+      '@babel/preset-env',
+      '@babel/preset-typescript',
+      '@babel/preset-react',
+    ],
+    plugins: [
+      'macros',
+      '@babel/proposal-class-properties',
+      '@babel/proposal-object-rest-spread',
+      '@babel/transform-runtime',
+      'babel-plugin-transform-dynamic-import',
+    ]
+  });
+};

src/redirects.development.json

@@ -0,0 +1,3 @@
+[
+  { "from": "/books/physics/pages/1-introduction301", "to": "/books/physics/pages/1-introduction" }
+]