Skip to content

Commit

Permalink
Issue 1498 (#1118)
Browse files Browse the repository at this point in the history
* Add getLastObservedReleaseId

* Pull out writing file logic from createRedirects.ts

* Redirect on locationChange if url was found in the redirects.json file

* Add support for developments version fo redirects json

* Create prepareRedirects function from createRedirects

* It is not working yet, but I am commiting so I can go back to this

* revert changes

* new way of handling redirects

* Add tests

* Pull out writing file logic from createRedirects.ts

* Create prepareRedirects function from createRedirects

* Create redirects on the fly

* move prepareRedirects to scripts folder

* dont forget about required presets and plugins

* Try increasing file watchers

* share babel config

* Revert "Try increasing file watchers"

This reverts commit 3067341.

* Move prepareRedirects outside of the middleware

* mock implementation for history replace

* increase number of watches

* try without sudo

* remove command for increasing file watches

* Check if this helps

* Check if requiring babel config twice breaks something

* Does requiring more stuff cause error

* What if i import only prepareRedirects?

* Uncomment everything in setupProxy - error is caused by prepareRedirects function

* Instead of reading all files in data/redirects/, read list off books from books.json

* Revert "Instead of reading all files in data/redirects/, read list off books from books.json"

This reverts commit 242ed7e.

* mock prepareRedirects to check if it is really realted to this function

* Revert "mock prepareRedirects to check if it is really realted to this function"

This reverts commit d638bdd.

* What if we require only createArchiveLoader

* And what about importing only createOSWebLoader

* Uncomment import of prepareRedirects

* change prepareRedirects to always return []

* Revert "change prepareRedirects to always return []"

This reverts commit c2d4905.

* Instead of reading all files in data/redirects/, read list off books from books.json

* Revert "Instead of reading all files in data/redirects/, read list off books from books.json"

This reverts commit 253212a.

* Check if this is about dynamic imports

* Check if importing content route is breaking something

* Run only Test CI job

* remove whole for loop from preprareRedirects

* setup job is still required

* Check if removing makeUnifiedBookLoader from prepareRedirects will help

* Reset to commit 662a2d2

* narrow ci to run only test and setup jobs

* Remove stripIdVersion from createArchiveLoader

* Import only createArchiveLoader

* Import only createArchiveLoader

* Import also prepareRedirects

* remove makeUnifiedBookLoader from prepareRedirects

* remove findArchiveTreeNodeById from prepareRedirects

* use stripIdVersion in createArchiveLoader again

* Import stripIdVersion from idUtils

* i didnt want to import findArchiveTreeNodeById in prepareRedirects

* import only prepareRedirects

* Display max_user_watches limit on gh

* Add modulePathIgnorePatterns to jest config

* Revert "Add modulePathIgnorePatterns to jest config"

This reverts commit 8b9d863.

* Try adding watchmanconfig - jest uses it by default

* Try adding hint_num_dirs to watchmanconfig

* Reset to 662a2d2

* remove watchmanconfig

* Dont remove util module even if it is not used

* Add sysctls settings to test job container

* rearange sysctls option

* nest sysctls in options

* try other syntax

* one more try

* try adding privileged to the options

* add alowed-unsafe-sysctls

* revert ci.yml

* Add --watchAll=false flag to test:unit

* Revert "Add --watchAll=false flag to test:unit"

This reverts commit fdc7bd8.

* remove -i flag from test:browser

* Revert "remove -i flag from test:browser"

This reverts commit aea0b2c.

* add logs to check what is triggering inotify

* Revert "add logs to check what is triggering inotify"

This reverts commit 411fadd.

* jest watchman: false

* Revert "jest watchman: false"

This reverts commit 5abc7e9.

* Add SKIP_PREFLIGHT_CHECK=true SERVER_MODE=built to test:unit and set jest resolution to 25.1.0

* Leave only SERVER_MODE=built and add it to test:browser

* just use yarn server instead of yarn start in jest-puppeteer.config.js

* Add and configure cracto to fix inotify limit issue

* Use another config for test:unit

* Run test:browser after test:unit/build is finished

* Remove default jest preset and use it in commands

* move browser job to matrix again

* make sure that craco options that wasnt working locally does not work also on CI

* Ignore all files

* Ignore only node_modules files

* Remove craco and do not test:browser in ci

* test:screenshots should use jest-puppeteer preset

Co-authored-by: staxly[bot] <35789409+staxly[bot]@users.noreply.github.com>
  • Loading branch information
PiotrKozlowski and staxly[bot] committed Mar 29, 2021
1 parent f46d390 commit 817a042
Show file tree
Hide file tree
Showing 13 changed files with 227 additions and 82 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ jobs:
strategy:
fail-fast: false
matrix:
suite: [unit, browser, build]
suite: [unit, build]
env:
CI: true
steps:
Expand Down
3 changes: 2 additions & 1 deletion data/redirects/types.ts
Original file line number Diff line number Diff line change
@@ -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 }>;
11 changes: 5 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -163,7 +163,6 @@
"@sentry/browser": "5.6.1"
},
"jest": {
"preset": "jest-puppeteer",
"testEnvironment": "jsdom",
"moduleFileExtensions": [
"ts",
Expand Down
27 changes: 2 additions & 25 deletions script/entry.js
Original file line number Diff line number Diff line change
@@ -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');
Expand All @@ -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);
49 changes: 3 additions & 46 deletions script/prerender/createRedirects.ts
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand Down
53 changes: 53 additions & 0 deletions script/utils/prepareRedirects.ts
Original file line number Diff line number Diff line change
@@ -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;
2 changes: 2 additions & 0 deletions src/app/content/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
];
68 changes: 68 additions & 0 deletions src/app/content/hooks/receivePageNotFoundId.spec.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
21 changes: 21 additions & 0 deletions src/app/content/hooks/receivePageNotFoundId.ts
Original file line number Diff line number Diff line change
@@ -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);
28 changes: 28 additions & 0 deletions src/babel-config.js
Original file line number Diff line number Diff line change
@@ -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',
]
});
};
3 changes: 3 additions & 0 deletions src/redirects.development.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[
{ "from": "/books/physics/pages/1-introduction301", "to": "/books/physics/pages/1-introduction" }
]

0 comments on commit 817a042

Please sign in to comment.