From 0f9844a2faf16073a85483c648443cada503efe2 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Tue, 20 Oct 2020 13:45:49 +0800 Subject: [PATCH 01/20] Implement retryableLazy-based loading --- website/src/views/tetris/TetrisContainer.tsx | 93 +++++++++----------- 1 file changed, 44 insertions(+), 49 deletions(-) diff --git a/website/src/views/tetris/TetrisContainer.tsx b/website/src/views/tetris/TetrisContainer.tsx index b4c1fd6328..1ad39b46f0 100644 --- a/website/src/views/tetris/TetrisContainer.tsx +++ b/website/src/views/tetris/TetrisContainer.tsx @@ -1,59 +1,54 @@ -import * as React from 'react'; -import Loadable, { LoadingComponentProps } from 'react-loadable'; +import React, { lazy, memo, Suspense, useCallback, useState } from 'react'; -import type { EmptyProps } from 'types/utils'; import ApiError from 'views/errors/ApiError'; import LoadingSpinner from 'views/components/LoadingSpinner'; +import ErrorBoundary from 'views/errors/ErrorBoundary'; import type { Props as TetrisGameProps } from './TetrisGame'; -type Props = { - readonly TetrisGame: React.ComponentType; -}; - -type State = { - readonly game: number; -}; - -/** - * Wrapper around TetrisGame which resets the game's internal state and components - * by forcing a remount after each game via the key prop - */ -class TetrisContainer extends React.PureComponent { - state = { - game: 0, - }; - - onResetGame = () => { - this.setState((state) => ({ game: state.game + 1 })); - }; - - render() { - const { TetrisGame } = this.props; - // Force a re-mount of the component, resetting its state by changing its key - return ; - } +// Source: https://github.com/facebook/react/issues/14254#issuecomment-441717770 +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function retryableLazy>( + lazyImport: () => Promise<{ default: T }>, + setComponent: (component: React.LazyExoticComponent) => void, +) { + setComponent( + lazy(() => + lazyImport().catch((err) => { + retryableLazy(lazyImport, setComponent); + throw err; + }), + ), + ); } -/** - * Lazy load the TetrisGame component and pass it down to TetrisContainer - */ -type Export = { TetrisGame: { default: React.ComponentType } }; -export default Loadable.Map({ - loader: { - TetrisGame: () => import(/* webpackChunkName: "tetris" */ './TetrisGame'), +let TetrisGame: React.ComponentType; +retryableLazy( + () => import(/* webpackChunkName: "tetris" */ './TetrisGame'), + (component) => { + TetrisGame = component; }, - loading: (props: LoadingComponentProps) => { - if (props.error) { - return ; - } - - if (props.pastDelay) { - return ; - } +); - return null; - }, - render(loaded, props) { - return ; - }, +/** + * Wrapper around TetrisGame which: + * - Lazy loads the TetrisGame component. + * - Resets the game's internal state and components by forcing a remount after + * each game via the key prop + */ +const TetrisContainer = memo(() => { + const [game, setGame] = useState(0); + const onResetGame = useCallback(() => setGame(game + 1), [game]); + + return ( + } + > + }> + ; + + + ); }); + +export default TetrisContainer; From d077a2245d91ce12c3c4b1b2818d4071a36f1c78 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Wed, 21 Oct 2020 18:50:03 +0800 Subject: [PATCH 02/20] Add React experimental and other deps --- website/package.json | 12 +++++-- website/yarn.lock | 75 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/website/package.json b/website/package.json index b81d1c1879..ab36f918bd 100644 --- a/website/package.json +++ b/website/package.json @@ -56,9 +56,11 @@ "@types/react-loadable": "5.5.3", "@types/react-modal": "3.10.6", "@types/react-redux": "7.1.9", + "@types/react-router-config": "^5.0.1", "@types/react-router-dom": "5.1.6", "@types/react-scrollspy": "3.3.3", "@types/redux-mock-store": "1.0.2", + "@types/use-subscription": "^1.0.0", "@types/webpack-env": "1.15.3", "@typescript-eslint/eslint-plugin": "4.5.0", "@typescript-eslint/parser": "4.5.0", @@ -141,6 +143,7 @@ "core-js": "3.6.5", "date-fns": "2.16.1", "downshift": "5.4.7", + "history": "^5.0.0", "ical-generator": "https://github.com/ZhangYiJiang/ical-generator.git#ed6928fe", "immer": "7.0.9", "json2mq": "0.2.0", @@ -151,9 +154,9 @@ "no-scroll": "2.1.1", "nusmoderator": "3.0.0", "query-string": "5.0.0", - "react": "16.14.0", + "react": "^0.0.0-experimental-4ead6b530", "react-beautiful-dnd": "13.0.0", - "react-dom": "16.14.0", + "react-dom": "^0.0.0-experimental-4ead6b530", "react-feather": "2.0.8", "react-helmet": "6.1.0", "react-hot-loader": "4.13.0", @@ -164,13 +167,16 @@ "react-modal": "3.11.2", "react-qr-svg": "2.2.2", "react-redux": "7.2.1", + "react-router-config": "^5.1.1", "react-router-dom": "5.2.0", "react-scrollspy": "3.4.3", "redux": "4.0.5", "redux-persist": "6.0.0", "redux-thunk": "2.3.0", "reselect": "4.0.0", - "searchkit": "2.4.1-alpha.5" + "resolve-pathname": "^3.0.0", + "searchkit": "2.4.1-alpha.5", + "use-subscription": "^0.0.0-experimental-4ead6b530" }, "browserslist": [ "extends browserslist-config-nusmods" diff --git a/website/yarn.lock b/website/yarn.lock index 119dc5c283..d77114cf13 100644 --- a/website/yarn.lock +++ b/website/yarn.lock @@ -2058,6 +2058,13 @@ dependencies: regenerator-runtime "^0.13.2" +"@babel/runtime@^7.7.6": + version "7.12.1" + resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.12.1.tgz#b4116a6b6711d010b2dad3b7b6e43bf1b9954740" + integrity sha512-J5AIf3vPj3UwXaAzb5j1xM4WAQDX3EMgemF8rjCP3SoW09LfRKAXQKt6CoVYl230P6iWdRcBbnLDDdnqWxZSCA== + dependencies: + regenerator-runtime "^0.13.4" + "@babel/runtime@^7.8.4": version "7.8.7" resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.8.7.tgz#8fefce9802db54881ba59f90bb28719b4996324d" @@ -3218,6 +3225,15 @@ hoist-non-react-statics "^3.3.0" redux "^4.0.0" +"@types/react-router-config@^5.0.1": + version "5.0.1" + resolved "https://registry.yarnpkg.com/@types/react-router-config/-/react-router-config-5.0.1.tgz#54da8418190ee47484dee279975e2b8038fb8b5d" + integrity sha512-D4srFL4XP21GjWWnM7mL8j+Nrrw13pc2TYr57WTHJxU9YTxnrXL7p1iqGtwecgwhyeXJSm4WrGwq0SOgSALVbA== + dependencies: + "@types/history" "*" + "@types/react" "*" + "@types/react-router" "*" + "@types/react-router-dom@5.1.6": version "5.1.6" resolved "https://registry.yarnpkg.com/@types/react-router-dom/-/react-router-dom-5.1.6.tgz#07b14e7ab1893a837c8565634960dc398564b1fb" @@ -3297,6 +3313,11 @@ resolved "https://registry.yarnpkg.com/@types/unist/-/unist-2.0.3.tgz#9c088679876f374eb5983f150d4787aa6fb32d7e" integrity sha512-FvUupuM3rlRsRtCN+fDudtmytGO6iHJuuRKS1Ss0pG5z8oX0diNEw94UEL7hgDbpN94rgaK5R7sWm6RrSkZuAQ== +"@types/use-subscription@^1.0.0": + version "1.0.0" + resolved "https://registry.yarnpkg.com/@types/use-subscription/-/use-subscription-1.0.0.tgz#d146f8d834f70f50d48bd8246a481d096f11db19" + integrity sha512-0WWZ5GUDKMXUY/1zy4Ur5/zsC0s/B+JjXfHdkvx6JgDNZzZV5eW+KKhDqsTGyqX56uh99gwGwbsKbVwkcVIKQA== + "@types/webpack-env@1.15.3": version "1.15.3" resolved "https://registry.yarnpkg.com/@types/webpack-env/-/webpack-env-1.15.3.tgz#fb602cd4c2f0b7c0fb857e922075fdf677d25d84" @@ -8127,6 +8148,13 @@ history@^4.9.0: tiny-warning "^1.0.0" value-equal "^0.4.0" +history@^5.0.0: + version "5.0.0" + resolved "https://registry.yarnpkg.com/history/-/history-5.0.0.tgz#0cabbb6c4bbf835addb874f8259f6d25101efd08" + integrity sha512-3NyRMKIiFSJmIPdq7FxkNMJkQ7ZEtVblOQ38VtKaA0zZMW1Eo6Q6W8oDKEflr1kNNTItSnk4JMCO1deeSgbLLg== + dependencies: + "@babel/runtime" "^7.7.6" + hmac-drbg@^1.0.0: version "1.0.1" resolved "https://registry.yarnpkg.com/hmac-drbg/-/hmac-drbg-1.0.1.tgz#d2745701025a6c775a6c545793ed502fc0c649a1" @@ -12863,15 +12891,14 @@ react-dev-utils@10.2.1: strip-ansi "6.0.0" text-table "0.2.0" -react-dom@16.14.0: - version "16.14.0" - resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.14.0.tgz#7ad838ec29a777fb3c75c3a190f661cf92ab8b89" - integrity sha512-1gCeQXDLoIqMgqD3IO2Ah9bnf0w9kzhwN5q4FGnHZ67hBm9yePzB5JJAIQCc8x3pFnNlwFq4RidZggNAAkzWWw== +react-dom@^0.0.0-experimental-4ead6b530: + version "0.0.0-experimental-4ead6b530" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-0.0.0-experimental-4ead6b530.tgz#6ca831f5aa7ab86f7299b9a2f7b81dffabfa4eb4" + integrity sha512-a03ptS8lhhEENNgne6zQMXQWX/Z6WMEBGJQY0laOC0NgJywidePYpgkiE72fUAaj/r7t9a6XsdVyqx4UsEZijg== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" - prop-types "^15.6.2" - scheduler "^0.19.1" + scheduler "0.0.0-experimental-4ead6b530" react-error-overlay@^6.0.7: version "6.0.7" @@ -13028,6 +13055,13 @@ react-redux@^7.1.1: prop-types "^15.7.2" react-is "^16.9.0" +react-router-config@^5.1.1: + version "5.1.1" + resolved "https://registry.yarnpkg.com/react-router-config/-/react-router-config-5.1.1.tgz#0f4263d1a80c6b2dc7b9c1902c9526478194a988" + integrity sha512-DuanZjaD8mQp1ppHjgnnUnyOlqYXZVjnov/JzFhjLEwd3Z4dYjMSnqrEzzGThH47vpCOqPPwJM2FtthLeJ8Pbg== + dependencies: + "@babel/runtime" "^7.1.2" + react-router-dom@5.2.0: version "5.2.0" resolved "https://registry.yarnpkg.com/react-router-dom/-/react-router-dom-5.2.0.tgz#9e65a4d0c45e13289e66c7b17c7e175d0ea15662" @@ -13091,14 +13125,13 @@ react-test-renderer@^16.0.0-0: react-is "^16.8.3" scheduler "^0.13.3" -react@16.14.0: - version "16.14.0" - resolved "https://registry.yarnpkg.com/react/-/react-16.14.0.tgz#94d776ddd0aaa37da3eda8fc5b6b18a4c9a3114d" - integrity sha512-0X2CImDkJGApiAlcf0ODKIneSwBPhqJawOa5wCtKbu7ZECrmS26NvtSILynQ66cgkT/RJ4LidJOc3bUESwmU8g== +react@^0.0.0-experimental-4ead6b530: + version "0.0.0-experimental-4ead6b530" + resolved "https://registry.yarnpkg.com/react/-/react-0.0.0-experimental-4ead6b530.tgz#88cdae012012a758dd039a63104758c6351115df" + integrity sha512-tpbYm6FEuC1L6tCVXIKYAhgGAkS8DShzKpmXosowZvLqeByeLQQe77Ef6bi5HdEkFm2v0lZffLWckSM8R4TToA== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" - prop-types "^15.6.2" read-pkg-up@^1.0.1: version "1.0.1" @@ -13653,6 +13686,11 @@ resolve-pathname@^2.2.0: resolved "https://registry.yarnpkg.com/resolve-pathname/-/resolve-pathname-2.2.0.tgz#7e9ae21ed815fd63ab189adeee64dc831eefa879" integrity sha512-bAFz9ld18RzJfddgrO2e/0S2O81710++chRMUxHjXOYKF6jTAMrUNZrEZ1PvV0zlhfjidm08iRPdTLPno1FuRg== +resolve-pathname@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/resolve-pathname/-/resolve-pathname-3.0.0.tgz#99d02224d3cf263689becbb393bc560313025dcd" + integrity sha512-C7rARubxI8bXFNB/hqcp/4iUeIXJhJZvFPFPiSPRnhU5UPxzMFIl+2E6yY6c4k9giDJAhtV+enfA+G89N6Csng== + resolve-url@^0.2.1: version "0.2.1" resolved "https://registry.yarnpkg.com/resolve-url/-/resolve-url-0.2.1.tgz#2c637fe77c893afd2a663fe21aa9080068e2052a" @@ -13850,6 +13888,14 @@ saxes@^5.0.0: dependencies: xmlchars "^2.2.0" +scheduler@0.0.0-experimental-4ead6b530: + version "0.0.0-experimental-4ead6b530" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.0.0-experimental-4ead6b530.tgz#0dca3287308d34caed0651941f1ce7c9d64a0824" + integrity sha512-AzUR6EiDuY32oAnfELgVFPasfovJw4+NtRy7RIam0IUOSgNZKcazqcHzsoW1zDw3AzIBlD1VlRvl5SPJRSlTPg== + dependencies: + loose-envify "^1.1.0" + object-assign "^4.1.1" + scheduler@^0.13.3: version "0.13.3" resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.13.3.tgz#bed3c5850f62ea9c716a4d781f9daeb9b2a58896" @@ -15770,6 +15816,13 @@ use-memo-one@^1.1.1: resolved "https://registry.yarnpkg.com/use-memo-one/-/use-memo-one-1.1.1.tgz#39e6f08fe27e422a7d7b234b5f9056af313bd22c" integrity sha512-oFfsyun+bP7RX8X2AskHNTxu+R3QdE/RC5IefMbqptmACAA/gfol1KDD5KRzPsGMa62sWxGZw+Ui43u6x4ddoQ== +use-subscription@^0.0.0-experimental-4ead6b530: + version "0.0.0-experimental-4ead6b530" + resolved "https://registry.yarnpkg.com/use-subscription/-/use-subscription-0.0.0-experimental-4ead6b530.tgz#94c611419a6c945b17f9926e94beaf9af148f81e" + integrity sha512-vdCAmupdX9iWfyNsBE6nLYH2XDu84RAGSRVzKJaVYaLOFd1LYHSBiZn7l5JPwxIrmP+Asbie5plTi1k+KYQSuQ== + dependencies: + object-assign "^4.1.1" + use@^3.1.0: version "3.1.1" resolved "https://registry.yarnpkg.com/use/-/use-3.1.1.tgz#d50c8cac79a19fbc20f2911f56eb973f4e10070f" From a76cab97b2e24f30cfe837ba456a501de3c09b47 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Wed, 21 Oct 2020 18:52:44 +0800 Subject: [PATCH 03/20] Big commit of many WIP things --- website/src/bootstrapping/configure-store.ts | 28 +- website/src/bootstrapping/matomo.ts | 3 +- website/src/entry/App.tsx | 35 ++- website/src/entry/main.tsx | 7 +- website/src/utils/JSResource.ts | 132 +++++++++ website/src/views/AppShell.tsx | 261 +++++++++--------- .../views/components/KeyboardShortcuts.tsx | 3 +- .../src/views/components/LinkModuleCodes.tsx | 2 +- website/src/views/components/ScrollToTop.tsx | 55 ++-- .../components/map/venueLocationResource.ts | 6 + .../components/map/withVenueLocations.tsx | 1 + .../module-info/LessonTimetable.tsx | 3 +- .../notfications/HacktoberfestBanner.tsx | 2 +- .../notfications/ModRegNotification.tsx | 3 +- .../ContributeContainer.tsx | 2 +- .../src/views/errors/ModuleNotFoundPage.tsx | 2 +- website/src/views/errors/NotFoundPage.tsx | 2 +- website/src/views/layout/Footer.tsx | 2 +- .../views/layout/GlobalSearchContainer.tsx | 3 +- website/src/views/layout/Navtabs.tsx | 119 ++++---- .../views/modules/ModuleArchiveContainer.tsx | 3 +- .../src/views/modules/ModuleFinderItem.tsx | 2 +- .../src/views/modules/ModulePageContainer.tsx | 3 +- .../src/views/modules/ModulePageContent.tsx | 2 +- website/src/views/planner/PlannerModule.tsx | 2 +- website/src/views/routes/Link.tsx | 68 +++++ website/src/views/routes/NavLink.tsx | 65 +++++ website/src/views/routes/RouteRenderer.tsx | 139 ++++++++++ website/src/views/routes/RoutingContext.ts | 47 ++++ website/src/views/routes/createRouter.ts | 121 ++++++++ website/src/views/routes/createRoutes.ts | 77 ++++++ website/src/views/routes/hooks.ts | 52 ++++ website/src/views/routes/locationUtils.ts | 111 ++++++++ website/src/views/static/FaqContainer.tsx | 2 +- website/src/views/timetable/ExamWeek.tsx | 2 +- website/src/views/timetable/ExportMenu.tsx | 2 +- .../views/timetable/TimetableContainer.tsx | 11 +- .../views/timetable/TimetableModulesTable.tsx | 2 +- website/src/views/today/BeforeLessonCard.tsx | 2 +- .../views/today/{EventMap => }/EventMap.scss | 0 .../views/today/{EventMap => }/EventMap.tsx | 22 +- website/src/views/today/EventMap/index.tsx | 9 - .../{EventMapInline => }/EventMapInline.scss | 0 .../{EventMapInline => }/EventMapInline.tsx | 23 +- .../src/views/today/EventMapInline/index.tsx | 8 - .../{TodayContainer => }/TodayContainer.scss | 0 .../TodayContainer.test.tsx | 4 +- .../{TodayContainer => }/TodayContainer.tsx | 10 +- .../src/views/today/TodayContainer/index.tsx | 31 --- .../__mocks__/forecasts.json | 0 website/src/views/venues/VenueDetails.tsx | 6 +- website/src/views/venues/VenueList.tsx | 2 +- website/src/views/venues/VenuesContainer.tsx | 3 +- website/webpack/webpack.config.dev.js | 12 +- 54 files changed, 1145 insertions(+), 369 deletions(-) create mode 100644 website/src/utils/JSResource.ts create mode 100644 website/src/views/components/map/venueLocationResource.ts create mode 100644 website/src/views/routes/Link.tsx create mode 100644 website/src/views/routes/NavLink.tsx create mode 100644 website/src/views/routes/RouteRenderer.tsx create mode 100644 website/src/views/routes/RoutingContext.ts create mode 100644 website/src/views/routes/createRouter.ts create mode 100644 website/src/views/routes/createRoutes.ts create mode 100644 website/src/views/routes/hooks.ts create mode 100644 website/src/views/routes/locationUtils.ts rename website/src/views/today/{EventMap => }/EventMap.scss (100%) rename website/src/views/today/{EventMap => }/EventMap.tsx (62%) delete mode 100644 website/src/views/today/EventMap/index.tsx rename website/src/views/today/{EventMapInline => }/EventMapInline.scss (100%) rename website/src/views/today/{EventMapInline => }/EventMapInline.tsx (69%) delete mode 100644 website/src/views/today/EventMapInline/index.tsx rename website/src/views/today/{TodayContainer => }/TodayContainer.scss (100%) rename website/src/views/today/{TodayContainer => }/TodayContainer.test.tsx (98%) rename website/src/views/today/{TodayContainer => }/TodayContainer.tsx (98%) delete mode 100644 website/src/views/today/TodayContainer/index.tsx rename website/src/views/today/{TodayContainer => }/__mocks__/forecasts.json (100%) diff --git a/website/src/bootstrapping/configure-store.ts b/website/src/bootstrapping/configure-store.ts index 2a49e1d0c6..90072b6b8a 100644 --- a/website/src/bootstrapping/configure-store.ts +++ b/website/src/bootstrapping/configure-store.ts @@ -27,20 +27,20 @@ export default function configureStore(defaultState?: State) { const middlewares = [ravenMiddleware, thunk, requestsMiddleware]; - if (process.env.NODE_ENV === 'development') { - // eslint-disable-next-line @typescript-eslint/no-var-requires, global-require, import/no-extraneous-dependencies - const { createLogger } = require('redux-logger'); - const logger = createLogger({ - level: 'info', - collapsed: true, - duration: true, - diff: true, - // Avoid diffing actions that insert a lot of stuff into the state to prevent console from lagging - diffPredicate: (getState: GetState, action: Actions) => - !action.type.startsWith('FETCH_MODULE_LIST') && !action.type.startsWith('persist/'), - }); - middlewares.push(logger); - } + // if (process.env.NODE_ENV === 'development') { + // // eslint-disable-next-line @typescript-eslint/no-var-requires, global-require, import/no-extraneous-dependencies + // const { createLogger } = require('redux-logger'); + // const logger = createLogger({ + // level: 'info', + // collapsed: true, + // duration: true, + // diff: true, + // // Avoid diffing actions that insert a lot of stuff into the state to prevent console from lagging + // diffPredicate: (getState: GetState, action: Actions) => + // !action.type.startsWith('FETCH_MODULE_LIST') && !action.type.startsWith('persist/'), + // }); + // middlewares.push(logger); + // } const storeEnhancer = applyMiddleware(...middlewares); diff --git a/website/src/bootstrapping/matomo.ts b/website/src/bootstrapping/matomo.ts index 4a97ade899..53d767a5ea 100644 --- a/website/src/bootstrapping/matomo.ts +++ b/website/src/bootstrapping/matomo.ts @@ -1,4 +1,3 @@ -// eslint-disable-next-line import/no-extraneous-dependencies import { History } from 'history'; import { each } from 'lodash'; @@ -68,7 +67,7 @@ export function setCustomDimensions(dimensions: { [id: number]: string }) { } export function trackPageView(history: History) { - history.listen((location, action) => { + return history.listen(({ action }) => { if (action === 'PUSH') { // Wait a bit for the page title to update setTimeout(() => { diff --git a/website/src/entry/App.tsx b/website/src/entry/App.tsx index dbb6bc3475..636446bf8d 100644 --- a/website/src/entry/App.tsx +++ b/website/src/entry/App.tsx @@ -2,42 +2,55 @@ import { Store } from 'redux'; import { State } from 'types/state'; import { Persistor } from 'storage/persistReducer'; -import React from 'react'; +import React, { useCallback, useMemo } from 'react'; import { hot } from 'react-hot-loader/root'; -import { BrowserRouter as Router } from 'react-router-dom'; +// import { BrowserRouter as Router } from 'react-router-dom'; import { Provider } from 'react-redux'; import { PersistGate } from 'redux-persist/integration/react'; -import AppShell from 'views/AppShell'; -import Routes from 'views/routes/Routes'; +// import AppShell from 'views/AppShell'; +// import Routes from 'views/routes/Routes'; +import createRoutes from 'views/routes/createRoutes'; import { DIMENSIONS, setCustomDimensions } from 'bootstrapping/matomo'; import ErrorBoundary from 'views/errors/ErrorBoundary'; import ErrorPage from 'views/errors/ErrorPage'; +import createRouter from 'views/routes/createRouter'; +import RoutingContext from 'views/routes/RoutingContext'; +import RouterRenderer from 'views/routes/RouteRenderer'; type Props = { store: Store; persistor: Persistor; }; +// const router = createRouter(createRoutes(store.dispatch)); + const App: React.FC = ({ store, persistor }) => { - const onBeforeLift = () => { + // Uses the custom router setup to define a router instanace that we can pass through context + const router = useMemo(() => createRouter(createRoutes(store.dispatch)), [store.dispatch]); + + const onBeforeLift = useCallback(() => { const { theme, settings } = store.getState(); setCustomDimensions({ [DIMENSIONS.theme]: theme.id, [DIMENSIONS.beta]: String(!!settings.beta), }); - }; + }, [store]); + // + // + // + // + // return ( }> - - - - - + + {/* Render the active route */} + + diff --git a/website/src/entry/main.tsx b/website/src/entry/main.tsx index eb146b829c..43410d7b50 100644 --- a/website/src/entry/main.tsx +++ b/website/src/entry/main.tsx @@ -7,7 +7,9 @@ import 'bootstrapping/sentry'; import 'core-js/es/promise/finally'; import React from 'react'; -import ReactDOM from 'react-dom'; +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore +import { unstable_createRoot as createRoot } from 'react-dom'; import ReactModal from 'react-modal'; import configureStore from 'bootstrapping/configure-store'; @@ -26,7 +28,8 @@ subscribeOnlineEvents(store); // Initialize ReactModal ReactModal.setAppElement('#app'); -ReactDOM.render(, document.getElementById('app')); +// ReactDOM.render(, document.getElementById('app')); +createRoot(document.getElementById('app')).render(); if ( ('serviceWorker' in navigator && diff --git a/website/src/utils/JSResource.ts b/website/src/utils/JSResource.ts new file mode 100644 index 0000000000..f5cc9d5130 --- /dev/null +++ b/website/src/utils/JSResource.ts @@ -0,0 +1,132 @@ +type Loadable = Result | { default: Result }; +type Loader = () => Promise>; + +/** + * A cache of resources to avoid loading the same module twice. This is important + * because Webpack dynamic imports only expose an asynchronous API for loading + * modules, so to be able to access already-loaded modules synchronously we + * must have stored the previous result somewhere. + */ +const resourceMap = new Map(); + +/** + * A generic resource: given some method to asynchronously load a value - the loader() + * argument - it allows accessing the state of the resource. + */ +class Resource { + private error: Error | null = null; + + private promise: Promise | null = null; + + private result: Result | null = null; + + private loader: Loader; + + constructor(loader: Loader) { + this.loader = loader; + } + + // TODO: Check if we should do this or implement a way to replace the resource + // instance entirely. I suspect this current approach will not trigger + // renders. + reset() { + this.error = null; + this.promise = null; + this.result = null; + } + + /** + * Loads the resource if necessary. + */ + preload() { + let { promise } = this; + if (promise == null) { + promise = this.loader() + .then((result) => { + let unmoduledResult: Result; + if (result.default) { + unmoduledResult = result.default; + } else { + unmoduledResult = result; + } + this.result = unmoduledResult; + return unmoduledResult; + }) + .catch((error) => { + this.error = error; + throw error; + }); + this.promise = promise; + } + return promise; + } + + preloadOrReloadIfError() { + if (this.error !== null) { + this.reset(); + } + this.preload(); + } + + /** + * Returns the result, if available. This can be useful to check if the value + * is resolved yet. + */ + get() { + if (this.result != null) { + return this.result; + } + return undefined; + } + + /** + * This is the key method for integrating with React Suspense. Read will: + * - "Suspend" if the resource is still pending (currently implemented as + * throwing a Promise, though this is subject to change in future + * versions of React) + * - Throw an error if the resource failed to load. + * - Return the data of the resource if available. + */ + read() { + if (this.result !== null) { + return this.result; + } + if (this.error !== null) { + throw this.error; + } + throw this.promise; + } + + fetch() { + this.preload(); + return this.read(); + } +} + +export type JSResource = Resource; + +/** + * A helper method to create a resource, intended for dynamically loading code. + * + * Example: + * ``` + * // Before rendering, ie in an event handler: + * const resource = JSResource('Foo', () => import('./Foo.js)); + * resource.load(); + * + * // in a React component: + * const Foo = resource.read(); + * return ; + * ``` + * + * @param {*} moduleId A globally unique identifier for the resource used for caching + * @param {*} loader A method to load the resource's data if necessary + */ +export function JSResource(moduleId: unknown, loader: Loader): JSResource { + let resource = resourceMap.get(moduleId); + if (resource == null) { + resource = new Resource(loader); + resourceMap.set(moduleId, resource); + } + return resource; +} diff --git a/website/src/views/AppShell.tsx b/website/src/views/AppShell.tsx index 4e67b5d66e..5d1990c5e4 100644 --- a/website/src/views/AppShell.tsx +++ b/website/src/views/AppShell.tsx @@ -1,20 +1,21 @@ -import React from 'react'; +import React, { Suspense, useCallback, useContext, useEffect } from 'react'; import { SemTimetableConfig, TimetableConfig } from 'types/timetables'; import { ModuleList, NotificationOptions } from 'types/reducers'; import { Semester } from 'types/modules'; import { DARK_MODE, Mode } from 'types/settings'; import { Helmet } from 'react-helmet'; -import { NavLink, RouteComponentProps, withRouter } from 'react-router-dom'; import { connect } from 'react-redux'; import classnames from 'classnames'; import { each } from 'lodash'; import weekText from 'utils/weekText'; import { captureException } from 'utils/error'; -import { openNotification } from 'actions/app'; -import { fetchModuleList } from 'actions/moduleBank'; -import { fetchTimetableModules, setTimetable, validateTimetable } from 'actions/timetables'; +import { openNotification as openNotificationAction } from 'actions/app'; +import { + fetchTimetableModules as fetchTimetableModulesAction, + validateTimetable as validateTimetableAction, +} from 'actions/timetables'; import Footer from 'views/layout/Footer'; import Navtabs from 'views/layout/Navtabs'; import GlobalSearchContainer from 'views/layout/GlobalSearchContainer'; @@ -26,164 +27,177 @@ import { trackPageView } from 'bootstrapping/matomo'; import { isIOS } from 'bootstrapping/browser'; import Logo from 'img/nusmods-logo.svg'; import { State as StoreState } from 'types/state'; +import type { JSResource } from 'utils/JSResource'; import LoadingSpinner from './components/LoadingSpinner'; import FeedbackModal from './components/FeedbackModal'; import KeyboardShortcuts from './components/KeyboardShortcuts'; +import NavLink from './routes/NavLink'; +import RoutingContext from './routes/RoutingContext'; +import { useLocation } from './routes/hooks'; import styles from './AppShell.scss'; -type Props = RouteComponentProps & { +type Props = { children: React.ReactNode; + // From router + prepared: { + moduleList: JSResource; + }; + // From Redux state moduleList: ModuleList; timetables: TimetableConfig; theme: string; mode: Mode; - activeSemester: Semester; // From Redux actions - fetchModuleList: () => Promise; // Typed as unknown because we don't actually need the output - fetchTimetableModules: (semTimetableConfig: SemTimetableConfig[]) => Promise; - setTimetable: (semester: Semester, semTimetableConfig: SemTimetableConfig) => void; + fetchTimetableModulesProp: (semTimetableConfig: SemTimetableConfig[]) => Promise; validateTimetable: (semester: Semester) => void; openNotification: (str: string, notificationOptions?: NotificationOptions) => void; }; -type State = { - moduleListError?: Error; -}; +export const AppShellComponent: React.FC = ({ + children, -export class AppShellComponent extends React.Component { - state: State = {}; + prepared, - componentDidMount() { - const { timetables } = this.props; + moduleList, + timetables, + theme, + mode, - // Retrieve module list - const moduleList = this.fetchModuleList(); + fetchTimetableModulesProp, + validateTimetable, + openNotification, +}) => { + const location = useLocation(); - // Fetch the module data of the existing modules in the timetable and ensure all - // lessons are filled - each(timetables, (timetable, semesterString) => { - const semester = Number(semesterString); - moduleList.then(() => { - // Wait for module list to be fetched before trying to fetch timetable modules - // TODO: There may be a more optimal way to do this - this.fetchTimetableModules(timetable, semester); - }); - }); - - // Enable Matomo analytics - trackPageView(this.props.history); - } - - fetchModuleList = () => - // TODO: This always re-fetch the entire modules list. Consider a better strategy for this - this.props.fetchModuleList().catch((error) => { - captureException(error); - this.setState({ moduleListError: error }); - }); - - fetchTimetableModules = (timetable: SemTimetableConfig, semester: Semester) => { - this.props - .fetchTimetableModules([timetable]) - .then(() => this.props.validateTimetable(semester)) - .catch((error) => { - captureException(error); - this.props.openNotification('Data for some modules failed to load', { - action: { - text: 'Retry', - handler: () => this.fetchTimetableModules(timetable, semester), - }, + // Enable Matomo analytics + const router = useContext(RoutingContext); + useEffect(() => { + if (router) { + // Unsubscribe when router changes or on unmount + return trackPageView(router.history); + } + return undefined; + }, [router]); + + const fetchTimetableModules = useCallback( + (timetable: SemTimetableConfig, semester: Semester) => { + fetchTimetableModulesProp([timetable]) + .then(() => validateTimetable(semester)) + .catch((error) => { + captureException(error); + openNotification('Data for some modules failed to load', { + action: { + text: 'Retry', + handler: () => fetchTimetableModules(timetable, semester), + }, + }); + }); + }, + [fetchTimetableModulesProp, openNotification, validateTimetable], + ); + + useEffect( + () => { + // Fetch the module data of the existing modules in the timetable and ensure all + // lessons are filled + each(timetables, (timetable, semesterString) => { + const semester = Number(semesterString); + prepared.moduleList.preload().then(() => { + // Wait for module list to be fetched before trying to fetch timetable modules + // TODO: There may be a more optimal way to do this + fetchTimetableModules(timetable, semester); }); }); - }; - - render() { - const isModuleListReady = this.props.moduleList.length; - const isDarkMode = this.props.mode === DARK_MODE; - - if (!isModuleListReady && this.state.moduleListError) { - return ; - } - - return ( -
- - - - - - -
- - -
- {isModuleListReady ? ( - }> - {this.props.children} - - ) : ( - - )} -
+ }, + // Only run this once, on component mount. Don't care if props change after mount. + // eslint-disable-next-line react-hooks/exhaustive-deps + [], + ); + + const isModuleListReady = moduleList.length; + const isDarkMode = mode === DARK_MODE; + + return ( +
+ + + + + + +
+ + +
+ {/* FIXME: Create error page that switches between network errors + and other (unexpected) errors. */} + {isModuleListReady ? ( + } + > + }>{children} + + ) : ( + + )} +
+
- - - + + + - - - + + + - -
- + +
+ - - - -
- ); - } -} + + + +
+ ); +}; const mapStateToProps = (state: StoreState) => ({ moduleList: state.moduleBank.moduleList, timetables: state.timetables.lessons, theme: state.theme.id, mode: state.settings.mode, - activeSemester: state.app.activeSemester, }); const connectedAppShell = connect( mapStateToProps, { - fetchModuleList, - fetchTimetableModules, - setTimetable, - validateTimetable, - openNotification, + fetchTimetableModulesProp: fetchTimetableModulesAction, + validateTimetable: validateTimetableAction, + openNotification: openNotificationAction, }, // TODO: Patch types for Redux for request-middleware // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -193,4 +207,5 @@ const connectedAppShell = connect( // connect implements shouldComponentUpdate based purely on props. If it // is removed, connect not detect prop changes when route is changed and // thus the pages are not re-rendered -export default withRouter(connectedAppShell); +// export default withRouter(connectedAppShell); +export default connectedAppShell; diff --git a/website/src/views/components/KeyboardShortcuts.tsx b/website/src/views/components/KeyboardShortcuts.tsx index 34a7128791..75d04454fa 100644 --- a/website/src/views/components/KeyboardShortcuts.tsx +++ b/website/src/views/components/KeyboardShortcuts.tsx @@ -196,4 +196,5 @@ const KeyboardShortcutsConnected = connect((state: StoreState) => ({ theme: state.theme.id, }))(KeyboardShortcutsComponent); -export default withRouter(KeyboardShortcutsConnected); +// export default withRouter(KeyboardShortcutsConnected); +export default KeyboardShortcutsConnected; diff --git a/website/src/views/components/LinkModuleCodes.tsx b/website/src/views/components/LinkModuleCodes.tsx index 828f30b74c..53cb276acb 100644 --- a/website/src/views/components/LinkModuleCodes.tsx +++ b/website/src/views/components/LinkModuleCodes.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { connect } from 'react-redux'; -import { Link } from 'react-router-dom'; +import Link from 'views/routes/Link'; import { ModuleCode, ModuleCondensed } from 'types/modules'; import { State } from 'types/state'; diff --git a/website/src/views/components/ScrollToTop.tsx b/website/src/views/components/ScrollToTop.tsx index 104ed4ef61..517edf5ce6 100644 --- a/website/src/views/components/ScrollToTop.tsx +++ b/website/src/views/components/ScrollToTop.tsx @@ -1,50 +1,29 @@ -import * as React from 'react'; -import { withRouter, RouteComponentProps } from 'react-router-dom'; +import React, { useEffect } from 'react'; import { scrollToHash } from 'utils/react'; -export type Props = RouteComponentProps & { - onComponentDidMount?: boolean; - onPathChange?: boolean; - scrollToHash?: boolean; -}; - function scrollToTop() { window.scrollTo(0, 0); } -export class ScrollToTopComponent extends React.Component { - static defaultProps = { - onComponentDidMount: false, - onPathChange: false, - scrollToHash: true, - }; +export type Props = { + onComponentDidMount?: boolean; + shouldScrollToHash?: boolean; +}; - componentDidMount() { - if (this.props.onComponentDidMount && !window.location.hash) { +const ScrollToTop: React.FC = ({ + onComponentDidMount = false, + shouldScrollToHash = true, +}) => { + useEffect(() => { + // FIXME: This effect runs on mount AND when any of those props change. + if (onComponentDidMount && !window.location.hash) { scrollToTop(); - } else if (this.props.scrollToHash) { + } else if (shouldScrollToHash) { scrollToHash(); } - } - - componentDidUpdate(prevProps: Props) { - const { - onPathChange, - location: { pathname, hash }, - } = this.props; + }, [onComponentDidMount, shouldScrollToHash]); - if ( - onPathChange && - pathname !== prevProps.location.pathname && - hash === prevProps.location.hash - ) { - scrollToTop(); - } - } - - render() { - return null; - } -} + return null; +}; -export default withRouter(ScrollToTopComponent); +export default ScrollToTop; diff --git a/website/src/views/components/map/venueLocationResource.ts b/website/src/views/components/map/venueLocationResource.ts new file mode 100644 index 0000000000..08e9a2db38 --- /dev/null +++ b/website/src/views/components/map/venueLocationResource.ts @@ -0,0 +1,6 @@ +import { getVenueLocations } from 'apis/github'; +import { JSResource } from 'utils/JSResource'; + +// FIXME: Disable getVenueLocations' built in promise memoization as it's +// already memoized in Resource. Breaks resource reloads. +export default JSResource('venueLocationResource', () => getVenueLocations()); diff --git a/website/src/views/components/map/withVenueLocations.tsx b/website/src/views/components/map/withVenueLocations.tsx index c716bcc39f..e39fa99c84 100644 --- a/website/src/views/components/map/withVenueLocations.tsx +++ b/website/src/views/components/map/withVenueLocations.tsx @@ -30,6 +30,7 @@ const defaultLoadingComponent = () => ; * Defaults to * @param Loading Component shown while the data is loading * Defaults to + * @deprecated Use `venueLocationResource` instead. */ export default function withVenueLocations( getComponent: () => Promise>, diff --git a/website/src/views/components/module-info/LessonTimetable.tsx b/website/src/views/components/module-info/LessonTimetable.tsx index 024c98ef17..d66ab3eb24 100644 --- a/website/src/views/components/module-info/LessonTimetable.tsx +++ b/website/src/views/components/module-info/LessonTimetable.tsx @@ -73,4 +73,5 @@ export class LessonTimetableComponent extends React.PureComponent } } -export default withRouter(LessonTimetableComponent); +// export default withRouter(LessonTimetableComponent); +export default LessonTimetableComponent; diff --git a/website/src/views/components/notfications/HacktoberfestBanner.tsx b/website/src/views/components/notfications/HacktoberfestBanner.tsx index 2a8cb472ce..1c18144b78 100644 --- a/website/src/views/components/notfications/HacktoberfestBanner.tsx +++ b/website/src/views/components/notfications/HacktoberfestBanner.tsx @@ -1,6 +1,6 @@ import React from 'react'; import classnames from 'classnames'; -import { Link } from 'react-router-dom'; +import Link from 'views/routes/Link'; import type { EmptyProps } from 'types/utils'; import storage from 'storage'; diff --git a/website/src/views/components/notfications/ModRegNotification.tsx b/website/src/views/components/notfications/ModRegNotification.tsx index a64e469e82..4e52586fb9 100644 --- a/website/src/views/components/notfications/ModRegNotification.tsx +++ b/website/src/views/components/notfications/ModRegNotification.tsx @@ -95,5 +95,6 @@ const withStoreModRegNotification = connect(mapStateToProps, { openNotification, })(React.memo(ModRegNotificationComponent)); -const ModRegNotification = withRouter(withStoreModRegNotification); +// const ModRegNotification = withRouter(withStoreModRegNotification); +const ModRegNotification = withStoreModRegNotification; export default ModRegNotification; diff --git a/website/src/views/contribute/ContributeContainer/ContributeContainer.tsx b/website/src/views/contribute/ContributeContainer/ContributeContainer.tsx index 92d11a6ee9..53341bbd9a 100644 --- a/website/src/views/contribute/ContributeContainer/ContributeContainer.tsx +++ b/website/src/views/contribute/ContributeContainer/ContributeContainer.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import { connect } from 'react-redux'; import classnames from 'classnames'; -import { Link } from 'react-router-dom'; +import Link from 'views/routes/Link'; import { flatten, map, mapValues, values } from 'lodash'; import { ModuleCondensed } from 'types/modules'; diff --git a/website/src/views/errors/ModuleNotFoundPage.tsx b/website/src/views/errors/ModuleNotFoundPage.tsx index 719c5fc5d9..563508e413 100644 --- a/website/src/views/errors/ModuleNotFoundPage.tsx +++ b/website/src/views/errors/ModuleNotFoundPage.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { Link } from 'react-router-dom'; +import Link from 'views/routes/Link'; import { connect } from 'react-redux'; import classnames from 'classnames'; import * as Sentry from '@sentry/browser'; diff --git a/website/src/views/errors/NotFoundPage.tsx b/website/src/views/errors/NotFoundPage.tsx index b886bb7171..9dc28afe8f 100644 --- a/website/src/views/errors/NotFoundPage.tsx +++ b/website/src/views/errors/NotFoundPage.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { Link } from 'react-router-dom'; +import Link from 'views/routes/Link'; import * as Sentry from '@sentry/browser'; import RandomKawaii from 'views/components/RandomKawaii'; diff --git a/website/src/views/layout/Footer.tsx b/website/src/views/layout/Footer.tsx index 1badba93d5..86e43cde9c 100644 --- a/website/src/views/layout/Footer.tsx +++ b/website/src/views/layout/Footer.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { connect } from 'react-redux'; -import { Link } from 'react-router-dom'; +import Link from 'views/routes/Link'; import ExternalLink from 'views/components/ExternalLink'; import config from 'config'; diff --git a/website/src/views/layout/GlobalSearchContainer.tsx b/website/src/views/layout/GlobalSearchContainer.tsx index bb3cfd8e07..9c2060e609 100644 --- a/website/src/views/layout/GlobalSearchContainer.tsx +++ b/website/src/views/layout/GlobalSearchContainer.tsx @@ -106,7 +106,8 @@ export class SearchContainerComponent extends React.Component { } } -const routedSearchContainer = withRouter(SearchContainerComponent); +// const routedSearchContainer = withRouter(SearchContainerComponent); +const routedSearchContainer = SearchContainerComponent; const connectedSearchContainer = connect( (state: State) => ({ moduleList: state.moduleBank.moduleList, diff --git a/website/src/views/layout/Navtabs.tsx b/website/src/views/layout/Navtabs.tsx index bde2fb499a..b4a5685017 100644 --- a/website/src/views/layout/Navtabs.tsx +++ b/website/src/views/layout/Navtabs.tsx @@ -1,92 +1,89 @@ -import * as React from 'react'; +import React, { memo } from 'react'; import { connect } from 'react-redux'; -import { NavLink, RouteComponentProps, withRouter } from 'react-router-dom'; import classnames from 'classnames'; import { BookOpen, Calendar, Clock, Heart, Map, Settings, Star, Trello } from 'react-feather'; import { Semester } from 'types/modules'; import ExternalLink from 'views/components/ExternalLink'; import { timetablePage } from 'views/routes/paths'; -import { preload as preloadToday } from 'views/today/TodayContainer'; import { preload as preloadVenues } from 'views/venues/VenuesContainer'; import { preload as preloadContribute } from 'views/contribute/ContributeContainer'; +import NavLink from 'views/routes/NavLink'; import { State } from 'types/state'; import styles from './Navtabs.scss'; export const NAVTAB_HEIGHT = 48; -type Props = RouteComponentProps & { +type Props = { activeSemester: Semester; beta: boolean; }; -export const NavtabsComponent: React.FC = (props) => { - const tabProps = { - className: styles.link, - activeClassName: styles.linkActive, - }; +const tabProps = { + className: styles.link, + activeClassName: styles.linkActive, +}; - return ( - +)); const connectedNavtabs = connect((state: State) => ({ activeSemester: state.app.activeSemester, beta: !!state.settings.beta, }))(NavtabsComponent); -export default withRouter(connectedNavtabs); +export default connectedNavtabs; diff --git a/website/src/views/modules/ModuleArchiveContainer.tsx b/website/src/views/modules/ModuleArchiveContainer.tsx index 306a2bc283..9df400dedb 100644 --- a/website/src/views/modules/ModuleArchiveContainer.tsx +++ b/website/src/views/modules/ModuleArchiveContainer.tsx @@ -154,5 +154,6 @@ const connectedModuleArchiveContainer = connect( // in our Dispatch mapDispatchToProps as MapDispatchToPropsNonObject, )(ModuleArchiveContainerComponent); -const routedModuleArchiveContainer = withRouter(connectedModuleArchiveContainer); +const routedModuleArchiveContainer = connectedModuleArchiveContainer; +// const routedModuleArchiveContainer = withRouter(connectedModuleArchiveContainer); export default deferComponentRender(routedModuleArchiveContainer); diff --git a/website/src/views/modules/ModuleFinderItem.tsx b/website/src/views/modules/ModuleFinderItem.tsx index 172e21979e..748a121569 100644 --- a/website/src/views/modules/ModuleFinderItem.tsx +++ b/website/src/views/modules/ModuleFinderItem.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { Link } from 'react-router-dom'; +import Link from 'views/routes/Link'; import classnames from 'classnames'; import { ModuleInformation } from 'types/modules'; diff --git a/website/src/views/modules/ModulePageContainer.tsx b/website/src/views/modules/ModulePageContainer.tsx index 5ab3396b2c..1798724135 100644 --- a/website/src/views/modules/ModulePageContainer.tsx +++ b/website/src/views/modules/ModulePageContainer.tsx @@ -144,5 +144,6 @@ const connectedModulePageContainer = connect( // in our Dispatch mapDispatchToProps as MapDispatchToPropsNonObject, )(ModulePageContainerComponent); -const routedModulePageContainer = withRouter(connectedModulePageContainer); +// const routedModulePageContainer = withRouter(connectedModulePageContainer); +const routedModulePageContainer = connectedModulePageContainer; export default deferComponentRender(routedModulePageContainer); diff --git a/website/src/views/modules/ModulePageContent.tsx b/website/src/views/modules/ModulePageContent.tsx index 5ad3851515..3096c4e98b 100644 --- a/website/src/views/modules/ModulePageContent.tsx +++ b/website/src/views/modules/ModulePageContent.tsx @@ -82,7 +82,7 @@ class ModulePageContent extends React.Component { - + {isArchive && (
diff --git a/website/src/views/planner/PlannerModule.tsx b/website/src/views/planner/PlannerModule.tsx index c73e56cb7c..0aab556251 100644 --- a/website/src/views/planner/PlannerModule.tsx +++ b/website/src/views/planner/PlannerModule.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { Draggable } from 'react-beautiful-dnd'; -import { Link } from 'react-router-dom'; +import Link from 'views/routes/Link'; import { format } from 'date-fns'; import classnames from 'classnames'; diff --git a/website/src/views/routes/Link.tsx b/website/src/views/routes/Link.tsx new file mode 100644 index 0000000000..ec557e1aab --- /dev/null +++ b/website/src/views/routes/Link.tsx @@ -0,0 +1,68 @@ +import type { Location, To } from 'history'; + +import React, { memo, useCallback, useContext, useMemo } from 'react'; +import { normalizeToLocation, resolveToLocation } from './locationUtils'; +import RoutingContext from './RoutingContext'; + +export type LinkProps = React.AnchorHTMLAttributes & { + replace?: boolean; + to: To | ((location: Location) => string); +}; + +/** + * An alternative to react-router's Link component that works with + * our custom RoutingContext. + */ +const Link = memo(({ replace, to, children, ...aProps }) => { + const router = useContext(RoutingContext); + if (!router) { + throw new Error('You should not use outside a '); + } + + const href = useMemo(() => { + const location = normalizeToLocation( + resolveToLocation(to, router.history.location), + router.history.location, + ); + return location ? router.history.createHref(location) : ''; + }, [router.history, to]); + + // When the user clicks, change route + const changeRoute = useCallback( + (event) => { + event.preventDefault(); + const finalLocation = resolveToLocation(to, router.history.location); + const method = replace ? router.history.replace : router.history.push; + method(finalLocation); + }, + [to, router.history.location, router.history.replace, router.history.push, replace], + ); + + // Callback to preload just the code for the route: + // we pass this to onMouseEnter, which is a weaker signal + // that the user *may* navigate to the route. + const preloadRouteCode = useCallback(() => { + router.preloadCode(href); + }, [href, router]); + + // Callback to preload the code and data for the route: + // we pass this to onMouseDown, since this is a stronger + // signal that the user will likely complete the navigation + const preloadRoute = useCallback(() => { + router.preload(href); + }, [href, router]); + + return ( + + {children} + + ); +}); + +export default Link; diff --git a/website/src/views/routes/NavLink.tsx b/website/src/views/routes/NavLink.tsx new file mode 100644 index 0000000000..6f3c6ee694 --- /dev/null +++ b/website/src/views/routes/NavLink.tsx @@ -0,0 +1,65 @@ +import React, { AriaAttributes, memo, useContext } from 'react'; +import classnames from 'classnames'; +import { matchPath } from 'react-router-dom'; +import type { To } from 'history'; +import Link, { LinkProps } from './Link'; +import RoutingContext from './RoutingContext'; +import { normalizeToLocation, resolveToLocation } from './locationUtils'; +import { useLocation } from './hooks'; + +type NavLinkProps = LinkProps & { + 'aria-current'?: AriaAttributes['aria-current']; + activeClassName?: string; + className?: string; + exact?: boolean; + sensitive?: boolean; + strict?: boolean; + to: To; +}; + +const NavLink = memo( + ({ + 'aria-current': ariaCurrent = 'page', + activeClassName = 'active', + className: classNameProp, + exact, + sensitive, + strict, + to, + ...rest + }) => { + if (!useContext(RoutingContext)) { + throw new Error('You should not use outside a '); + } + + const currentLocation = useLocation(); + + const toLocation = normalizeToLocation(resolveToLocation(to, currentLocation), currentLocation); + const { pathname: path } = toLocation; + // Regex taken from: https://github.com/pillarjs/path-to-regexp/blob/master/index.js#L202 + const escapedPath = path && path.replace(/([.+*?=^!:${}()[\]|/\\])/g, '\\$1'); + + const match = escapedPath + ? matchPath(currentLocation.pathname, { + path: escapedPath, + exact, + sensitive, + strict, + }) + : null; + const isActive = !!match; + + const className = isActive ? classnames(classNameProp, activeClassName) : classNameProp; + + const props = { + 'aria-current': (isActive && ariaCurrent) || undefined, + className, + to: toLocation, + ...rest, + }; + + return ; + }, +); + +export default NavLink; diff --git a/website/src/views/routes/RouteRenderer.tsx b/website/src/views/routes/RouteRenderer.tsx new file mode 100644 index 0000000000..3e5704ee8e --- /dev/null +++ b/website/src/views/routes/RouteRenderer.tsx @@ -0,0 +1,139 @@ +import React, { + useContext, + useEffect, + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + unstable_useTransition as useTransition, + Suspense, + useState, +} from 'react'; +import LoadingSpinner from 'views/components/LoadingSpinner'; +import ApiError from 'views/errors/ApiError'; +import ErrorBoundary from 'views/errors/ErrorBoundary'; +import RoutingContext, { RouteComponentProps } from './RoutingContext'; + +const SUSPENSE_CONFIG = { timeoutMs: 2000 }; + +/** + * A component that accesses the current route entry from RoutingContext and renders + * that entry. + */ +export default function RouterRenderer() { + // Access the router + const router = useContext(RoutingContext); + // Improve the route transition UX by delaying transitions: show the previous route entry + // for a brief period while the next route is being prepared. See + // https://reactjs.org/docs/concurrent-mode-patterns.html#transitions + const [startTransition, isPending] = useTransition(SUSPENSE_CONFIG); + + // Store the active entry in state - this allows the renderer to use features like + // useTransition to delay when state changes become visible to the user. + const [routeEntry, setRouteEntry] = useState(router!.get()); + + // On mount subscribe for route changes + useEffect(() => { + // Check if the route has changed between the last render and commit: + const currentEntry = router!.get(); + if (currentEntry !== routeEntry) { + // if there was a concurrent modification, rerender and exit + setRouteEntry(currentEntry); + return undefined; + } + + // If there *wasn't* a concurrent change to the route, then the UI + // is current: subscribe for subsequent route updates + const dispose = router?.subscribe((nextEntry) => { + // startTransition() delays the effect of the setRouteEntry (setState) call + // for a brief period, continuing to show the old state while the new + // state (route) is prepared. + startTransition(() => { + setRouteEntry(nextEntry); + }); + }); + return () => dispose?.(); + + // Note: this hook updates routeEntry manually; we exclude that variable + // from the hook deps to avoid recomputing the effect after each change + // triggered by the effect itself. + // eslint-disable-next-line + }, [router, startTransition]); + + // The current route value is an array of matching entries - one entry per + // level of routes (to allow nested routes). We have to map each one to a + // RouteComponent to allow suspending, and also pass its children correctly. + // Conceptually, we want this structure: + // ``` + // + // + // // continue for nested items... + // + // + // ``` + // To achieve this, we reverse the list so we can start at the bottom-most + // component, and iteratively construct parent components w the previous + // value as the child of the next one: + const reversedItems = [...routeEntry.entries].reverse(); // reverse is in place so we make a copy + const firstItem = reversedItems[0]; + // the bottom-most component is special since it will have no children + // (though we could probably just pass null children to it) + let routeComponent = ( + + ); + for (let ii = 1; ii < reversedItems.length; ii++) { + const nextItem = reversedItems[ii]; + routeComponent = ( + + {routeComponent} + + ); + } + + // Routes can error so wrap in an + // Routes can suspend, so wrap in + return ( + }> + }> + {/* Indicate to the user that a transition is pending, even while showing the previous UI */} + {isPending ?
Loading pending...
: null} + {routeComponent} + + + ); +} + +/** + * The `component` property from the route entry is a Resource, which may or may not be ready. + * We use a helper child component to unwrap the resource with component.read(), and then + * render it if its ready. + * + * NOTE: calling routeEntry.route.component.read() directly in RouteRenderer woldn't work the + * way we'd expect. Because that method could throw - either suspending or on error - the error + * would bubble up to the *caller* of RouteRenderer. We want the suspend/error to bubble up to + * our ErrorBoundary/Suspense components, so we have to ensure that the suspend/error happens + * in a child component. + */ +const RouteComponent: React.FC> = ({ + component, + routeData, + prepared, + children, +}) => { + const Component = component.read(); + return ( + + {children} + + ); +}; diff --git a/website/src/views/routes/RoutingContext.ts b/website/src/views/routes/RoutingContext.ts new file mode 100644 index 0000000000..25efc3534d --- /dev/null +++ b/website/src/views/routes/RoutingContext.ts @@ -0,0 +1,47 @@ +import type { Location, BrowserHistory, State } from 'history'; +import type { RouteConfig } from 'react-router-config'; +import type { JSResource } from 'utils/JSResource'; + +import { createContext } from 'react'; +import { match } from 'react-router-dom'; + +type PreparedProps = { [key: string]: JSResource }; + +export interface EntryPointRouteConfig { + key?: React.Key; + location?: Location; + path?: string | string[]; + exact?: boolean; + strict?: boolean; + routes?: RouteConfig[]; + componentResource: JSResource; + prepare: (matchParams: Params) => PreparedProps; +} + +export type RouteComponentProps = { + component: JSResource; + prepared: PreparedProps; + routeData: match; +}; + +export type RouteEntry = { + location: Location; + entries: RouteComponentProps[]; +}; + +export type RoutingSubscriber = (nextEntry: RouteEntry) => void; + +export type RoutingContextType = Readonly<{ + history: BrowserHistory; + get(): RouteEntry; + preloadCode(pathname: string): void; + preload(pathname: string): void; + subscribe(cb: RoutingSubscriber): () => void; +}>; + +const RoutingContext = createContext(null); + +/** + * A custom context instance for our router type + */ +export default RoutingContext; diff --git a/website/src/views/routes/createRouter.ts b/website/src/views/routes/createRouter.ts new file mode 100644 index 0000000000..087aca7309 --- /dev/null +++ b/website/src/views/routes/createRouter.ts @@ -0,0 +1,121 @@ +import { BrowserHistoryOptions, createBrowserHistory } from 'history'; +import { MatchedRoute, matchRoutes } from 'react-router-config'; +import { + EntryPointRouteConfig, + PreparedRouteEntry, + RouteEntry, + RoutingContextType, + RoutingSubscriber, +} from './RoutingContext'; + +interface MatchedEntryPointRoute + extends MatchedRoute { + route: EntryPointRouteConfig; +} + +/** + * Match the current location to the corresponding route entry. + */ +function matchEntryPointRoutes( + routes: EntryPointRouteConfig[], + locationPathname: string, +): MatchedEntryPointRoute[] { + const matchedRoutes = (matchRoutes( + routes, + locationPathname, + ) as unknown) as MatchedEntryPointRoute[]; + if (!Array.isArray(matchedRoutes) || matchedRoutes.length === 0) { + throw new Error(`No route for ${locationPathname}`); + } + return matchedRoutes; +} + +/** + * Load the data for the matched route, given the params extracted from the route + */ +function prepareMatches( + matches: MatchedEntryPointRoute[], +): PreparedRouteEntry[] { + return matches.map((match) => { + const { route, match: matchData } = match; + const prepared = route.prepare(matchData.params); + const Component = route.componentResource.get(); + if (Component == null) { + route.componentResource.preloadOrReloadIfError(); + } + return { component: route.componentResource, prepared, routeData: matchData }; + }); +} + +/** + * A custom router built from the same primitives as react-router. Each object in `routes` + * contains both a Component and a prepare() function that can preload data for the component. + * The router watches for changes to the current location via the `history` package, maps the + * location to the corresponding route entry, and then preloads the code and data for the route. + */ +export default function createRouter( + routes: EntryPointRouteConfig[], + options: BrowserHistoryOptions | undefined, +) { + // Initialize history + const history = createBrowserHistory(options); + + // Find the initial match and prepare it + const initialMatches = matchEntryPointRoutes(routes, history.location.pathname); + const initialEntries = prepareMatches(initialMatches); + let currentEntry: RouteEntry = { + location: history.location, + entries: initialEntries, + }; + + // maintain a set of subscribers to the active entry + let nextId = 0; + const subscribers = new Map(); + + // Listen for location changes, match to the route entry, prepare the entry, + // and notify subscribers. Note that this pattern ensures that data-loading + // occurs *outside* of - and *before* - rendering. + const cleanup = history.listen(({ location }) => { + if (location.pathname === currentEntry.location.pathname) { + return; + } + const matches = matchEntryPointRoutes(routes, location.pathname); + const entries = prepareMatches(matches); + const nextEntry: RouteEntry = { + location, + entries, + }; + currentEntry = nextEntry; + subscribers.forEach((cb) => cb(nextEntry)); + }); + + // The actual object that will be passed on the RoutingConext. + const context: RoutingContextType = { + history, + get() { + return currentEntry; + }, + preloadCode(pathname) { + // preload just the code for a route, without storing the result + const matches = matchEntryPointRoutes(routes, pathname); + matches.forEach(({ route }) => route.componentResource.preloadOrReloadIfError()); + }, + preload(pathname) { + // preload the code and data for a route, without storing the result + const matches = matchEntryPointRoutes(routes, pathname); + prepareMatches(matches); + }, + subscribe(cb) { + const id = nextId; + nextId += 1; + const dispose = () => { + subscribers.delete(id); + }; + subscribers.set(id, cb); + return dispose; + }, + }; + + // Return both the context object and a cleanup function + return { cleanup, context }; +} diff --git a/website/src/views/routes/createRoutes.ts b/website/src/views/routes/createRoutes.ts new file mode 100644 index 0000000000..ba2f21d2eb --- /dev/null +++ b/website/src/views/routes/createRoutes.ts @@ -0,0 +1,77 @@ +import type { Dispatch } from 'redux'; +import { JSResource } from 'utils/JSResource'; +import { fetchModuleList } from 'actions/moduleBank'; +import { captureException } from '@sentry/browser'; +import venueLocationResource from 'views/components/map/venueLocationResource'; +import { EntryPointRouteConfig } from './RoutingContext'; + +// TODO: Define entry points closer to modules +export default function createRoutes(dispatch: Dispatch): EntryPointRouteConfig[] { + return [ + { + componentResource: JSResource( + 'AppShell', + () => import(/* webpackChunkName: "AppShell.route" */ 'views/AppShell'), + ), + prepare() { + const moduleList = JSResource('moduleList', async () => { + try { + // Typed as unknown because we don't actually need the output + return (dispatch(fetchModuleList()) as unknown) as Promise; + } catch (error) { + captureException(error); + throw error; + } + }); + moduleList.preload(); + return { + moduleList, + }; + }, + routes: [ + { + path: '/timetable/:semester?/:action?', + exact: true, + componentResource: JSResource( + 'TimetableContainer', + () => + import( + /* webpackChunkName: "TimetableContainer.route" */ 'views/timetable/TimetableContainer' + ), + ), + prepare: ({ semester, action }) => ({}), + }, + { + path: '/today', + exact: true, + componentResource: JSResource( + 'TodayContainer', + () => + import(/* webpackChunkName: "TodayContainer.route" */ 'views/today/TodayContainer'), + ), + prepare: () => { + venueLocationResource.preloadOrReloadIfError(); + return {}; + }, + }, + // { + // path: '/issue/:id', + // component: JSResource('IssueDetailRoot', () => import('./IssueDetailRoot')), + // prepare: (params) => { + // const IssueDetailQuery = require('./__generated__/IssueDetailRootQuery.graphql'); + // return { + // issueDetailQuery: preloadQuery( + // RelayEnvironment, + // IssueDetailQuery, + // { + // id: params.id, + // }, + // { fetchPolicy: 'store-or-network' }, + // ), + // }; + // }, + // }, + ], + }, + ]; +} diff --git a/website/src/views/routes/hooks.ts b/website/src/views/routes/hooks.ts new file mode 100644 index 0000000000..8a0c1e23a3 --- /dev/null +++ b/website/src/views/routes/hooks.ts @@ -0,0 +1,52 @@ +import type { Location, History } from 'history'; + +import { useContext, useMemo } from 'react'; + +// FIXME: Find a way to replace this with useMutableSource if possible; +// useSubscription may tear. See +// https://gist.github.com/bvaughn/054b82781bec875345bd85a5b1344698 +import { useSubscription, Subscription } from 'use-subscription'; + +import RoutingContext from './RoutingContext'; + +export function useHistory() { + const router = useContext(RoutingContext); + if (!router) { + throw new Error('You should not use useHistory outside a '); + } + + const subscription = useMemo>( + () => ({ + getCurrentValue: () => router.history, + subscribe(callback) { + const dispose = router.subscribe(callback); + return () => dispose(); + }, + }), + [router], + ); + const history = useSubscription(subscription); + + return history; +} + +export function useLocation() { + const router = useContext(RoutingContext); + if (!router) { + throw new Error('You should not use useLocation outside a '); + } + + const subscription = useMemo>( + () => ({ + getCurrentValue: () => router.history.location, + subscribe(callback) { + const dispose = router.subscribe(callback); + return () => dispose(); + }, + }), + [router], + ); + const location = useSubscription(subscription); + + return location; +} diff --git a/website/src/views/routes/locationUtils.ts b/website/src/views/routes/locationUtils.ts new file mode 100644 index 0000000000..44b1e30324 --- /dev/null +++ b/website/src/views/routes/locationUtils.ts @@ -0,0 +1,111 @@ +/* eslint-disable @typescript-eslint/ban-types */ + +import type { Location, PartialPath, State, To } from 'history'; + +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore +import resolvePathname from 'resolve-pathname'; + +// Source: https://github.com/ReactTraining/history/blob/3f69f9e07b0a739419704cffc3b3563133281548/modules/PathUtils.js#L24 +function parsePath(path: string) { + let pathname = path || '/'; + let search = ''; + let hash = ''; + + const hashIndex = pathname.indexOf('#'); + if (hashIndex !== -1) { + hash = pathname.substr(hashIndex); + pathname = pathname.substr(0, hashIndex); + } + + const searchIndex = pathname.indexOf('?'); + if (searchIndex !== -1) { + search = pathname.substr(searchIndex); + pathname = pathname.substr(0, searchIndex); + } + + return { + pathname, + search: search === '?' ? '' : search, + hash: hash === '#' ? '' : hash, + }; +} + +// Source: https://github.com/ReactTraining/history/blob/3f69f9e07b0a739419704cffc3b3563133281548/modules/LocationUtils.js#L6 +function createLocation( + path: string | Location, + state: State, + key: string | null, + currentLocation: Location, +): Location { + let location: Partial; + if (typeof path === 'string') { + // Two-arg form: push(path, state) + location = parsePath(path); + location.state = state; + } else { + // One-arg form: push(location) + location = { ...path }; + + if (location.pathname === undefined) location.pathname = ''; + + if (location.search) { + if (location.search.charAt(0) !== '?') location.search = `?${location.search}`; + } else { + location.search = ''; + } + + if (location.hash) { + if (location.hash.charAt(0) !== '#') location.hash = `#${location.hash}`; + } else { + location.hash = ''; + } + + if (state !== undefined && location.state === undefined) location.state = state; + } + + try { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + location.pathname = decodeURI(location.pathname!); + } catch (e) { + if (e instanceof URIError) { + throw new URIError( + `Pathname "${location.pathname}" could not be decoded. ` + + `This is likely caused by an invalid percent-encoding.`, + ); + } else { + throw e; + } + } + + if (key) location.key = key; + + if (currentLocation) { + // Resolve incomplete/relative pathname relative to current location. + if (!location.pathname) { + location.pathname = currentLocation.pathname; + } else if (location.pathname.charAt(0) !== '/') { + location.pathname = resolvePathname(location.pathname, currentLocation.pathname); + } + } else { + // When there is no prior location and pathname is empty, set it to / + // eslint-disable-next-line no-lonely-if + if (!location.pathname) { + location.pathname = '/'; + } + } + + return location as Location; // FIXME: Dangerous cast as state may be undefined +} + +// Source: https://github.com/ReactTraining/react-router/blob/9016c5191cb1b8ce727dc5aaed97376b1b291a70/packages/react-router-dom/modules/utils/locationUtils.js +export function resolveToLocation( + to: To | ((location: Location) => string), + currentLocation: Location, +): To { + return typeof to === 'function' ? to(currentLocation) : to; +} + +export function normalizeToLocation(to: To, currentLocation: Location): PartialPath { + return typeof to === 'string' ? createLocation(to, null, null, currentLocation) : to; +} diff --git a/website/src/views/static/FaqContainer.tsx b/website/src/views/static/FaqContainer.tsx index f1540c582b..6ce8eb1611 100644 --- a/website/src/views/static/FaqContainer.tsx +++ b/website/src/views/static/FaqContainer.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { Link } from 'react-router-dom'; +import Link from 'views/routes/Link'; import config from 'config'; import ExternalLink from 'views/components/ExternalLink'; import StaticPage from './StaticPage'; diff --git a/website/src/views/timetable/ExamWeek.tsx b/website/src/views/timetable/ExamWeek.tsx index 138c18f6b3..509c958348 100644 --- a/website/src/views/timetable/ExamWeek.tsx +++ b/website/src/views/timetable/ExamWeek.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { Link } from 'react-router-dom'; +import Link from 'views/routes/Link'; import { range } from 'lodash'; import { isSameDay, addDays } from 'date-fns'; diff --git a/website/src/views/timetable/ExportMenu.tsx b/website/src/views/timetable/ExportMenu.tsx index cc1481b90e..3cbfb4b917 100644 --- a/website/src/views/timetable/ExportMenu.tsx +++ b/website/src/views/timetable/ExportMenu.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import Downshift, { ChildrenFunction } from 'downshift'; import { connect } from 'react-redux'; import classnames from 'classnames'; -import { Link } from 'react-router-dom'; +import Link from 'views/routes/Link'; import { AlertTriangle, Calendar, ChevronDown, Download, FileText, Image } from 'react-feather'; import { Semester } from 'types/modules'; diff --git a/website/src/views/timetable/TimetableContainer.tsx b/website/src/views/timetable/TimetableContainer.tsx index 06c2a9267a..3144b49853 100644 --- a/website/src/views/timetable/TimetableContainer.tsx +++ b/website/src/views/timetable/TimetableContainer.tsx @@ -16,7 +16,6 @@ import { getModuleCondensed } from 'selectors/moduleBank'; import { deserializeTimetable } from 'utils/timetables'; import { fillColorMapping } from 'utils/colors'; import { semesterForTimetablePage, TIMETABLE_SHARE, timetablePage } from 'views/routes/paths'; -import deferComponentRender from 'views/hocs/deferComponentRender'; import { Repeat } from 'react-feather'; import SemesterSwitcher from 'views/components/semester-switcher/SemesterSwitcher'; import LoadingSpinner from 'views/components/LoadingSpinner'; @@ -66,9 +65,10 @@ export class TimetableContainerComponent extends React.PureComponent; +const EventMap = memo(({ venue }) => { + const venueLocations = venueLocationResource.fetch(); -const EventMap: React.FC = (props) => { - if (!props.venue) { + if (!venue) { return (
@@ -24,13 +22,13 @@ const EventMap: React.FC = (props) => { ); } - const venueLocation = props.venueLocations[props.venue]; + const venueLocation = venueLocations[venue]; if (!venueLocation || !venueLocation.location) { return

We don't have information about this venue :(

; } const position: [number, number] = [venueLocation.location.y, venueLocation.location.x]; return ; -}; +}); export default EventMap; diff --git a/website/src/views/today/EventMap/index.tsx b/website/src/views/today/EventMap/index.tsx deleted file mode 100644 index 11e352417b..0000000000 --- a/website/src/views/today/EventMap/index.tsx +++ /dev/null @@ -1,9 +0,0 @@ -import withVenueLocations from 'views/components/map/withVenueLocations'; -import { Props } from './EventMap'; - -const EventMap = withVenueLocations(() => - // TypeScript is sad about resolving dynamic import - import(/* webpackChunkName: "venue" */ './EventMap').then((module) => module.default), -); - -export default EventMap; diff --git a/website/src/views/today/EventMapInline/EventMapInline.scss b/website/src/views/today/EventMapInline.scss similarity index 100% rename from website/src/views/today/EventMapInline/EventMapInline.scss rename to website/src/views/today/EventMapInline.scss diff --git a/website/src/views/today/EventMapInline/EventMapInline.tsx b/website/src/views/today/EventMapInline.tsx similarity index 69% rename from website/src/views/today/EventMapInline/EventMapInline.tsx rename to website/src/views/today/EventMapInline.tsx index dabc664fd5..c118b62402 100644 --- a/website/src/views/today/EventMapInline/EventMapInline.tsx +++ b/website/src/views/today/EventMapInline.tsx @@ -1,29 +1,22 @@ -import React from 'react'; +import React, { memo } from 'react'; import classnames from 'classnames'; -import { LatLngTuple, Venue, VenueLocation, VenueLocationMap } from 'types/venues'; +import { LatLngTuple, Venue, VenueLocation } from 'types/venues'; import LocationMap from 'views/components/map/LocationMap'; +import venueLocationResource from 'views/components/map/venueLocationResource'; import styles from './EventMapInline.scss'; -export type OwnProps = Readonly<{ +export type Props = { isOpen: boolean; className?: string; venue: Venue; toggleOpen: () => void; -}>; - -export type Props = OwnProps & { - readonly venueLocations: VenueLocationMap; }; -const EventMapInline: React.FunctionComponent = ({ - venue, - isOpen, - className, - toggleOpen, - venueLocations, -}) => { +const EventMapInline = memo(({ venue, isOpen, className, toggleOpen }) => { + const venueLocations = venueLocationResource.fetch(); + const venueLocation: VenueLocation = venueLocations[venue]; if (!venueLocation || !venueLocation.location) { return null; @@ -45,6 +38,6 @@ const EventMapInline: React.FunctionComponent = ({
); -}; +}); export default EventMapInline; diff --git a/website/src/views/today/EventMapInline/index.tsx b/website/src/views/today/EventMapInline/index.tsx deleted file mode 100644 index d4e02c0059..0000000000 --- a/website/src/views/today/EventMapInline/index.tsx +++ /dev/null @@ -1,8 +0,0 @@ -import withVenueLocations from 'views/components/map/withVenueLocations'; -import { Props } from './EventMapInline'; - -export default withVenueLocations( - () => import(/* webpackChunkName: "venue" */ './EventMapInline').then((module) => module.default), - // Don't show spinner or errors inline - { Loading: () => null, Error: () => null }, -); diff --git a/website/src/views/today/TodayContainer/TodayContainer.scss b/website/src/views/today/TodayContainer.scss similarity index 100% rename from website/src/views/today/TodayContainer/TodayContainer.scss rename to website/src/views/today/TodayContainer.scss diff --git a/website/src/views/today/TodayContainer/TodayContainer.test.tsx b/website/src/views/today/TodayContainer.test.tsx similarity index 98% rename from website/src/views/today/TodayContainer/TodayContainer.test.tsx rename to website/src/views/today/TodayContainer.test.tsx index 82bb3eec9b..2f5bd3d4bd 100644 --- a/website/src/views/today/TodayContainer/TodayContainer.test.tsx +++ b/website/src/views/today/TodayContainer.test.tsx @@ -9,8 +9,8 @@ import { captureException } from 'utils/error'; import { Props, DaySection, TodayContainerComponent, mapStateToProps } from './TodayContainer'; import forecasts from './__mocks__/forecasts.json'; -import DayEvents from '../DayEvents'; -import styles from '../DayEvents.scss'; +import DayEvents from './DayEvents'; +import styles from './DayEvents.scss'; /* eslint-disable no-useless-computed-key */ diff --git a/website/src/views/today/TodayContainer/TodayContainer.tsx b/website/src/views/today/TodayContainer.tsx similarity index 98% rename from website/src/views/today/TodayContainer/TodayContainer.tsx rename to website/src/views/today/TodayContainer.tsx index 41aa27ec49..9308538eeb 100644 --- a/website/src/views/today/TodayContainer/TodayContainer.tsx +++ b/website/src/views/today/TodayContainer.tsx @@ -41,11 +41,11 @@ import { formatTime, getDayIndex } from 'utils/timify'; import { breakpointUp } from 'utils/css'; import { State as StoreState } from 'types/state'; -import DayEvents from '../DayEvents'; -import DayHeader from '../DayHeader'; -import EmptyLessonGroup from '../EmptyLessonGroup'; -import BeforeLessonCard from '../BeforeLessonCard'; -import EventMap from '../EventMap'; +import DayEvents from './DayEvents'; +import DayHeader from './DayHeader'; +import EmptyLessonGroup from './EmptyLessonGroup'; +import BeforeLessonCard from './BeforeLessonCard'; +import EventMap from './EventMap'; import styles from './TodayContainer.scss'; const EMPTY_LESSONS: ColoredLesson[] = []; diff --git a/website/src/views/today/TodayContainer/index.tsx b/website/src/views/today/TodayContainer/index.tsx deleted file mode 100644 index 1237b2f905..0000000000 --- a/website/src/views/today/TodayContainer/index.tsx +++ /dev/null @@ -1,31 +0,0 @@ -import * as React from 'react'; -import Loadable, { LoadingComponentProps } from 'react-loadable'; - -import LoadingSpinner from 'views/components/LoadingSpinner'; -import ApiError from 'views/errors/ApiError'; -import { retryImport } from 'utils/error'; -import EventMapInline from '../EventMapInline'; -import EventMap from '../EventMap'; - -const AsyncTodayContainer = Loadable({ - loader: () => retryImport(() => import(/* webpackChunkName: "today" */ './TodayContainer')), - loading: (props: LoadingComponentProps) => { - if (props.error) { - return ; - } - - if (props.pastDelay) { - return ; - } - - return null; - }, -}); - -export default AsyncTodayContainer; - -export function preload() { - AsyncTodayContainer.preload(); - EventMapInline.preload(); - EventMap.preload(); -} diff --git a/website/src/views/today/TodayContainer/__mocks__/forecasts.json b/website/src/views/today/__mocks__/forecasts.json similarity index 100% rename from website/src/views/today/TodayContainer/__mocks__/forecasts.json rename to website/src/views/today/__mocks__/forecasts.json diff --git a/website/src/views/venues/VenueDetails.tsx b/website/src/views/venues/VenueDetails.tsx index 52a41f78b5..edfaebcccb 100644 --- a/website/src/views/venues/VenueDetails.tsx +++ b/website/src/views/venues/VenueDetails.tsx @@ -1,7 +1,8 @@ import * as React from 'react'; -import { Link, RouteComponentProps, withRouter } from 'react-router-dom'; +import { RouteComponentProps } from 'react-router-dom'; import classnames from 'classnames'; import { flatMap } from 'lodash'; +import Link from 'views/routes/Link'; import { DayAvailability, TimePeriod, Venue } from 'types/venues'; import { Lesson } from 'types/timetables'; @@ -97,4 +98,5 @@ const ResponsiveVenueDetails = makeResponsive( React.memo(VenueDetailsComponent), breakpointDown('lg'), ); -export default withRouter(ResponsiveVenueDetails); +export default ResponsiveVenueDetails; +// export default withRouter(ResponsiveVenueDetails); diff --git a/website/src/views/venues/VenueList.tsx b/website/src/views/venues/VenueList.tsx index 3f027c2118..f0fa6b53d8 100644 --- a/website/src/views/venues/VenueList.tsx +++ b/website/src/views/venues/VenueList.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import classnames from 'classnames'; import { groupBy, toPairs, sortBy } from 'lodash'; -import { Link, LinkProps } from 'react-router-dom'; +import Link, { LinkProps } from 'views/routes/Link'; import { Venue } from 'types/venues'; import { venuePage } from 'views/routes/paths'; diff --git a/website/src/views/venues/VenuesContainer.tsx b/website/src/views/venues/VenuesContainer.tsx index 24fd9e0ab8..508c5eac29 100644 --- a/website/src/views/venues/VenuesContainer.tsx +++ b/website/src/views/venues/VenuesContainer.tsx @@ -368,7 +368,8 @@ export class VenuesContainerComponent extends React.Component { // Explicitly declare top level components for React hot reloading to work. const ResponsiveVenuesContainer = makeResponsive(VenuesContainerComponent, breakpointDown('sm')); -const RoutedVenuesContainer = withRouter(ResponsiveVenuesContainer); +// const RoutedVenuesContainer = withRouter(ResponsiveVenuesContainer); +const RoutedVenuesContainer = ResponsiveVenuesContainer; const AsyncVenuesContainer = Loadable.Map, { venues: AxiosResponse }>({ loader: { venues: () => axios.get(nusmods.venuesUrl(config.semester)), diff --git a/website/webpack/webpack.config.dev.js b/website/webpack/webpack.config.dev.js index 396732373c..5f9b97dad8 100644 --- a/website/webpack/webpack.config.dev.js +++ b/website/webpack/webpack.config.dev.js @@ -26,12 +26,12 @@ const developmentConfig = merge([ 'webpack/hot/only-dev-server', 'entry/main', ], - resolve: { - alias: { - // Replace React DOM with the hot reload patched version in development - 'react-dom': '@hot-loader/react-dom', - }, - }, + // resolve: { + // alias: { + // // Replace React DOM with the hot reload patched version in development + // 'react-dom': '@hot-loader/react-dom', + // }, + // }, plugins: [ new HtmlWebpackPlugin({ template: path.join(parts.PATHS.src, 'index.html'), From b9eccea23b95426e66035bfebb467b599a10e197 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Wed, 21 Oct 2020 19:36:29 +0800 Subject: [PATCH 04/20] Convert TimetableContainer to FC and fix sem switcher --- website/src/views/routes/RouteRenderer.tsx | 18 +- website/src/views/routes/createRouter.ts | 4 +- .../views/timetable/TimetableContainer.tsx | 287 ++++++++++-------- 3 files changed, 166 insertions(+), 143 deletions(-) diff --git a/website/src/views/routes/RouteRenderer.tsx b/website/src/views/routes/RouteRenderer.tsx index 3e5704ee8e..a6fc1af466 100644 --- a/website/src/views/routes/RouteRenderer.tsx +++ b/website/src/views/routes/RouteRenderer.tsx @@ -18,9 +18,13 @@ const SUSPENSE_CONFIG = { timeoutMs: 2000 }; * A component that accesses the current route entry from RoutingContext and renders * that entry. */ -export default function RouterRenderer() { +const RouterRenderer: React.FC = () => { // Access the router const router = useContext(RoutingContext); + if (!router) { + throw new Error('You should not use outside a '); + } + // Improve the route transition UX by delaying transitions: show the previous route entry // for a brief period while the next route is being prepared. See // https://reactjs.org/docs/concurrent-mode-patterns.html#transitions @@ -28,12 +32,12 @@ export default function RouterRenderer() { // Store the active entry in state - this allows the renderer to use features like // useTransition to delay when state changes become visible to the user. - const [routeEntry, setRouteEntry] = useState(router!.get()); + const [routeEntry, setRouteEntry] = useState(router.get()); // On mount subscribe for route changes useEffect(() => { // Check if the route has changed between the last render and commit: - const currentEntry = router!.get(); + const currentEntry = router.get(); if (currentEntry !== routeEntry) { // if there was a concurrent modification, rerender and exit setRouteEntry(currentEntry); @@ -42,7 +46,7 @@ export default function RouterRenderer() { // If there *wasn't* a concurrent change to the route, then the UI // is current: subscribe for subsequent route updates - const dispose = router?.subscribe((nextEntry) => { + const dispose = router.subscribe((nextEntry) => { // startTransition() delays the effect of the setRouteEntry (setState) call // for a brief period, continuing to show the old state while the new // state (route) is prepared. @@ -50,7 +54,7 @@ export default function RouterRenderer() { setRouteEntry(nextEntry); }); }); - return () => dispose?.(); + return () => dispose(); // Note: this hook updates routeEntry manually; we exclude that variable // from the hook deps to avoid recomputing the effect after each change @@ -111,7 +115,7 @@ export default function RouterRenderer() { ); -} +}; /** * The `component` property from the route entry is a Resource, which may or may not be ready. @@ -137,3 +141,5 @@ const RouteComponent: React.FC ); }; + +export default RouterRenderer; diff --git a/website/src/views/routes/createRouter.ts b/website/src/views/routes/createRouter.ts index 087aca7309..2128aa00cf 100644 --- a/website/src/views/routes/createRouter.ts +++ b/website/src/views/routes/createRouter.ts @@ -2,7 +2,7 @@ import { BrowserHistoryOptions, createBrowserHistory } from 'history'; import { MatchedRoute, matchRoutes } from 'react-router-config'; import { EntryPointRouteConfig, - PreparedRouteEntry, + RouteComponentProps, RouteEntry, RoutingContextType, RoutingSubscriber, @@ -35,7 +35,7 @@ function matchEntryPointRoutes( */ function prepareMatches( matches: MatchedEntryPointRoute[], -): PreparedRouteEntry[] { +): RouteComponentProps[] { return matches.map((match) => { const { route, match: matchData } = match; const prepared = route.prepare(matchData.params); diff --git a/website/src/views/timetable/TimetableContainer.tsx b/website/src/views/timetable/TimetableContainer.tsx index 3144b49853..83eac7ba0e 100644 --- a/website/src/views/timetable/TimetableContainer.tsx +++ b/website/src/views/timetable/TimetableContainer.tsx @@ -1,21 +1,25 @@ -import * as React from 'react'; +import React, { memo, useCallback, useEffect, useMemo, useState } from 'react'; import { connect } from 'react-redux'; -import { Redirect, RouteComponentProps, withRouter } from 'react-router-dom'; +import { match as Match, Redirect } from 'react-router-dom'; import classnames from 'classnames'; import { ModuleCode, Semester } from 'types/modules'; import { SemTimetableConfig } from 'types/timetables'; import { ColorMapping, ModulesMap, NotificationOptions } from 'types/reducers'; -import { selectSemester } from 'actions/settings'; +import { selectSemester as selectSemesterAction } from 'actions/settings'; import { getSemesterTimetable } from 'selectors/timetables'; -import { fetchTimetableModules, setTimetable } from 'actions/timetables'; -import { openNotification } from 'actions/app'; -import { undo } from 'actions/undoHistory'; +import { + fetchTimetableModules as fetchTimetableModulesAction, + setTimetable as setTimetableAction, +} from 'actions/timetables'; +import { openNotification as openNotificationAction } from 'actions/app'; +import { undo as undoAction } from 'actions/undoHistory'; import { getModuleCondensed } from 'selectors/moduleBank'; import { deserializeTimetable } from 'utils/timetables'; import { fillColorMapping } from 'utils/colors'; import { semesterForTimetablePage, TIMETABLE_SHARE, timetablePage } from 'views/routes/paths'; +import { useHistory } from 'views/routes/hooks'; import { Repeat } from 'react-feather'; import SemesterSwitcher from 'views/components/semester-switcher/SemesterSwitcher'; import LoadingSpinner from 'views/components/LoadingSpinner'; @@ -30,7 +34,9 @@ export type QueryParam = { semester: string; }; -type OwnProps = RouteComponentProps; +type OwnProps = { + match: Match; +}; type Props = OwnProps & { modules: ModulesMap; @@ -40,19 +46,15 @@ type Props = OwnProps & { colors: ColorMapping; isValidModule: (moduleCode: ModuleCode) => boolean; - selectSemester: (semester: Semester) => void; - setTimetable: ( + selectSemesterProp: (semester: Semester) => void; + setTimetableProp: ( semester: Semester, semTimetableConfig: SemTimetableConfig, colorMapping: ColorMapping, ) => void; - fetchTimetableModules: (semTimetableConfig: SemTimetableConfig[]) => void; - openNotification: (str: string, notificationOptions: NotificationOptions) => void; - undo: () => void; -}; - -type State = { - importedTimetable: SemTimetableConfig | null; + fetchTimetableModulesProp: (semTimetableConfig: SemTimetableConfig[]) => void; + openNotificationProp: (str: string, notificationOptions: NotificationOptions) => void; + undoProp: () => void; }; /** @@ -61,123 +63,138 @@ type State = { * - Import timetable data from query string if action is defined * - Create the UI for the user to confirm their actions */ -export class TimetableContainerComponent extends React.PureComponent { - constructor(props: Props) { - super(props); - - // const { semester, match, location } = props; - // const importedTimetable = - // semester && match.params.action ? deserializeTimetable(location.search) : null; - const importedTimetable = null; - - this.state = { - importedTimetable, - }; - } - - componentDidMount() { - if (this.state.importedTimetable) { - this.props.fetchTimetableModules([this.state.importedTimetable]); - } - } +export const TimetableContainerComponent = memo( + ({ + match, - selectSemester = (semester: Semester) => { - this.props.selectSemester(semester); - - this.props.history.push({ - ...this.props.history.location, - pathname: timetablePage(semester), - }); - }; + modules, + semester, + activeSemester, + timetable, + colors, - isLoading() { - // Check that all modules are fully loaded into the ModuleBank - const { modules, timetable } = this.props; - const { importedTimetable } = this.state; + isValidModule, + selectSemesterProp, + setTimetableProp, + fetchTimetableModulesProp, + openNotificationProp, + undoProp, + }) => { + const history = useHistory(); + const { location } = history; + const { action } = match.params; - const moduleCodes = new Set(Object.keys(timetable)); - if (importedTimetable) { - Object.keys(importedTimetable) - .filter(this.props.isValidModule) - .forEach((moduleCode) => moduleCodes.add(moduleCode)); - } + const [importedTimetable, setImportedTimetable] = useState(() => + semester && action ? deserializeTimetable(location.search) : null, + ); - // TODO: Account for loading error - return Array.from(moduleCodes).some((moduleCode) => !modules[moduleCode]); - } - - importTimetable(semester: Semester, timetable: SemTimetableConfig) { - const colors = fillColorMapping(timetable, this.props.colors); - this.props.setTimetable(semester, timetable, colors); - this.clearImportedTimetable(); - - this.props.openNotification('Timetable imported', { - timeout: 12000, - overwritable: true, - action: { - text: 'Undo', - handler: this.props.undo, + useEffect(() => { + // TODO: Preload this + if (importedTimetable) { + fetchTimetableModulesProp([importedTimetable]); + } + }, [fetchTimetableModulesProp, importedTimetable]); + + const selectSemester = useCallback( + (selectedSemester: Semester) => { + selectSemesterProp(selectedSemester); + + history.push({ + ...location, + pathname: timetablePage(selectedSemester), + }); }, - }); - } - - clearImportedTimetable = () => { - const { semester } = this.props; - if (semester) { - this.setState({ importedTimetable: null }, () => - this.props.history.push(timetablePage(semester)), - ); - } - }; + [history, location, selectSemesterProp], + ); - sharingHeader(semester: Semester, timetable: SemTimetableConfig) { - return ( -
- - -
-
-

This timetable was shared with you

-

- Clicking import will replace your saved timetable with the one below. -

-
+ const isLoading = useMemo(() => { + // Check that all modules are fully loaded into the ModuleBank + const moduleCodes = new Set(Object.keys(timetable)); + if (importedTimetable) { + Object.keys(importedTimetable) + .filter(isValidModule) + .forEach((moduleCode) => moduleCodes.add(moduleCode)); + } + + // TODO: Account for loading error + return Array.from(moduleCodes).some((moduleCode) => !modules[moduleCode]); + }, [importedTimetable, isValidModule, modules, timetable]); + + const clearImportedTimetable = useCallback(() => { + if (semester) { + setImportedTimetable(null); + history.push(timetablePage(semester)); + } + }, [history, semester]); + + const importTimetable = useCallback( + (guaranteedSemester: Semester, sharedTimetable: SemTimetableConfig) => { + const filledColors = fillColorMapping(sharedTimetable, colors); + setTimetableProp(guaranteedSemester, sharedTimetable, filledColors); + clearImportedTimetable(); + + openNotificationProp('Timetable imported', { + timeout: 12000, + overwritable: true, + action: { + text: 'Undo', + handler: undoProp, + }, + }); + }, + [clearImportedTimetable, colors, openNotificationProp, setTimetableProp, undoProp], + ); -
- - + const renderSharingHeader = useCallback( + (guaranteedSemester: Semester, sharedTimetable: SemTimetableConfig) => { + return ( +
+ + +
+
+

This timetable was shared with you

+

+ Clicking import will replace your saved timetable with the one + below. +

+
+ +
+ + +
+
-
-
+ ); + }, + [clearImportedTimetable, importTimetable], ); - } - timetableHeader(semester: Semester, readOnly?: boolean) { - return ( - + const renderTimetableHeader = useCallback( + (guaranteedSemester: Semester, readOnly?: boolean) => { + return ( + + ); + }, + [selectSemester], ); - } - - render() { - const { timetable, semester, activeSemester, match } = this.props; - const { importedTimetable } = this.state; - const { action } = match.params; // 1. If the URL doesn't look correct, we'll direct the user to the home page if (semester == null || (action && action !== TIMETABLE_SHARE)) { @@ -186,23 +203,23 @@ export class TimetableContainerComponent extends React.PureComponent; } // 3. Construct the color map const displayedTimetable = importedTimetable || timetable; - const colors = fillColorMapping(displayedTimetable, this.props.colors); + const filledColors = fillColorMapping(displayedTimetable, colors); // 4. If there is an imported timetable, we show the sharing header which // asks the user if they want to import the shared timetable const header = importedTimetable ? ( <> - {this.sharingHeader(semester, importedTimetable)} - {this.timetableHeader(semester, true)} + {renderSharingHeader(semester, importedTimetable)} + {renderTimetableHeader(semester, true)} ) : ( - this.timetableHeader(semester) + renderTimetableHeader(semester) ); return ( @@ -213,14 +230,14 @@ export class TimetableContainerComponent extends React.PureComponent
); - } -} + }, +); const mapStateToProps = (state: StoreState, ownProps: OwnProps) => { const semester = semesterForTimetablePage(ownProps.match.params.semester); @@ -241,11 +258,11 @@ const mapStateToProps = (state: StoreState, ownProps: OwnProps) => { // Explicitly declare top level components for React hot reloading to work. const connectedTimetableContainer = connect(mapStateToProps, { - selectSemester, - setTimetable, - fetchTimetableModules, - openNotification, - undo, + selectSemesterProp: selectSemesterAction, + setTimetableProp: setTimetableAction, + fetchTimetableModulesProp: fetchTimetableModulesAction, + openNotificationProp: openNotificationAction, + undoProp: undoAction, })(TimetableContainerComponent); export default connectedTimetableContainer; From f38c34f486c64ffc5656f9feeeb88213a0e37b0f Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Wed, 21 Oct 2020 22:08:56 +0800 Subject: [PATCH 05/20] Improve entry point component types --- website/src/views/routes/RouteRenderer.tsx | 2 +- website/src/views/routes/RoutingContext.ts | 18 +++++++++++------- .../timetable/TimetableContainer.test.tsx | 10 +++++----- website/src/views/today/TodayContainer.tsx | 3 ++- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/website/src/views/routes/RouteRenderer.tsx b/website/src/views/routes/RouteRenderer.tsx index a6fc1af466..e51aea1011 100644 --- a/website/src/views/routes/RouteRenderer.tsx +++ b/website/src/views/routes/RouteRenderer.tsx @@ -128,7 +128,7 @@ const RouterRenderer: React.FC = () => { * our ErrorBoundary/Suspense components, so we have to ensure that the suspend/error happens * in a child component. */ -const RouteComponent: React.FC> = ({ +const RouteComponent: React.FC> = ({ component, routeData, prepared, diff --git a/website/src/views/routes/RoutingContext.ts b/website/src/views/routes/RoutingContext.ts index 25efc3534d..a2e60d7d07 100644 --- a/website/src/views/routes/RoutingContext.ts +++ b/website/src/views/routes/RoutingContext.ts @@ -1,32 +1,36 @@ import type { Location, BrowserHistory, State } from 'history'; -import type { RouteConfig } from 'react-router-config'; import type { JSResource } from 'utils/JSResource'; import { createContext } from 'react'; -import { match } from 'react-router-dom'; +import type { match } from 'react-router-dom'; type PreparedProps = { [key: string]: JSResource }; +export type EntryPointComponentProps = React.PropsWithChildren<{ + match: match; + prepared: { [key: string]: unknown }; +}>; + export interface EntryPointRouteConfig { key?: React.Key; location?: Location; path?: string | string[]; exact?: boolean; strict?: boolean; - routes?: RouteConfig[]; - componentResource: JSResource; + routes?: EntryPointRouteConfig[]; + componentResource: JSResource>; prepare: (matchParams: Params) => PreparedProps; } -export type RouteComponentProps = { - component: JSResource; +export type RouteComponentProps = { + component: JSResource>; prepared: PreparedProps; routeData: match; }; export type RouteEntry = { location: Location; - entries: RouteComponentProps[]; + entries: RouteComponentProps[]; }; export type RoutingSubscriber = (nextEntry: RouteEntry) => void; diff --git a/website/src/views/timetable/TimetableContainer.test.tsx b/website/src/views/timetable/TimetableContainer.test.tsx index 42e53a6355..2a17a7c6aa 100644 --- a/website/src/views/timetable/TimetableContainer.test.tsx +++ b/website/src/views/timetable/TimetableContainer.test.tsx @@ -44,12 +44,12 @@ describe(TimetableContainerComponent, () => { timetable={timetable} colors={{}} modules={modules} - selectSemester={selectSemester} - setTimetable={setTimetable} - fetchTimetableModules={fetchTimetableModules} - openNotification={openNotification} + selectSemesterProp={selectSemester} + setTimetableProp={setTimetable} + fetchTimetableModulesProp={fetchTimetableModules} + openNotificationProp={openNotification} isValidModule={isModuleValid} - undo={undo} + undoProp={undo} {...router} />, ), diff --git a/website/src/views/today/TodayContainer.tsx b/website/src/views/today/TodayContainer.tsx index 9308538eeb..5853d462be 100644 --- a/website/src/views/today/TodayContainer.tsx +++ b/website/src/views/today/TodayContainer.tsx @@ -40,6 +40,7 @@ import MapContext from 'views/components/map/MapContext'; import { formatTime, getDayIndex } from 'utils/timify'; import { breakpointUp } from 'utils/css'; import { State as StoreState } from 'types/state'; +import type { EntryPointComponentProps } from 'views/routes/RoutingContext'; import DayEvents from './DayEvents'; import DayHeader from './DayHeader'; @@ -58,7 +59,7 @@ const semesterNameMap: Record = { 'Special Sem 2': 4, }; -export type OwnProps = TimerData; +export type OwnProps = TimerData & EntryPointComponentProps; export type Props = OwnProps & Readonly<{ From 71fea5e5786675332d008285b273e6756f0fb3ae Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Wed, 21 Oct 2020 22:29:24 +0800 Subject: [PATCH 06/20] Improve router type annotations but TS still cannot infer I miss Flow --- website/src/views/routes/RouteRenderer.tsx | 2 +- website/src/views/routes/RoutingContext.ts | 30 ++++++++++++------- website/src/views/routes/createRouter.ts | 21 +++++++------ website/src/views/routes/createRoutes.ts | 6 ++-- .../views/timetable/TimetableContainer.tsx | 7 ++--- website/src/views/today/TodayContainer.tsx | 2 +- 6 files changed, 38 insertions(+), 30 deletions(-) diff --git a/website/src/views/routes/RouteRenderer.tsx b/website/src/views/routes/RouteRenderer.tsx index e51aea1011..97b9d6f12d 100644 --- a/website/src/views/routes/RouteRenderer.tsx +++ b/website/src/views/routes/RouteRenderer.tsx @@ -128,7 +128,7 @@ const RouterRenderer: React.FC = () => { * our ErrorBoundary/Suspense components, so we have to ensure that the suspend/error happens * in a child component. */ -const RouteComponent: React.FC> = ({ +const RouteComponent: React.FC> = ({ component, routeData, prepared, diff --git a/website/src/views/routes/RoutingContext.ts b/website/src/views/routes/RoutingContext.ts index a2e60d7d07..af5613df69 100644 --- a/website/src/views/routes/RoutingContext.ts +++ b/website/src/views/routes/RoutingContext.ts @@ -4,33 +4,41 @@ import type { JSResource } from 'utils/JSResource'; import { createContext } from 'react'; import type { match } from 'react-router-dom'; -type PreparedProps = { [key: string]: JSResource }; +// type PreparedProps = { [key: string]: JSResource }; -export type EntryPointComponentProps = React.PropsWithChildren<{ - match: match; - prepared: { [key: string]: unknown }; +export type EntryPointComponentProps = React.PropsWithChildren<{ + match: match; + prepared: PreparedProps; }>; -export interface EntryPointRouteConfig { +export interface EntryPointRouteConfig< + PreparedProps, + Params, + ComponentProps = EntryPointComponentProps +> { key?: React.Key; location?: Location; path?: string | string[]; exact?: boolean; strict?: boolean; - routes?: EntryPointRouteConfig[]; - componentResource: JSResource>; - prepare: (matchParams: Params) => PreparedProps; + routes?: EntryPointRouteConfig[]; + componentResource: JSResource>; + prepare: (matchParams: Params) => PreparedProps; } -export type RouteComponentProps = { - component: JSResource>; +export type RouteComponentProps< + PreparedProps, + Params, + ComponentProps = EntryPointComponentProps +> = { + component: JSResource>; prepared: PreparedProps; routeData: match; }; export type RouteEntry = { location: Location; - entries: RouteComponentProps[]; + entries: RouteComponentProps[]; }; export type RoutingSubscriber = (nextEntry: RouteEntry) => void; diff --git a/website/src/views/routes/createRouter.ts b/website/src/views/routes/createRouter.ts index 2128aa00cf..e27763e389 100644 --- a/website/src/views/routes/createRouter.ts +++ b/website/src/views/routes/createRouter.ts @@ -8,22 +8,21 @@ import { RoutingSubscriber, } from './RoutingContext'; -interface MatchedEntryPointRoute - extends MatchedRoute { - route: EntryPointRouteConfig; +interface MatchedEntryPointRoute extends MatchedRoute { + route: EntryPointRouteConfig; } /** * Match the current location to the corresponding route entry. */ -function matchEntryPointRoutes( - routes: EntryPointRouteConfig[], +function matchEntryPointRoutes( + routes: EntryPointRouteConfig[], locationPathname: string, -): MatchedEntryPointRoute[] { +): MatchedEntryPointRoute[] { const matchedRoutes = (matchRoutes( routes, locationPathname, - ) as unknown) as MatchedEntryPointRoute[]; + ) as unknown) as MatchedEntryPointRoute[]; if (!Array.isArray(matchedRoutes) || matchedRoutes.length === 0) { throw new Error(`No route for ${locationPathname}`); } @@ -33,9 +32,9 @@ function matchEntryPointRoutes( /** * Load the data for the matched route, given the params extracted from the route */ -function prepareMatches( - matches: MatchedEntryPointRoute[], -): RouteComponentProps[] { +function prepareMatches( + matches: MatchedEntryPointRoute[], +): RouteComponentProps[] { return matches.map((match) => { const { route, match: matchData } = match; const prepared = route.prepare(matchData.params); @@ -54,7 +53,7 @@ function prepareMatches( * location to the corresponding route entry, and then preloads the code and data for the route. */ export default function createRouter( - routes: EntryPointRouteConfig[], + routes: EntryPointRouteConfig[], options: BrowserHistoryOptions | undefined, ) { // Initialize history diff --git a/website/src/views/routes/createRoutes.ts b/website/src/views/routes/createRoutes.ts index ba2f21d2eb..6745a4e98b 100644 --- a/website/src/views/routes/createRoutes.ts +++ b/website/src/views/routes/createRoutes.ts @@ -6,7 +6,7 @@ import venueLocationResource from 'views/components/map/venueLocationResource'; import { EntryPointRouteConfig } from './RoutingContext'; // TODO: Define entry points closer to modules -export default function createRoutes(dispatch: Dispatch): EntryPointRouteConfig[] { +export default function createRoutes(dispatch: Dispatch): EntryPointRouteConfig[] { return [ { componentResource: JSResource( @@ -39,7 +39,9 @@ export default function createRoutes(dispatch: Dispatch): EntryPointRouteConfig[ /* webpackChunkName: "TimetableContainer.route" */ 'views/timetable/TimetableContainer' ), ), - prepare: ({ semester, action }) => ({}), + prepare: () => { + /* noop */ + }, }, { path: '/today', diff --git a/website/src/views/timetable/TimetableContainer.tsx b/website/src/views/timetable/TimetableContainer.tsx index 83eac7ba0e..d831c1474e 100644 --- a/website/src/views/timetable/TimetableContainer.tsx +++ b/website/src/views/timetable/TimetableContainer.tsx @@ -1,6 +1,6 @@ import React, { memo, useCallback, useEffect, useMemo, useState } from 'react'; import { connect } from 'react-redux'; -import { match as Match, Redirect } from 'react-router-dom'; +import { Redirect } from 'react-router-dom'; import classnames from 'classnames'; import { ModuleCode, Semester } from 'types/modules'; @@ -25,6 +25,7 @@ import SemesterSwitcher from 'views/components/semester-switcher/SemesterSwitche import LoadingSpinner from 'views/components/LoadingSpinner'; import ScrollToTop from 'views/components/ScrollToTop'; import { State as StoreState } from 'types/state'; +import { EntryPointComponentProps } from 'views/routes/RoutingContext'; import TimetableContent from './TimetableContent'; import styles from './TimetableContainer.scss'; @@ -34,9 +35,7 @@ export type QueryParam = { semester: string; }; -type OwnProps = { - match: Match; -}; +type OwnProps = EntryPointComponentProps; type Props = OwnProps & { modules: ModulesMap; diff --git a/website/src/views/today/TodayContainer.tsx b/website/src/views/today/TodayContainer.tsx index 5853d462be..919b7f00db 100644 --- a/website/src/views/today/TodayContainer.tsx +++ b/website/src/views/today/TodayContainer.tsx @@ -59,7 +59,7 @@ const semesterNameMap: Record = { 'Special Sem 2': 4, }; -export type OwnProps = TimerData & EntryPointComponentProps; +export type OwnProps = TimerData & EntryPointComponentProps<{}, {}>; export type Props = OwnProps & Readonly<{ From 20a28899b8bb73e27e2ce78879463e4a795a2790 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Thu, 22 Oct 2020 08:49:50 +0800 Subject: [PATCH 07/20] Disable Sentry on this branch --- website/src/bootstrapping/sentry.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/website/src/bootstrapping/sentry.ts b/website/src/bootstrapping/sentry.ts index 93304f8c6d..4aae9671bd 100644 --- a/website/src/bootstrapping/sentry.ts +++ b/website/src/bootstrapping/sentry.ts @@ -4,7 +4,8 @@ import * as Sentry from '@sentry/browser'; import { isBrowserSupported } from './browser'; // Configure Raven - the client for Sentry, which we use to handle errors -const loadRaven = process.env.NODE_ENV === 'production'; +// const loadRaven = process.env.NODE_ENV === 'production'; +const loadRaven = false; // TODO: Reenable before merging if (loadRaven) { Sentry.init({ dsn: 'https://4b4fe71954424fd39ac88a4f889ffe20@sentry.io/213986', From 5a99e30b9f9c9454a16284dd29ece5bcd7c4e35b Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Thu, 22 Oct 2020 08:50:11 +0800 Subject: [PATCH 08/20] Fix TS build errors in TodayContainer --- website/src/views/today/TodayContainer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/src/views/today/TodayContainer.tsx b/website/src/views/today/TodayContainer.tsx index 919b7f00db..5528db369c 100644 --- a/website/src/views/today/TodayContainer.tsx +++ b/website/src/views/today/TodayContainer.tsx @@ -59,7 +59,7 @@ const semesterNameMap: Record = { 'Special Sem 2': 4, }; -export type OwnProps = TimerData & EntryPointComponentProps<{}, {}>; +export type OwnProps = TimerData & EntryPointComponentProps; export type Props = OwnProps & Readonly<{ From 2600a3c141238f45842a64ec2c7494b7612a7631 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Thu, 22 Oct 2020 18:39:32 +0800 Subject: [PATCH 09/20] Replace custom router with React Router v6/experimental --- website/package.json | 5 +- website/src/entry/App.tsx | 26 +- website/src/entry/main.tsx | 2 +- website/src/utils/JSResource.ts | 3 + website/src/views/AppShell.entrypoint.ts | 32 ++ website/src/views/AppShell.tsx | 25 +- .../views/components/LinkModuleCodes.test.tsx | 8 +- .../src/views/components/LinkModuleCodes.tsx | 6 +- .../notfications/HacktoberfestBanner.tsx | 6 +- .../ContributeContainer.tsx | 14 +- .../views/errors/ModuleNotFoundPage.test.tsx | 4 +- .../src/views/errors/ModuleNotFoundPage.tsx | 14 +- website/src/views/errors/NotFoundPage.tsx | 2 +- website/src/views/layout/Footer.tsx | 24 +- website/src/views/layout/Navtabs.tsx | 35 +- .../modules/ModuleArchiveContainer.test.tsx | 4 +- .../views/modules/ModuleArchiveContainer.tsx | 4 +- .../src/views/modules/ModuleFinderItem.tsx | 6 +- .../modules/ModulePageContainer.test.tsx | 4 +- .../src/views/modules/ModulePageContainer.tsx | 6 +- website/src/views/planner/PlannerModule.tsx | 8 +- .../src/views/routes/EntryPointContainer.tsx | 22 + website/src/views/routes/Link.tsx | 68 ---- website/src/views/routes/NavLink.tsx | 65 --- website/src/views/routes/PreloadingLink.tsx | 24 ++ .../views/routes/RoutePreloaderContext.tsx | 76 ++++ website/src/views/routes/RouteRenderer.tsx | 145 ------- website/src/views/routes/Routes.tsx | 188 ++++++--- website/src/views/routes/RoutingContext.ts | 59 --- website/src/views/routes/createRouter.ts | 120 ------ website/src/views/routes/createRoutes.ts | 79 ---- website/src/views/routes/hooks.ts | 52 --- website/src/views/routes/locationUtils.ts | 111 ----- website/src/views/routes/types.ts | 23 ++ website/src/views/static/FaqContainer.tsx | 5 +- .../src/views/timetable/ExamCalendar.test.tsx | 8 +- website/src/views/timetable/ExamWeek.tsx | 6 +- website/src/views/timetable/ExportMenu.tsx | 6 +- .../views/timetable/Timetable.entrypoint.ts | 14 + .../timetable/TimetableContainer.test.tsx | 4 +- .../views/timetable/TimetableContainer.tsx | 381 ++++++++---------- .../views/timetable/TimetableModulesTable.tsx | 6 +- .../timetable/TimetableRootRedirector.tsx | 18 + website/src/views/today/BeforeLessonCard.tsx | 4 +- website/src/views/today/Today.entrypoint.ts | 17 + website/src/views/today/TodayContainer.tsx | 4 +- website/src/views/venues/VenueDetails.tsx | 10 +- website/src/views/venues/VenueList.tsx | 7 +- website/yarn.lock | 139 +------ 49 files changed, 677 insertions(+), 1222 deletions(-) create mode 100644 website/src/views/AppShell.entrypoint.ts create mode 100644 website/src/views/routes/EntryPointContainer.tsx delete mode 100644 website/src/views/routes/Link.tsx delete mode 100644 website/src/views/routes/NavLink.tsx create mode 100644 website/src/views/routes/PreloadingLink.tsx create mode 100644 website/src/views/routes/RoutePreloaderContext.tsx delete mode 100644 website/src/views/routes/RouteRenderer.tsx delete mode 100644 website/src/views/routes/RoutingContext.ts delete mode 100644 website/src/views/routes/createRouter.ts delete mode 100644 website/src/views/routes/createRoutes.ts delete mode 100644 website/src/views/routes/hooks.ts delete mode 100644 website/src/views/routes/locationUtils.ts create mode 100644 website/src/views/routes/types.ts create mode 100644 website/src/views/timetable/Timetable.entrypoint.ts create mode 100644 website/src/views/timetable/TimetableRootRedirector.tsx create mode 100644 website/src/views/today/Today.entrypoint.ts diff --git a/website/package.json b/website/package.json index ab36f918bd..487c6dcdc3 100644 --- a/website/package.json +++ b/website/package.json @@ -56,8 +56,6 @@ "@types/react-loadable": "5.5.3", "@types/react-modal": "3.10.6", "@types/react-redux": "7.1.9", - "@types/react-router-config": "^5.0.1", - "@types/react-router-dom": "5.1.6", "@types/react-scrollspy": "3.3.3", "@types/redux-mock-store": "1.0.2", "@types/use-subscription": "^1.0.0", @@ -167,8 +165,7 @@ "react-modal": "3.11.2", "react-qr-svg": "2.2.2", "react-redux": "7.2.1", - "react-router-config": "^5.1.1", - "react-router-dom": "5.2.0", + "react-router-dom": "^0.0.0-experimental-ffd8c7d0", "react-scrollspy": "3.4.3", "redux": "4.0.5", "redux-persist": "6.0.0", diff --git a/website/src/entry/App.tsx b/website/src/entry/App.tsx index 636446bf8d..2ba3c50d66 100644 --- a/website/src/entry/App.tsx +++ b/website/src/entry/App.tsx @@ -2,33 +2,26 @@ import { Store } from 'redux'; import { State } from 'types/state'; import { Persistor } from 'storage/persistReducer'; -import React, { useCallback, useMemo } from 'react'; +import React, { Suspense, useCallback, useMemo } from 'react'; import { hot } from 'react-hot-loader/root'; -// import { BrowserRouter as Router } from 'react-router-dom'; +import { BrowserRouter as Router } from 'react-router-dom'; import { Provider } from 'react-redux'; import { PersistGate } from 'redux-persist/integration/react'; // import AppShell from 'views/AppShell'; -// import Routes from 'views/routes/Routes'; -import createRoutes from 'views/routes/createRoutes'; +import Routes from 'views/routes/Routes'; import { DIMENSIONS, setCustomDimensions } from 'bootstrapping/matomo'; import ErrorBoundary from 'views/errors/ErrorBoundary'; import ErrorPage from 'views/errors/ErrorPage'; -import createRouter from 'views/routes/createRouter'; -import RoutingContext from 'views/routes/RoutingContext'; -import RouterRenderer from 'views/routes/RouteRenderer'; +import LoadingSpinner from 'views/components/LoadingSpinner'; +import ApiError from 'views/errors/ApiError'; type Props = { store: Store; persistor: Persistor; }; -// const router = createRouter(createRoutes(store.dispatch)); - const App: React.FC = ({ store, persistor }) => { - // Uses the custom router setup to define a router instanace that we can pass through context - const router = useMemo(() => createRouter(createRoutes(store.dispatch)), [store.dispatch]); - const onBeforeLift = useCallback(() => { const { theme, settings } = store.getState(); @@ -47,10 +40,11 @@ const App: React.FC = ({ store, persistor }) => { }> - - {/* Render the active route */} - - + + }> + + + diff --git a/website/src/entry/main.tsx b/website/src/entry/main.tsx index 43410d7b50..2e4650ef43 100644 --- a/website/src/entry/main.tsx +++ b/website/src/entry/main.tsx @@ -9,7 +9,7 @@ import 'core-js/es/promise/finally'; import React from 'react'; // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore -import { unstable_createRoot as createRoot } from 'react-dom'; +import { unstable_createRoot as createRoot } from 'react-dom/profiling'; import ReactModal from 'react-modal'; import configureStore from 'bootstrapping/configure-store'; diff --git a/website/src/utils/JSResource.ts b/website/src/utils/JSResource.ts index f5cc9d5130..eb335c60d9 100644 --- a/website/src/utils/JSResource.ts +++ b/website/src/utils/JSResource.ts @@ -94,6 +94,9 @@ class Resource { if (this.error !== null) { throw this.error; } + if (this.promise === null) { + throw new Error('preload() must be called before read().'); + } throw this.promise; } diff --git a/website/src/views/AppShell.entrypoint.ts b/website/src/views/AppShell.entrypoint.ts new file mode 100644 index 0000000000..6183e9c0ed --- /dev/null +++ b/website/src/views/AppShell.entrypoint.ts @@ -0,0 +1,32 @@ +import { captureException } from '@sentry/browser'; +import { fetchModuleList } from 'actions/moduleBank'; +import { JSResource } from 'utils/JSResource'; +import { EntryPoint } from 'views/routes/EntryPointContainer'; + +export type PreparedProps = { + moduleList: JSResource; +}; + +const entryPoint: EntryPoint = { + component: JSResource( + 'AppShell', + () => import(/* webpackChunkName: "AppShell.route" */ 'views/AppShell'), + ), + prepare(_params, dispatch) { + const moduleList = JSResource('moduleList', async () => { + try { + // Typed as unknown because we don't actually need the output + return (dispatch(fetchModuleList()) as unknown) as Promise; + } catch (error) { + captureException(error); + throw error; + } + }); + moduleList.preload(); + return { + moduleList, + }; + }, +}; + +export default entryPoint; diff --git a/website/src/views/AppShell.tsx b/website/src/views/AppShell.tsx index 5d1990c5e4..16cfc8bb14 100644 --- a/website/src/views/AppShell.tsx +++ b/website/src/views/AppShell.tsx @@ -8,6 +8,7 @@ import { Helmet } from 'react-helmet'; import { connect } from 'react-redux'; import classnames from 'classnames'; import { each } from 'lodash'; +import { useLocation } from 'react-router-dom'; import weekText from 'utils/weekText'; import { captureException } from 'utils/error'; @@ -21,6 +22,7 @@ import Navtabs from 'views/layout/Navtabs'; import GlobalSearchContainer from 'views/layout/GlobalSearchContainer'; import Notification from 'views/components/notfications/Notification'; import ErrorBoundary from 'views/errors/ErrorBoundary'; +import { PreloadingNavLink } from 'views/routes/PreloadingLink'; import ErrorPage from 'views/errors/ErrorPage'; import ApiError from 'views/errors/ApiError'; import { trackPageView } from 'bootstrapping/matomo'; @@ -31,9 +33,6 @@ import type { JSResource } from 'utils/JSResource'; import LoadingSpinner from './components/LoadingSpinner'; import FeedbackModal from './components/FeedbackModal'; import KeyboardShortcuts from './components/KeyboardShortcuts'; -import NavLink from './routes/NavLink'; -import RoutingContext from './routes/RoutingContext'; -import { useLocation } from './routes/hooks'; import styles from './AppShell.scss'; @@ -74,14 +73,14 @@ export const AppShellComponent: React.FC = ({ const location = useLocation(); // Enable Matomo analytics - const router = useContext(RoutingContext); - useEffect(() => { - if (router) { - // Unsubscribe when router changes or on unmount - return trackPageView(router.history); - } - return undefined; - }, [router]); + // const router = useContext(RoutingContext); + // useEffect(() => { + // if (router) { + // // Unsubscribe when router changes or on unmount + // return trackPageView(router.history); + // } + // return undefined; + // }, [router]); const fetchTimetableModules = useCallback( (timetable: SemTimetableConfig, semester: Semester) => { @@ -134,9 +133,9 @@ export const AppShellComponent: React.FC = ({
diff --git a/website/src/views/layout/Navtabs.tsx b/website/src/views/layout/Navtabs.tsx index b4a5685017..11a9436d7c 100644 --- a/website/src/views/layout/Navtabs.tsx +++ b/website/src/views/layout/Navtabs.tsx @@ -8,7 +8,7 @@ import ExternalLink from 'views/components/ExternalLink'; import { timetablePage } from 'views/routes/paths'; import { preload as preloadVenues } from 'views/venues/VenuesContainer'; import { preload as preloadContribute } from 'views/contribute/ContributeContainer'; -import NavLink from 'views/routes/NavLink'; +import { PreloadingNavLink } from 'views/routes/PreloadingLink'; import { State } from 'types/state'; import styles from './Navtabs.scss'; @@ -27,40 +27,45 @@ const tabProps = { export const NavtabsComponent = memo(({ activeSemester, beta }) => (
- + Find out more - + diff --git a/website/src/views/timetable/Timetable.entrypoint.ts b/website/src/views/timetable/Timetable.entrypoint.ts new file mode 100644 index 0000000000..3fd4568b4d --- /dev/null +++ b/website/src/views/timetable/Timetable.entrypoint.ts @@ -0,0 +1,14 @@ +import { JSResource } from 'utils/JSResource'; +import { EntryPoint } from 'views/routes/EntryPointContainer'; + +const entryPoint: EntryPoint = { + component: JSResource( + 'Timetable', + () => import(/* webpackChunkName: "Timetable.route" */ 'views/timetable/TimetableContainer'), + ), + prepare() { + return {}; + }, +}; + +export default entryPoint; diff --git a/website/src/views/timetable/TimetableContainer.test.tsx b/website/src/views/timetable/TimetableContainer.test.tsx index 2a17a7c6aa..045bafb994 100644 --- a/website/src/views/timetable/TimetableContainer.test.tsx +++ b/website/src/views/timetable/TimetableContainer.test.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { Redirect } from 'react-router-dom'; +import { Navigate } from 'react-router-dom'; import { shallow, ShallowWrapper } from 'enzyme'; import { SemTimetableConfig } from 'types/timetables'; import { ModulesMap } from 'types/reducers'; @@ -67,7 +67,7 @@ describe(TimetableContainerComponent, () => { } function expectRedirect(wrapper: ShallowWrapper, to = timetablePage(1)) { - expect(wrapper.type()).toEqual(Redirect); + expect(wrapper.type()).toEqual(Navigate); expect(wrapper.prop('to')).toEqual(to); } diff --git a/website/src/views/timetable/TimetableContainer.tsx b/website/src/views/timetable/TimetableContainer.tsx index d831c1474e..a834e9bceb 100644 --- a/website/src/views/timetable/TimetableContainer.tsx +++ b/website/src/views/timetable/TimetableContainer.tsx @@ -1,11 +1,10 @@ import React, { memo, useCallback, useEffect, useMemo, useState } from 'react'; -import { connect } from 'react-redux'; -import { Redirect } from 'react-router-dom'; +import { useDispatch, useSelector } from 'react-redux'; +import { Navigate, useParams, useNavigate, useLocation } from 'react-router-dom'; import classnames from 'classnames'; import { ModuleCode, Semester } from 'types/modules'; import { SemTimetableConfig } from 'types/timetables'; -import { ColorMapping, ModulesMap, NotificationOptions } from 'types/reducers'; import { selectSemester as selectSemesterAction } from 'actions/settings'; import { getSemesterTimetable } from 'selectors/timetables'; @@ -19,13 +18,12 @@ import { getModuleCondensed } from 'selectors/moduleBank'; import { deserializeTimetable } from 'utils/timetables'; import { fillColorMapping } from 'utils/colors'; import { semesterForTimetablePage, TIMETABLE_SHARE, timetablePage } from 'views/routes/paths'; -import { useHistory } from 'views/routes/hooks'; import { Repeat } from 'react-feather'; import SemesterSwitcher from 'views/components/semester-switcher/SemesterSwitcher'; import LoadingSpinner from 'views/components/LoadingSpinner'; import ScrollToTop from 'views/components/ScrollToTop'; -import { State as StoreState } from 'types/state'; -import { EntryPointComponentProps } from 'views/routes/RoutingContext'; +import { State } from 'types/state'; +import type { EntryPointComponentProps } from 'views/routes/EntryPointContainer'; import TimetableContent from './TimetableContent'; import styles from './TimetableContainer.scss'; @@ -35,26 +33,7 @@ export type QueryParam = { semester: string; }; -type OwnProps = EntryPointComponentProps; - -type Props = OwnProps & { - modules: ModulesMap; - semester: Semester | null; - activeSemester: Semester; - timetable: SemTimetableConfig; - colors: ColorMapping; - - isValidModule: (moduleCode: ModuleCode) => boolean; - selectSemesterProp: (semester: Semester) => void; - setTimetableProp: ( - semester: Semester, - semTimetableConfig: SemTimetableConfig, - colorMapping: ColorMapping, - ) => void; - fetchTimetableModulesProp: (semTimetableConfig: SemTimetableConfig[]) => void; - openNotificationProp: (str: string, notificationOptions: NotificationOptions) => void; - undoProp: () => void; -}; +type Props = EntryPointComponentProps; /** * Manages semester switching and sync/shared timetables @@ -62,206 +41,180 @@ type Props = OwnProps & { * - Import timetable data from query string if action is defined * - Create the UI for the user to confirm their actions */ -export const TimetableContainerComponent = memo( - ({ - match, - - modules, - semester, - activeSemester, - timetable, - colors, - - isValidModule, - selectSemesterProp, - setTimetableProp, - fetchTimetableModulesProp, - openNotificationProp, - undoProp, - }) => { - const history = useHistory(); - const { location } = history; - const { action } = match.params; - - const [importedTimetable, setImportedTimetable] = useState(() => - semester && action ? deserializeTimetable(location.search) : null, - ); - - useEffect(() => { - // TODO: Preload this - if (importedTimetable) { - fetchTimetableModulesProp([importedTimetable]); - } - }, [fetchTimetableModulesProp, importedTimetable]); - - const selectSemester = useCallback( - (selectedSemester: Semester) => { - selectSemesterProp(selectedSemester); - - history.push({ - ...location, - pathname: timetablePage(selectedSemester), - }); - }, - [history, location, selectSemesterProp], - ); - - const isLoading = useMemo(() => { - // Check that all modules are fully loaded into the ModuleBank - const moduleCodes = new Set(Object.keys(timetable)); - if (importedTimetable) { - Object.keys(importedTimetable) - .filter(isValidModule) - .forEach((moduleCode) => moduleCodes.add(moduleCode)); - } +export const TimetableContainerComponent: React.FC = () => { + const navigate = useNavigate(); + const location = useLocation(); + const params = useParams(); + const action = params['*']; + + const semester = useMemo(() => semesterForTimetablePage(params.semester), [params.semester]); + + const { timetable, colors } = useSelector((state: State) => + semester ? getSemesterTimetable(semester, state.timetables) : { timetable: {}, colors: {} }, + ); + const getModule = useSelector((state: State) => getModuleCondensed(state.moduleBank)); + const isValidModule = useCallback((moduleCode: ModuleCode) => !!getModule(moduleCode), [ + getModule, + ]); + const modules = useSelector((state: State) => state.moduleBank.modules); + const activeSemester = useSelector((state: State) => state.app.activeSemester); + + const dispatch = useDispatch(); + + const [importedTimetable, setImportedTimetable] = useState(() => + semester && action ? deserializeTimetable(location.search) : null, + ); + + useEffect(() => { + // TODO: Preload this + if (importedTimetable) { + dispatch(fetchTimetableModulesAction([importedTimetable])); + } + }, [dispatch, importedTimetable]); + + const selectSemester = useCallback( + (selectedSemester: Semester) => { + dispatch(selectSemesterAction(selectedSemester)); + + navigate({ + ...location, + pathname: timetablePage(selectedSemester), + }); + }, + [dispatch, location, navigate], + ); + + const isLoading = useMemo(() => { + // Check that all modules are fully loaded into the ModuleBank + const moduleCodes = new Set(Object.keys(timetable)); + if (importedTimetable) { + Object.keys(importedTimetable) + .filter(isValidModule) + .forEach((moduleCode) => moduleCodes.add(moduleCode)); + } - // TODO: Account for loading error - return Array.from(moduleCodes).some((moduleCode) => !modules[moduleCode]); - }, [importedTimetable, isValidModule, modules, timetable]); + // TODO: Account for loading error + return Array.from(moduleCodes).some((moduleCode) => !modules[moduleCode]); + }, [importedTimetable, isValidModule, modules, timetable]); - const clearImportedTimetable = useCallback(() => { - if (semester) { - setImportedTimetable(null); - history.push(timetablePage(semester)); - } - }, [history, semester]); + const clearImportedTimetable = useCallback(() => { + if (semester) { + setImportedTimetable(null); + navigate(timetablePage(semester)); + } + }, [navigate, semester]); - const importTimetable = useCallback( - (guaranteedSemester: Semester, sharedTimetable: SemTimetableConfig) => { - const filledColors = fillColorMapping(sharedTimetable, colors); - setTimetableProp(guaranteedSemester, sharedTimetable, filledColors); - clearImportedTimetable(); + const importTimetable = useCallback( + (guaranteedSemester: Semester, sharedTimetable: SemTimetableConfig) => { + const filledColors = fillColorMapping(sharedTimetable, colors); + dispatch(setTimetableAction(guaranteedSemester, sharedTimetable, filledColors)); + clearImportedTimetable(); - openNotificationProp('Timetable imported', { + dispatch( + openNotificationAction('Timetable imported', { timeout: 12000, overwritable: true, action: { text: 'Undo', - handler: undoProp, + handler: () => !!dispatch(undoAction), }, - }); - }, - [clearImportedTimetable, colors, openNotificationProp, setTimetableProp, undoProp], - ); - - const renderSharingHeader = useCallback( - (guaranteedSemester: Semester, sharedTimetable: SemTimetableConfig) => { - return ( -
- - -
-
-

This timetable was shared with you

-

- Clicking import will replace your saved timetable with the one - below. -

-
+ }), + ); + }, + [clearImportedTimetable, colors, dispatch], + ); + + const renderSharingHeader = useCallback( + (guaranteedSemester: Semester, sharedTimetable: SemTimetableConfig) => { + return ( +
+ + +
+
+

This timetable was shared with you

+

+ Clicking import will replace your saved timetable with the one + below. +

+
-
- - -
+
+ +
- ); - }, - [clearImportedTimetable, importTimetable], - ); - - const renderTimetableHeader = useCallback( - (guaranteedSemester: Semester, readOnly?: boolean) => { - return ( - - ); - }, - [selectSemester], - ); - - // 1. If the URL doesn't look correct, we'll direct the user to the home page - if (semester == null || (action && action !== TIMETABLE_SHARE)) { - return ; - } - - // 2. If we are importing a timetable, check that all imported modules are - // loaded first, and display a spinner if they're not. - if (isLoading) { - return ; - } - - // 3. Construct the color map - const displayedTimetable = importedTimetable || timetable; - const filledColors = fillColorMapping(displayedTimetable, colors); - - // 4. If there is an imported timetable, we show the sharing header which - // asks the user if they want to import the shared timetable - const header = importedTimetable ? ( - <> - {renderSharingHeader(semester, importedTimetable)} - {renderTimetableHeader(semester, true)} - - ) : ( - renderTimetableHeader(semester) - ); - - return ( -
- - - + ); + }, + [clearImportedTimetable, importTimetable], + ); + + const renderTimetableHeader = useCallback( + (guaranteedSemester: Semester, readOnly?: boolean) => { + return ( + -
- ); - }, -); - -const mapStateToProps = (state: StoreState, ownProps: OwnProps) => { - const semester = semesterForTimetablePage(ownProps.match.params.semester); - const { timetable, colors } = semester - ? getSemesterTimetable(semester, state.timetables) - : { timetable: {}, colors: {} }; - const getModule = getModuleCondensed(state.moduleBank); - - return { - semester, - timetable, - colors, - isValidModule: (moduleCode: ModuleCode) => !!getModule(moduleCode), - modules: state.moduleBank.modules, - activeSemester: state.app.activeSemester, - }; + ); + }, + [selectSemester], + ); + + // 1. If the URL doesn't look correct, we'll direct the user to the home page + if (semester == null || (action && action !== TIMETABLE_SHARE)) { + return ; + } + + // 2. If we are importing a timetable, check that all imported modules are + // loaded first, and display a spinner if they're not. + if (isLoading) { + return ; + } + + // 3. Construct the color map + const displayedTimetable = importedTimetable || timetable; + const filledColors = fillColorMapping(displayedTimetable, colors); + + // 4. If there is an imported timetable, we show the sharing header which + // asks the user if they want to import the shared timetable + const header = importedTimetable ? ( + <> + {renderSharingHeader(semester, importedTimetable)} + {renderTimetableHeader(semester, true)} + + ) : ( + renderTimetableHeader(semester) + ); + + return ( +
+ + + +
+ ); }; -// Explicitly declare top level components for React hot reloading to work. -const connectedTimetableContainer = connect(mapStateToProps, { - selectSemesterProp: selectSemesterAction, - setTimetableProp: setTimetableAction, - fetchTimetableModulesProp: fetchTimetableModulesAction, - openNotificationProp: openNotificationAction, - undoProp: undoAction, -})(TimetableContainerComponent); - -export default connectedTimetableContainer; +export default memo(TimetableContainerComponent); diff --git a/website/src/views/timetable/TimetableModulesTable.tsx b/website/src/views/timetable/TimetableModulesTable.tsx index af6ad2d76d..df5ff9f186 100644 --- a/website/src/views/timetable/TimetableModulesTable.tsx +++ b/website/src/views/timetable/TimetableModulesTable.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { connect } from 'react-redux'; -import Link from 'views/routes/Link'; +import { PreloadingLink } from 'views/routes/PreloadingLink'; import classnames from 'classnames'; import { sortBy } from 'lodash'; import produce from 'immer'; @@ -121,9 +121,9 @@ export const TimetableModulesTableComponent: React.FC = (props) => {
{!readOnly && renderModuleActions(module)} - + {module.moduleCode} {module.title} - +
{intersperse(secondRowText, BULLET_NBSP)}
diff --git a/website/src/views/timetable/TimetableRootRedirector.tsx b/website/src/views/timetable/TimetableRootRedirector.tsx new file mode 100644 index 0000000000..b741e904a0 --- /dev/null +++ b/website/src/views/timetable/TimetableRootRedirector.tsx @@ -0,0 +1,18 @@ +import React, { useEffect } from 'react'; +import { useSelector } from 'react-redux'; +import { useNavigate } from 'react-router-dom'; +import type { State } from 'types/state'; +import { timetablePage } from 'views/routes/paths'; + +const TimetableRootRedirector: React.FC = () => { + const navigate = useNavigate(); + const activeSemester = useSelector((state: State) => state.app.activeSemester); + + useEffect(() => { + navigate(timetablePage(activeSemester)); + }, [activeSemester, navigate]); + + return null; +}; + +export default TimetableRootRedirector; diff --git a/website/src/views/today/BeforeLessonCard.tsx b/website/src/views/today/BeforeLessonCard.tsx index 94a3b82f6c..f04920e69b 100644 --- a/website/src/views/today/BeforeLessonCard.tsx +++ b/website/src/views/today/BeforeLessonCard.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import classnames from 'classnames'; -import Link from 'views/routes/Link'; +import { PreloadingLink } from 'views/routes/PreloadingLink'; import { Lesson } from 'types/timetables'; import { getStartTimeAsDate } from 'utils/timetables'; @@ -19,7 +19,7 @@ type Props = { const freeRoomMessage = ( <> Need help finding a free classroom to study in? Check out our{' '} - free room finder. + free room finder. ); diff --git a/website/src/views/today/Today.entrypoint.ts b/website/src/views/today/Today.entrypoint.ts new file mode 100644 index 0000000000..76ebbf8f6e --- /dev/null +++ b/website/src/views/today/Today.entrypoint.ts @@ -0,0 +1,17 @@ +import { JSResource } from 'utils/JSResource'; +import venueLocationResource from 'views/components/map/venueLocationResource'; +import { EntryPoint } from 'views/routes/EntryPointContainer'; + +export type PreparedProps = unknown; + +const entryPoint: EntryPoint = { + component: JSResource( + 'Today', + () => import(/* webpackChunkName: "Today.route" */ './TodayContainer'), + ), + prepare() { + venueLocationResource.preloadOrReloadIfError(); + }, +}; + +export default entryPoint; diff --git a/website/src/views/today/TodayContainer.tsx b/website/src/views/today/TodayContainer.tsx index 5528db369c..4a8734dbaf 100644 --- a/website/src/views/today/TodayContainer.tsx +++ b/website/src/views/today/TodayContainer.tsx @@ -40,7 +40,7 @@ import MapContext from 'views/components/map/MapContext'; import { formatTime, getDayIndex } from 'utils/timify'; import { breakpointUp } from 'utils/css'; import { State as StoreState } from 'types/state'; -import type { EntryPointComponentProps } from 'views/routes/RoutingContext'; +import type { EntryPointComponentProps } from 'views/routes/EntryPointContainer'; import DayEvents from './DayEvents'; import DayHeader from './DayHeader'; @@ -59,7 +59,7 @@ const semesterNameMap: Record = { 'Special Sem 2': 4, }; -export type OwnProps = TimerData & EntryPointComponentProps; +export type OwnProps = TimerData & EntryPointComponentProps; export type Props = OwnProps & Readonly<{ diff --git a/website/src/views/venues/VenueDetails.tsx b/website/src/views/venues/VenueDetails.tsx index edfaebcccb..c07ab4e539 100644 --- a/website/src/views/venues/VenueDetails.tsx +++ b/website/src/views/venues/VenueDetails.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import { RouteComponentProps } from 'react-router-dom'; import classnames from 'classnames'; import { flatMap } from 'lodash'; -import Link from 'views/routes/Link'; +import { PreloadingLink } from 'views/routes/PreloadingLink'; import { DayAvailability, TimePeriod, Venue } from 'types/venues'; import { Lesson } from 'types/timetables'; @@ -51,7 +51,7 @@ export const VenueDetailsComponent: React.FC = (props) => { {`${venue} - Venues`}
- = (props) => { }} > {previous} - +

{venue}

- = (props) => { }} > {next} - +
diff --git a/website/src/views/venues/VenueList.tsx b/website/src/views/venues/VenueList.tsx index f0fa6b53d8..f294c711df 100644 --- a/website/src/views/venues/VenueList.tsx +++ b/website/src/views/venues/VenueList.tsx @@ -1,7 +1,8 @@ import * as React from 'react'; import classnames from 'classnames'; import { groupBy, toPairs, sortBy } from 'lodash'; -import Link, { LinkProps } from 'views/routes/Link'; +import type { LinkProps } from 'react-router-dom'; +import { PreloadingLink } from 'views/routes/PreloadingLink'; import { Venue } from 'types/venues'; import { venuePage } from 'views/routes/paths'; @@ -29,7 +30,7 @@ const VenueList: React.FC = (props) => {
    {venues.map((venue) => (
  • - = (props) => { {...props.linkProps} > {venue} - +
    • ))}
diff --git a/website/yarn.lock b/website/yarn.lock index d77114cf13..92cb6b05d6 100644 --- a/website/yarn.lock +++ b/website/yarn.lock @@ -2030,13 +2030,6 @@ core-js-pure "^3.0.0" regenerator-runtime "^0.13.4" -"@babel/runtime@^7.1.2": - version "7.3.4" - resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.3.4.tgz#73d12ba819e365fcf7fd152aed56d6df97d21c83" - integrity sha512-IvfvnMdSaLBateu0jfsYIpZTxAc2cKEXEMiezGGN75QcBcecDUKd3PgLAncT0oOgxKy8dd8hrJKj9MfzgfZd6g== - dependencies: - regenerator-runtime "^0.12.0" - "@babel/runtime@^7.10.2": version "7.10.4" resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.10.4.tgz#a6724f1a6b8d2f6ea5236dbfe58c7d7ea9c5eb99" @@ -2997,7 +2990,7 @@ dependencies: "@types/node" "*" -"@types/history@*", "@types/history@^4.6.0": +"@types/history@^4.6.0": version "4.7.2" resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.2.tgz#0e670ea254d559241b6eeb3894f8754991e73220" integrity sha512-ui3WwXmjTaY73fOQ3/m3nnajU/Orhi6cEu5rzX+BrAAJxa3eITXZ5ch9suPqtM03OWhAHhPSyBGCN4UKoxO20Q== @@ -3225,32 +3218,6 @@ hoist-non-react-statics "^3.3.0" redux "^4.0.0" -"@types/react-router-config@^5.0.1": - version "5.0.1" - resolved "https://registry.yarnpkg.com/@types/react-router-config/-/react-router-config-5.0.1.tgz#54da8418190ee47484dee279975e2b8038fb8b5d" - integrity sha512-D4srFL4XP21GjWWnM7mL8j+Nrrw13pc2TYr57WTHJxU9YTxnrXL7p1iqGtwecgwhyeXJSm4WrGwq0SOgSALVbA== - dependencies: - "@types/history" "*" - "@types/react" "*" - "@types/react-router" "*" - -"@types/react-router-dom@5.1.6": - version "5.1.6" - resolved "https://registry.yarnpkg.com/@types/react-router-dom/-/react-router-dom-5.1.6.tgz#07b14e7ab1893a837c8565634960dc398564b1fb" - integrity sha512-gjrxYqxz37zWEdMVvQtWPFMFj1dRDb4TGOcgyOfSXTrEXdF92L00WE3C471O3TV/RF1oskcStkXsOU0Ete4s/g== - dependencies: - "@types/history" "*" - "@types/react" "*" - "@types/react-router" "*" - -"@types/react-router@*": - version "5.0.3" - resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.0.3.tgz#855a1606e62de3f4d69ea34fb3c0e50e98e964d5" - integrity sha512-j2Gge5cvxca+5lK9wxovmGPgpVJMwjyu5lTA/Cd6fLGoPq7FXcUE1jFkEdxeyqGGz8VfHYSHCn5Lcn24BzaNKA== - dependencies: - "@types/history" "*" - "@types/react" "*" - "@types/react-scrollspy@3.3.3": version "3.3.3" resolved "https://registry.yarnpkg.com/@types/react-scrollspy/-/react-scrollspy-3.3.3.tgz#ebf82b20f0a14cf2db175f6b3d65f2e3d1acb911" @@ -8136,18 +8103,6 @@ history@4.7.2: value-equal "^0.4.0" warning "^3.0.0" -history@^4.9.0: - version "4.9.0" - resolved "https://registry.yarnpkg.com/history/-/history-4.9.0.tgz#84587c2068039ead8af769e9d6a6860a14fa1bca" - integrity sha512-H2DkjCjXf0Op9OAr6nJ56fcRkTSNrUiv41vNJ6IswJjif6wlpZK0BTfFbi7qK9dXLSYZxkq5lBsj3vUjlYBYZA== - dependencies: - "@babel/runtime" "^7.1.2" - loose-envify "^1.2.0" - resolve-pathname "^2.2.0" - tiny-invariant "^1.0.2" - tiny-warning "^1.0.0" - value-equal "^0.4.0" - history@^5.0.0: version "5.0.0" resolved "https://registry.yarnpkg.com/history/-/history-5.0.0.tgz#0cabbb6c4bbf835addb874f8259f6d25101efd08" @@ -8164,7 +8119,7 @@ hmac-drbg@^1.0.0: minimalistic-assert "^1.0.0" minimalistic-crypto-utils "^1.0.1" -hoist-non-react-statics@^3.1.0, hoist-non-react-statics@^3.3.0: +hoist-non-react-statics@^3.3.0: version "3.3.0" resolved "https://registry.yarnpkg.com/hoist-non-react-statics/-/hoist-non-react-statics-3.3.0.tgz#b09178f0122184fb95acf525daaecb4d8f45958b" integrity sha512-0XsbTXxgiaCDYDIWFcwkmerZPSwywfUqYmwT4jzewKTQSWoE6FCMoUVOeBJWK3E/CrWbxRG3m5GzY4lnIwGRBA== @@ -10196,7 +10151,7 @@ longest-streak@^2.0.1: resolved "https://registry.yarnpkg.com/longest-streak/-/longest-streak-2.0.2.tgz#2421b6ba939a443bb9ffebf596585a50b4c38e2e" integrity sha512-TmYTeEYxiAmSVdpbnQDXGtvYOIRsCMg89CVZzwzc2o7GFL1CjoiRPjH5ec0NFAVlAx3fVof9dX/t6KKRAo2OWA== -loose-envify@^1.0.0, loose-envify@^1.1.0, loose-envify@^1.2.0, loose-envify@^1.3.1, loose-envify@^1.4.0: +loose-envify@^1.0.0, loose-envify@^1.1.0, loose-envify@^1.2.0, loose-envify@^1.4.0: version "1.4.0" resolved "https://registry.yarnpkg.com/loose-envify/-/loose-envify-1.4.0.tgz#71ee51fa7be4caec1a63839f7e682d8132d30caf" integrity sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q== @@ -10552,14 +10507,6 @@ min-indent@^1.0.0: resolved "https://registry.yarnpkg.com/min-indent/-/min-indent-1.0.0.tgz#cfc45c37e9ec0d8f0a0ec3dd4ef7f7c3abe39256" integrity sha1-z8RcN+nsDY8KDsPdTvf3w6vjklY= -mini-create-react-context@^0.4.0: - version "0.4.0" - resolved "https://registry.yarnpkg.com/mini-create-react-context/-/mini-create-react-context-0.4.0.tgz#df60501c83151db69e28eac0ef08b4002efab040" - integrity sha512-b0TytUgFSbgFJGzJqXPKCFCBWigAjpjo+Fl7Vf7ZbKRDptszpppKxXH6DRXEABZ/gcEQczeb0iZ7JvL8e8jjCA== - dependencies: - "@babel/runtime" "^7.5.5" - tiny-warning "^1.0.3" - mini-css-extract-plugin@1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/mini-css-extract-plugin/-/mini-css-extract-plugin-1.0.0.tgz#4afb39f3d97b1b92eacb1ac45025416089f831bd" @@ -11823,13 +11770,6 @@ path-to-regexp@0.1.7: resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-0.1.7.tgz#df604178005f522f15eb4490e7247a1bfaa67f8c" integrity sha1-32BBeABfUi8V60SQ5yR6G/qmf4w= -path-to-regexp@^1.7.0: - version "1.7.0" - resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-1.7.0.tgz#59fde0f435badacba103a84e9d3bc64e96b9937d" - integrity sha1-Wf3g9DW62suhA6hOnTvGTpa5k30= - dependencies: - isarray "0.0.1" - path-type@^1.0.0: version "1.1.0" resolved "https://registry.yarnpkg.com/path-type/-/path-type-1.1.0.tgz#59c44f7ee491da704da415da5a4070ba4f8fe441" @@ -12965,16 +12905,16 @@ react-is@^16.13.1: resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4" integrity sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ== -react-is@^16.6.0, react-is@^16.8.6: - version "16.8.6" - resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.8.6.tgz#5bbc1e2d29141c9fbdfed456343fe2bc430a6a16" - integrity sha512-aUk3bHfZ2bRSVFFbbeVS4i+lNPZr3/WM5jT2J5omUVV1zzcs1nAaf3l51ctA5FFvCRbhrH0bdAsRRQddFJZPtA== - react-is@^16.7.0, react-is@^16.8.1, react-is@^16.8.3: version "16.8.3" resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.8.3.tgz#4ad8b029c2a718fc0cfc746c8d4e1b7221e5387d" integrity sha512-Y4rC1ZJmsxxkkPuMLwvKvlL1Zfpbcu+Bf4ZigkHup3v9EfdYhAlWAaVyA19olXq2o2mGn0w+dFKvk3pVVlYcIA== +react-is@^16.8.6: + version "16.8.6" + resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.8.6.tgz#5bbc1e2d29141c9fbdfed456343fe2bc430a6a16" + integrity sha512-aUk3bHfZ2bRSVFFbbeVS4i+lNPZr3/WM5jT2J5omUVV1zzcs1nAaf3l51ctA5FFvCRbhrH0bdAsRRQddFJZPtA== + react-is@^16.9.0: version "16.9.0" resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.9.0.tgz#21ca9561399aad0ff1a7701c01683e8ca981edcb" @@ -13055,41 +12995,20 @@ react-redux@^7.1.1: prop-types "^15.7.2" react-is "^16.9.0" -react-router-config@^5.1.1: - version "5.1.1" - resolved "https://registry.yarnpkg.com/react-router-config/-/react-router-config-5.1.1.tgz#0f4263d1a80c6b2dc7b9c1902c9526478194a988" - integrity sha512-DuanZjaD8mQp1ppHjgnnUnyOlqYXZVjnov/JzFhjLEwd3Z4dYjMSnqrEzzGThH47vpCOqPPwJM2FtthLeJ8Pbg== +react-router-dom@^0.0.0-experimental-ffd8c7d0: + version "0.0.0-experimental-ffd8c7d0" + resolved "https://registry.yarnpkg.com/react-router-dom/-/react-router-dom-0.0.0-experimental-ffd8c7d0.tgz#8742c2c7dbbc01237ea2fb97005ec3ace4be6937" + integrity sha512-tu8XLvu7ZpOeCP6xO4lNGzf+Iy9G8ElsiU/UlC/T4gb5ZbfxB/IAfZIKSzgqn8+Oh/dGca+1Xr1HZbKrib2Hfw== dependencies: - "@babel/runtime" "^7.1.2" + prop-types "^15.7.2" + react-router "0.0.0-experimental-ffd8c7d0" -react-router-dom@5.2.0: - version "5.2.0" - resolved "https://registry.yarnpkg.com/react-router-dom/-/react-router-dom-5.2.0.tgz#9e65a4d0c45e13289e66c7b17c7e175d0ea15662" - integrity sha512-gxAmfylo2QUjcwxI63RhQ5G85Qqt4voZpUXSEqCwykV0baaOTQDR1f0PmY8AELqIyVc0NEZUj0Gov5lNGcXgsA== +react-router@0.0.0-experimental-ffd8c7d0: + version "0.0.0-experimental-ffd8c7d0" + resolved "https://registry.yarnpkg.com/react-router/-/react-router-0.0.0-experimental-ffd8c7d0.tgz#ccc6187d554a4082bc503ad392318d407ea652ac" + integrity sha512-DMEkLwdFfpKnPaQi2M739iIW4erffoAF/fkbxE8j9c/uIqyX+Sy632DEGt8UsoCJEGNfrkaIUFmNqfTSz2JBzA== dependencies: - "@babel/runtime" "^7.1.2" - history "^4.9.0" - loose-envify "^1.3.1" - prop-types "^15.6.2" - react-router "5.2.0" - tiny-invariant "^1.0.2" - tiny-warning "^1.0.0" - -react-router@5.2.0: - version "5.2.0" - resolved "https://registry.yarnpkg.com/react-router/-/react-router-5.2.0.tgz#424e75641ca8747fbf76e5ecca69781aa37ea293" - integrity sha512-smz1DUuFHRKdcJC0jobGo8cVbhO3x50tCL4icacOlcwDOEQPq4TMqwx3sY1TP+DvtTgz4nm3thuo7A+BK2U0Dw== - dependencies: - "@babel/runtime" "^7.1.2" - history "^4.9.0" - hoist-non-react-statics "^3.1.0" - loose-envify "^1.3.1" - mini-create-react-context "^0.4.0" - path-to-regexp "^1.7.0" - prop-types "^15.6.2" - react-is "^16.6.0" - tiny-invariant "^1.0.2" - tiny-warning "^1.0.0" + prop-types "^15.7.2" react-scrollspy@3.4.3: version "3.4.3" @@ -13356,11 +13275,6 @@ regenerator-runtime@^0.11.0: resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.11.1.tgz#be05ad7f9bf7d22e056f9726cee5017fbf19e2e9" integrity sha512-MguG95oij0fC3QV3URf4V2SDYGJhJnJGqvIIgdECeODCT98wSWDAJ94SSuVpYQUoTcGUIL6L4yNB7j1DFFHSBg== -regenerator-runtime@^0.12.0: - version "0.12.1" - resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.12.1.tgz#fa1a71544764c036f8c49b13a08b2594c9f8a0de" - integrity sha512-odxIc1/vDlo4iZcfXqRYFj0vpXFNoGdKMAUieAlFYO6m/nl5e9KR/beGf41z4a1FI+aQgtjhuaSlDxQ0hmkrHg== - regenerator-runtime@^0.13.2: version "0.13.2" resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.13.2.tgz#32e59c9a6fb9b1a4aff09b4930ca2d4477343447" @@ -15317,11 +15231,6 @@ timsort@^0.3.0: resolved "https://registry.yarnpkg.com/timsort/-/timsort-0.3.0.tgz#405411a8e7e6339fe64db9a234de11dc31e02bd4" integrity sha1-QFQRqOfmM5/mTbmiNN4R3DHgK9Q= -tiny-invariant@^1.0.2: - version "1.0.4" - resolved "https://registry.yarnpkg.com/tiny-invariant/-/tiny-invariant-1.0.4.tgz#346b5415fd93cb696b0c4e8a96697ff590f92463" - integrity sha512-lMhRd/djQJ3MoaHEBrw8e2/uM4rs9YMNk0iOr8rHQ0QdbM7D4l0gFl3szKdeixrlyfm9Zqi4dxHCM2qVG8ND5g== - tiny-invariant@^1.0.6: version "1.0.6" resolved "https://registry.yarnpkg.com/tiny-invariant/-/tiny-invariant-1.0.6.tgz#b3f9b38835e36a41c843a3b0907a5a7b3755de73" @@ -15332,16 +15241,6 @@ tiny-json-http@^7.1.2: resolved "https://registry.yarnpkg.com/tiny-json-http/-/tiny-json-http-7.1.2.tgz#620e189849bab08992ec23fada7b48c7c61637b4" integrity sha512-XB9Bu+ohdQso6ziPFNVqK+pcTt0l8BSRkW/CCBq0pUVlLxcYDsorpo7ae5yPhu2CF1xYgJuKVLF7cfOGeLCTlA== -tiny-warning@^1.0.0: - version "1.0.2" - resolved "https://registry.yarnpkg.com/tiny-warning/-/tiny-warning-1.0.2.tgz#1dfae771ee1a04396bdfde27a3adcebc6b648b28" - integrity sha512-rru86D9CpQRLvsFG5XFdy0KdLAvjdQDyZCsRcuu60WtzFylDM3eAWSxEVz5kzL2Gp544XiUvPbVKtOA/txLi9Q== - -tiny-warning@^1.0.3: - version "1.0.3" - resolved "https://registry.yarnpkg.com/tiny-warning/-/tiny-warning-1.0.3.tgz#94a30db453df4c643d0fd566060d60a875d84754" - integrity sha512-lBN9zLN/oAf68o3zNXYrdCt1kP8WsiGW8Oo2ka41b2IM5JL/S1CTyX1rW0mb/zSuJun0ZUrDxx4sqvYS2FWzPA== - tippy.js@^5.1.1: version "5.1.1" resolved "https://registry.yarnpkg.com/tippy.js/-/tippy.js-5.1.1.tgz#2a85cf3fb302ddc5ba1fca944e1f39bec62cb7b6" From 82bae87f2388ebce971e231ed6386bb53dea29f4 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Thu, 22 Oct 2020 22:39:05 +0800 Subject: [PATCH 10/20] Allow onMouseOver, onFocus and ref passthrough on Preloading(Nav)Link --- website/src/views/routes/PreloadingLink.tsx | 44 +++++++++++++++------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/website/src/views/routes/PreloadingLink.tsx b/website/src/views/routes/PreloadingLink.tsx index 9f08becb28..ce5fa62407 100644 --- a/website/src/views/routes/PreloadingLink.tsx +++ b/website/src/views/routes/PreloadingLink.tsx @@ -1,24 +1,42 @@ -import React, { useCallback, useContext } from 'react'; +import React, { forwardRef, useCallback, useContext } from 'react'; import { LinkProps, Link, NavLink, NavLinkProps } from 'react-router-dom'; import { RoutePreloaderContext } from './RoutePreloaderContext'; -function usePreloadCallbacks(to: LinkProps['to'] | NavLinkProps['to']) { +function usePreloadCallbacks({ onMouseOver, onFocus, to }: LinkProps | NavLinkProps) { const preloaderContext = useContext(RoutePreloaderContext); if (!preloaderContext) { throw new Error('Preloading link components cannot be used outside RoutePreloaderContext.'); } const preloadCode = useCallback(() => preloaderContext.preloadCode(to), [preloaderContext, to]); - return { preloadCode }; + + const combinedOnMouseOver = useCallback( + (e) => { + onMouseOver?.(e); + preloadCode(); + }, + [onMouseOver, preloadCode], + ); + const combinedOnFocus = useCallback( + (e) => { + onFocus?.(e); + preloadCode(); + }, + [onFocus, preloadCode], + ); + + return { onMouseOver: combinedOnMouseOver, onFocus: combinedOnFocus }; } -export const PreloadingLink: React.FC> = (props) => { - const { preloadCode } = usePreloadCallbacks(props.to); - return ; -}; +export const PreloadingLink = forwardRef( + function PreloadingLinkComponent(props, ref) { + const preloadCallbacks = usePreloadCallbacks(props); + return ; + }, +); -export const PreloadingNavLink: React.FC> = ( - props, -) => { - const { preloadCode } = usePreloadCallbacks(props.to); - return ; -}; +export const PreloadingNavLink = forwardRef( + function PreloadingNavLinkComponent(props, ref) { + const preloadCallbacks = usePreloadCallbacks(props); + return ; + }, +); From 4620ed547ba11672b8d2467b0ea3847b03badd65 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Thu, 22 Oct 2020 22:40:35 +0800 Subject: [PATCH 11/20] Define entry points for Module{Finder,Page} --- website/src/entry/main.tsx | 2 +- .../views/modules/ModuleFinder.entrypoint.ts | 16 ++ .../ModuleFinderContainer.tsx | 5 +- .../views/modules/ModulePage.entrypoint.ts | 39 +++++ .../src/views/modules/ModulePageContainer.tsx | 161 ++++++------------ .../src/views/modules/ModulePageContent.tsx | 14 +- .../src/views/routes/EntryPointContainer.tsx | 5 +- website/src/views/routes/Routes.tsx | 29 ++-- website/src/views/routes/types.ts | 4 +- .../views/timetable/Timetable.entrypoint.ts | 2 +- .../views/timetable/TimetableContainer.tsx | 5 +- website/src/views/today/Today.entrypoint.ts | 2 +- website/src/views/today/TodayContainer.tsx | 2 +- 13 files changed, 146 insertions(+), 140 deletions(-) create mode 100644 website/src/views/modules/ModuleFinder.entrypoint.ts create mode 100644 website/src/views/modules/ModulePage.entrypoint.ts diff --git a/website/src/entry/main.tsx b/website/src/entry/main.tsx index 2e4650ef43..43410d7b50 100644 --- a/website/src/entry/main.tsx +++ b/website/src/entry/main.tsx @@ -9,7 +9,7 @@ import 'core-js/es/promise/finally'; import React from 'react'; // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore -import { unstable_createRoot as createRoot } from 'react-dom/profiling'; +import { unstable_createRoot as createRoot } from 'react-dom'; import ReactModal from 'react-modal'; import configureStore from 'bootstrapping/configure-store'; diff --git a/website/src/views/modules/ModuleFinder.entrypoint.ts b/website/src/views/modules/ModuleFinder.entrypoint.ts new file mode 100644 index 0000000000..6ed615f511 --- /dev/null +++ b/website/src/views/modules/ModuleFinder.entrypoint.ts @@ -0,0 +1,16 @@ +import { JSResource } from 'utils/JSResource'; +import { EntryPoint } from 'views/routes/types'; + +export type PreparedProps = unknown; + +const entryPoint: EntryPoint = { + component: JSResource( + 'ModuleFinder', + () => import(/* webpackChunkName: "ModuleFinder.route" */ './ModuleFinderContainer'), + ), + prepare() { + return {}; + }, +}; + +export default entryPoint; diff --git a/website/src/views/modules/ModuleFinderContainer/ModuleFinderContainer.tsx b/website/src/views/modules/ModuleFinderContainer/ModuleFinderContainer.tsx index 2adbef8866..6e3349769d 100644 --- a/website/src/views/modules/ModuleFinderContainer/ModuleFinderContainer.tsx +++ b/website/src/views/modules/ModuleFinderContainer/ModuleFinderContainer.tsx @@ -13,6 +13,7 @@ import { import classnames from 'classnames'; import { hot } from 'react-hot-loader/root'; +import type { EntryPointComponentProps } from 'views/routes/types'; import { ElasticSearchResult } from 'types/vendor/elastic-search'; import { ModuleInformation } from 'types/modules'; @@ -30,6 +31,8 @@ import { HIGHLIGHT_OPTIONS } from 'utils/elasticSearch'; import config from 'config'; import styles from './ModuleFinderContainer.scss'; +type Props = EntryPointComponentProps; + const esIndex = 'modules_v2'; const esHostUrl = `${forceElasticsearchHost() || config.elasticsearchBaseUrl}/${esIndex}`; const searchkit = new SearchkitManager(esHostUrl, { @@ -56,7 +59,7 @@ const ModuleInformationListComponent: React.FC = ({ hits }) => ( ); -const ModuleFinderContainer: React.FC = () => { +const ModuleFinderContainer: React.FC = () => { return (
{pageHead} diff --git a/website/src/views/modules/ModulePage.entrypoint.ts b/website/src/views/modules/ModulePage.entrypoint.ts new file mode 100644 index 0000000000..722bbd38b3 --- /dev/null +++ b/website/src/views/modules/ModulePage.entrypoint.ts @@ -0,0 +1,39 @@ +import type { Params } from 'react-router'; +import { fetchModule } from 'actions/moduleBank'; +import { Module } from 'types/modules'; +import { captureException } from 'utils/error'; +import { JSResource } from 'utils/JSResource'; +import { EntryPoint } from 'views/routes/types'; + +export type PreparedProps = { + module: JSResource; +}; + +const getPropsFromParams = (params: Params) => ({ + moduleCode: (params.moduleCode ?? '').toUpperCase(), +}); + +const entryPoint: EntryPoint = { + component: JSResource( + 'ModulePage', + () => import(/* webpackChunkName: "ModulePage.route" */ './ModulePageContainer'), + ), + prepare(params, dispatch) { + const { moduleCode } = getPropsFromParams(params); + const module = JSResource(`ModulePageContainer-module-${moduleCode}`, () => + dispatch(fetchModule(moduleCode)).catch((error) => { + captureException(error); + // TODO: If there is an error but module data can still be found, we + // can assume module has been loaded at some point, so we can just show + // that instead + throw error; + }), + ); + module.preloadOrReloadIfError(); + return { + module, + }; + }, +}; + +export default entryPoint; diff --git a/website/src/views/modules/ModulePageContainer.tsx b/website/src/views/modules/ModulePageContainer.tsx index bd56ad0be6..cd21de07ee 100644 --- a/website/src/views/modules/ModulePageContainer.tsx +++ b/website/src/views/modules/ModulePageContainer.tsx @@ -1,46 +1,42 @@ -import * as React from 'react'; -import { AxiosError } from 'axios'; -import { connect, MapDispatchToPropsNonObject } from 'react-redux'; -import { match as Match, Navigate, RouteComponentProps, withRouter } from 'react-router-dom'; -import deferComponentRender from 'views/hocs/deferComponentRender'; +import React, { Suspense, useEffect } from 'react'; +import type { Params } from 'react-router'; +import { useLocation, useNavigate, useParams } from 'react-router-dom'; import { get } from 'lodash'; -import type { Module, ModuleCode } from 'types/modules'; -import type { Dispatch } from 'types/redux'; -import type { State as StoreState } from 'types/state'; +import type { Module } from 'types/modules'; +import type { EntryPointComponentProps } from 'views/routes/types'; +import type { JSResource } from 'utils/JSResource'; -import { fetchModule } from 'actions/moduleBank'; -import { captureException, retryImport } from 'utils/error'; import ApiError from 'views/errors/ApiError'; import ModuleNotFoundPage from 'views/errors/ModuleNotFoundPage'; import LoadingSpinner from 'views/components/LoadingSpinner'; import { modulePage } from 'views/routes/paths'; +import ErrorBoundary from 'views/errors/ErrorBoundary'; -import { Props as ModulePageContentProp } from './ModulePageContent'; +import ModulePageContent from './ModulePageContent'; -type Params = { - moduleCode: string; +type PreparedProps = { + module: JSResource; }; -type OwnProps = RouteComponentProps; +type Props = EntryPointComponentProps; -type DispatchProps = { - fetchModule: () => Promise; -}; +const getPropsFromParams = (params: Params) => ({ + moduleCode: (params.moduleCode ?? '').toUpperCase(), +}); -type Props = OwnProps & - DispatchProps & { - module: Module | null; - moduleCode: ModuleCode; - fetchModule: () => Promise; - }; +// TODO: Generalize this smart error fallback for other error boundaries +const ErrorFallback: React.FC<{ error: Error; moduleCode: string }> = ({ error, moduleCode }) => { + if (get(error, ['response', 'status'], 200) === 404) { + return ; + } -type State = { - ModulePageContent: React.ComponentType | null; - error?: Error; + // TODO: Handle other errors; we can't just assume everything's a load error + return ; }; /** + * TODO: Update docstring * Wrapper component that loads both module data and the module page component * simultaneously, and displays the correct component depending on the state. * @@ -55,95 +51,36 @@ type State = { * - Loading: Either requests are pending * - Loaded: Both requests are successfully loaded */ -export class ModulePageContainerComponent extends React.PureComponent { - state: State = { - ModulePageContent: null, - }; +export const ModulePageContainerComponent: React.FC = ({ prepared: { module } }) => { + const location = useLocation(); + const navigate = useNavigate(); - componentDidMount() { - this.fetchModule(); - this.fetchPageImport(); - } - - componentDidUpdate(prevProps: Props) { - if (prevProps.moduleCode !== this.props.moduleCode) { - this.fetchModule(); - } - } - - fetchModule = () => { - this.setState({ error: undefined }); - this.props.fetchModule().catch(this.handleFetchError); - }; - - fetchPageImport() { - // Try importing ModulePageContent thrice if we're online and - // getting the "Loading chunk x failed." error. - retryImport(() => import(/* webpackChunkName: "module" */ 'views/modules/ModulePageContent')) - .then((module) => this.setState({ ModulePageContent: module.default })) - .catch(this.handleFetchError); - } - - handleFetchError = (error: AxiosError) => { - this.setState({ error }); - captureException(error); - }; - - render() { - const { ModulePageContent, error } = this.state; - const { module, moduleCode, match, location } = this.props; - - if (get(error, ['response', 'status'], 200) === 404) { - return ; - } - - // If there is an error but module data can still be found, we assume module has - // been loaded at some point, so we just show that instead - if (error && !module) { - return ; - } + const params = useParams(); + const { moduleCode } = getPropsFromParams(params); + useEffect(() => { // Navigate to canonical URL - if (module && match.url !== modulePage(moduleCode, module.title)) { - return ; + if (module.get() && location.pathname !== modulePage(moduleCode, module.get()?.title)) { + navigate( + { ...location, pathname: modulePage(moduleCode, module.get()?.title) }, + { replace: true }, + ); } - - if (module && ModulePageContent) { - // Unique key forces component to remount whenever the user moves to - // a new module. This allows the internal state (eg. currently selected - // timetable semester) of to be consistent - return ; - } - - return ; - } -} - -const getPropsFromMatch = (match: Match) => ({ - moduleCode: (match.params.moduleCode ?? '').toUpperCase(), -}); - -const mapStateToProps = ({ moduleBank }: StoreState, ownProps: OwnProps) => { - const { moduleCode } = getPropsFromMatch(ownProps.match); - return { - moduleCode, - module: moduleBank.modules[moduleCode], - }; -}; - -const mapDispatchToProps = (dispatch: Dispatch, ownProps: OwnProps) => { - const { moduleCode } = getPropsFromMatch(ownProps.match); - return { - fetchModule: () => dispatch(fetchModule(moduleCode)), - }; + }, [location, module, moduleCode, navigate]); + + // Unique key forces component to remount whenever the user moves to + // a new module. This allows the internal state (eg. currently selected + // timetable semester) of to be consistent + return ( + } + > + }> + + + + ); }; -const connectedModulePageContainer = connect( - mapStateToProps, - // Cast required because the version of Dispatch defined by connect does not have the extensions defined - // in our Dispatch - mapDispatchToProps as MapDispatchToPropsNonObject, -)(ModulePageContainerComponent); -// const routedModulePageContainer = withRouter(connectedModulePageContainer); -const routedModulePageContainer = connectedModulePageContainer; -export default deferComponentRender(routedModulePageContainer); +export default ModulePageContainerComponent; diff --git a/website/src/views/modules/ModulePageContent.tsx b/website/src/views/modules/ModulePageContent.tsx index 3096c4e98b..55472c5641 100644 --- a/website/src/views/modules/ModulePageContent.tsx +++ b/website/src/views/modules/ModulePageContent.tsx @@ -29,12 +29,13 @@ import Title from 'views/components/Title'; import ScrollToTop from 'views/components/ScrollToTop'; import { Archive, Check } from 'react-feather'; import ErrorBoundary from 'views/errors/ErrorBoundary'; +import type { JSResource } from 'utils/JSResource'; import styles from './ModulePageContent.scss'; import ReportError from './ReportError'; export type Props = { - module: Module; + module: Module | JSResource; archiveYear?: string; }; @@ -59,7 +60,16 @@ class ModulePageContent extends React.Component { toggleMenu = (isMenuOpen: boolean) => this.setState({ isMenuOpen }); render() { - const { module, archiveYear } = this.props; + const { module: moduleOrResource, archiveYear } = this.props; + + // TODO: Deprecate module: Module; always use resource + let module: Module; + if ('read' in moduleOrResource) { + module = moduleOrResource.read(); + } else { + module = moduleOrResource; + } + const { moduleCode, title } = module; const pageTitle = `${moduleCode} ${title}`; diff --git a/website/src/views/routes/EntryPointContainer.tsx b/website/src/views/routes/EntryPointContainer.tsx index b6f7eb93e9..befad800c3 100644 --- a/website/src/views/routes/EntryPointContainer.tsx +++ b/website/src/views/routes/EntryPointContainer.tsx @@ -1,7 +1,8 @@ import React, { memo } from 'react'; import { useDispatch } from 'react-redux'; import { Outlet, useParams } from 'react-router-dom'; -import { EntryPoint } from './types'; +import type { Dispatch } from 'types/redux'; +import type { EntryPoint } from './types'; type Props = { entryPoint: EntryPoint; @@ -10,7 +11,7 @@ type Props = { const EntryPointContainer: React.FC = ({ entryPoint }) => { const EntryPointComponent = entryPoint.component.read(); const params = useParams(); - const dispatch = useDispatch(); + const dispatch = useDispatch(); // TODO: Figure out a way to prepare outside render. Should reuse prepare result from preload return ( diff --git a/website/src/views/routes/Routes.tsx b/website/src/views/routes/Routes.tsx index dff5f9d912..60bfa8a035 100644 --- a/website/src/views/routes/Routes.tsx +++ b/website/src/views/routes/Routes.tsx @@ -7,11 +7,13 @@ import TimetableRootRedirector from 'views/timetable/TimetableRootRedirector'; import appShellEntryPoint from 'views/AppShell.entrypoint'; import timetableEntryPoint from 'views/timetable/Timetable.entrypoint'; import todayEntryPoint from 'views/today/Today.entrypoint'; +import moduleFinderEntryPoint from 'views/modules/ModuleFinder.entrypoint'; +import modulePageEntryPoint from 'views/modules/ModulePage.entrypoint'; + +import type { Dispatch } from 'types/redux'; +import type { EntryPoint, EntryPointPartialRouteObject, EntryPointRouteObject } from './types'; -// import TimetableContainer from 'views/timetable/TimetableContainer'; -// import ModulePageContainer from 'views/modules/ModulePageContainer'; // import ModuleArchiveContainer from 'views/modules/ModuleArchiveContainer'; -// import ModuleFinderContainer from 'views/modules/ModuleFinderContainer'; // import VenuesContainer from 'views/venues/VenuesContainer'; // import SettingsContainer from 'views/settings/SettingsContainer'; // import AboutContainer from 'views/static/AboutContainer'; @@ -26,11 +28,10 @@ import todayEntryPoint from 'views/today/Today.entrypoint'; import EntryPointContainer from './EntryPointContainer'; import { RoutePreloaderProvider } from './RoutePreloaderContext'; -import { EntryPoint, EntryPointPartialRouteObject, EntryPointRouteObject } from './types'; -function makeEntryPointRoute( +function entryPointRoute( entryPoint: EntryPoint, - dispatch: ReturnType, + dispatch: Dispatch, ): EntryPointPartialRouteObject { return { element: , @@ -42,8 +43,6 @@ function makeEntryPointRoute( }; } -// -// // // // today @@ -58,9 +57,7 @@ function makeEntryPointRoute( // // -function createPartialRoutes( - dispatch: ReturnType, -): EntryPointPartialRouteObject[] { +function createPartialRoutes(dispatch: Dispatch): EntryPointPartialRouteObject[] { // IMPORTANT: Remember to update any route changes on the sitemap return [ // v2 routes @@ -79,17 +76,19 @@ function createPartialRoutes( // { path: '/api', element: }, { - ...makeEntryPointRoute(appShellEntryPoint, dispatch), + ...entryPointRoute(appShellEntryPoint, dispatch), children: [ { path: '/', element: }, { path: '/timetable', children: [ - { path: ':semester/*', ...makeEntryPointRoute(timetableEntryPoint, dispatch) }, + { path: ':semester/*', ...entryPointRoute(timetableEntryPoint, dispatch) }, { path: '/', element: }, ], }, - { path: '/today', ...makeEntryPointRoute(todayEntryPoint, dispatch) }, + { path: '/today', ...entryPointRoute(todayEntryPoint, dispatch) }, + { path: '/modules', ...entryPointRoute(moduleFinderEntryPoint, dispatch) }, + { path: '/modules/:moduleCode/*', ...entryPointRoute(modulePageEntryPoint, dispatch) }, // 404 page { element: }, @@ -124,7 +123,7 @@ function createRoutesFromArray(array: EntryPointPartialRouteObject[]): EntryPoin } const Routes: React.FC = () => { - const dispatch = useDispatch(); + const dispatch = useDispatch(); const routes = useMemo(() => createRoutesFromArray(createPartialRoutes(dispatch)), [dispatch]); const element = useRoutes(routes); return {element}; diff --git a/website/src/views/routes/types.ts b/website/src/views/routes/types.ts index 909ef1bbc6..0c05bb7396 100644 --- a/website/src/views/routes/types.ts +++ b/website/src/views/routes/types.ts @@ -1,6 +1,6 @@ import type { Params, PartialRouteObject, RouteObject } from 'react-router'; -import type { useDispatch } from 'react-redux'; import type { JSResource } from 'utils/JSResource'; +import type { Dispatch } from 'types/redux'; export type EntryPointComponentProps = React.PropsWithChildren<{ params: Params; @@ -9,7 +9,7 @@ export type EntryPointComponentProps = React.PropsWithChildren<{ export type EntryPoint> = { component: JSResource>; - prepare: (matchParams: Params, dispatch: ReturnType) => PreparedProps; + prepare: (matchParams: Params, dispatch: Dispatch) => PreparedProps; }; export interface EntryPointRouteObject extends RouteObject { diff --git a/website/src/views/timetable/Timetable.entrypoint.ts b/website/src/views/timetable/Timetable.entrypoint.ts index 3fd4568b4d..4f8c90e6e1 100644 --- a/website/src/views/timetable/Timetable.entrypoint.ts +++ b/website/src/views/timetable/Timetable.entrypoint.ts @@ -1,5 +1,5 @@ import { JSResource } from 'utils/JSResource'; -import { EntryPoint } from 'views/routes/EntryPointContainer'; +import { EntryPoint } from 'views/routes/types'; const entryPoint: EntryPoint = { component: JSResource( diff --git a/website/src/views/timetable/TimetableContainer.tsx b/website/src/views/timetable/TimetableContainer.tsx index a834e9bceb..5f759f5a19 100644 --- a/website/src/views/timetable/TimetableContainer.tsx +++ b/website/src/views/timetable/TimetableContainer.tsx @@ -23,7 +23,8 @@ import SemesterSwitcher from 'views/components/semester-switcher/SemesterSwitche import LoadingSpinner from 'views/components/LoadingSpinner'; import ScrollToTop from 'views/components/ScrollToTop'; import { State } from 'types/state'; -import type { EntryPointComponentProps } from 'views/routes/EntryPointContainer'; +import type { EntryPointComponentProps } from 'views/routes/types'; +import type { Dispatch } from 'types/redux'; import TimetableContent from './TimetableContent'; import styles from './TimetableContainer.scss'; @@ -59,7 +60,7 @@ export const TimetableContainerComponent: React.FC = () => { const modules = useSelector((state: State) => state.moduleBank.modules); const activeSemester = useSelector((state: State) => state.app.activeSemester); - const dispatch = useDispatch(); + const dispatch = useDispatch(); const [importedTimetable, setImportedTimetable] = useState(() => semester && action ? deserializeTimetable(location.search) : null, diff --git a/website/src/views/today/Today.entrypoint.ts b/website/src/views/today/Today.entrypoint.ts index 76ebbf8f6e..aa845e0cf5 100644 --- a/website/src/views/today/Today.entrypoint.ts +++ b/website/src/views/today/Today.entrypoint.ts @@ -1,6 +1,6 @@ import { JSResource } from 'utils/JSResource'; import venueLocationResource from 'views/components/map/venueLocationResource'; -import { EntryPoint } from 'views/routes/EntryPointContainer'; +import { EntryPoint } from 'views/routes/types'; export type PreparedProps = unknown; diff --git a/website/src/views/today/TodayContainer.tsx b/website/src/views/today/TodayContainer.tsx index 4a8734dbaf..c3d2062405 100644 --- a/website/src/views/today/TodayContainer.tsx +++ b/website/src/views/today/TodayContainer.tsx @@ -40,7 +40,7 @@ import MapContext from 'views/components/map/MapContext'; import { formatTime, getDayIndex } from 'utils/timify'; import { breakpointUp } from 'utils/css'; import { State as StoreState } from 'types/state'; -import type { EntryPointComponentProps } from 'views/routes/EntryPointContainer'; +import type { EntryPointComponentProps } from 'views/routes/types'; import DayEvents from './DayEvents'; import DayHeader from './DayHeader'; From 6f00456af4718adfa11b3ace2145251407aba44d Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Sat, 24 Oct 2020 10:32:08 +0800 Subject: [PATCH 12/20] Fix 404 route --- website/src/views/routes/Routes.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/website/src/views/routes/Routes.tsx b/website/src/views/routes/Routes.tsx index 60bfa8a035..4799b24aa1 100644 --- a/website/src/views/routes/Routes.tsx +++ b/website/src/views/routes/Routes.tsx @@ -91,7 +91,10 @@ function createPartialRoutes(dispatch: Dispatch): EntryPointPartialRouteObject[] { path: '/modules/:moduleCode/*', ...entryPointRoute(modulePageEntryPoint, dispatch) }, // 404 page - { element: }, + { + path: '*', + element: , + }, ], }, ]; From 21ebc3366ee7d56db51cf6ddd1361652c23d8ff8 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Sat, 24 Oct 2020 11:21:34 +0800 Subject: [PATCH 13/20] Define entry points for ModuleArchive --- .../views/modules/ModuleArchive.entrypoint.ts | 51 +++++ .../views/modules/ModuleArchiveContainer.tsx | 184 +++++------------- .../views/modules/ModulePage.entrypoint.ts | 9 +- .../src/views/modules/ModulePageContainer.tsx | 31 ++- website/src/views/routes/Routes.tsx | 37 +++- 5 files changed, 149 insertions(+), 163 deletions(-) create mode 100644 website/src/views/modules/ModuleArchive.entrypoint.ts diff --git a/website/src/views/modules/ModuleArchive.entrypoint.ts b/website/src/views/modules/ModuleArchive.entrypoint.ts new file mode 100644 index 0000000000..ed13e33f10 --- /dev/null +++ b/website/src/views/modules/ModuleArchive.entrypoint.ts @@ -0,0 +1,51 @@ +import type { Params } from 'react-router'; +import { fetchModuleArchive } from 'actions/moduleBank'; +import { captureException } from 'utils/error'; +import { JSResource } from 'utils/JSResource'; +import type { Module, ModuleCode } from 'types/modules'; +import type { EntryPoint } from 'views/routes/types'; + +export type PreparedProps = { + module: JSResource; + moduleCode: ModuleCode; + archiveYear: string; +}; + +const getPropsFromParams = (params: Params) => { + const { archiveYear = '', moduleCode = '' } = params; + return { + moduleCode: moduleCode.toUpperCase(), + archiveYear: archiveYear.replace('-', '/'), + }; +}; + +/** + * This is very similar to ModulePage.entrypoint except it is used for the + * archive page, so it uses different code paths for data fetching. + */ +const entryPoint: EntryPoint = { + component: JSResource( + 'ModuleArchive', + () => import(/* webpackChunkName: "ModuleArchive.route" */ './ModuleArchiveContainer'), + ), + prepare(params, dispatch) { + const { moduleCode, archiveYear } = getPropsFromParams(params); + const module = JSResource(`ModuleArchiveContainer-module-${moduleCode}-${archiveYear}`, () => + dispatch(fetchModuleArchive(moduleCode, archiveYear)).catch((error) => { + captureException(error); + // TODO: If there is an error but module data can still be found, we + // can assume module has been loaded at some point, so we can just show + // that instead + throw error; + }), + ); + module.preloadOrReloadIfError(); + return { + module, + moduleCode, + archiveYear, + }; + }, +}; + +export default entryPoint; diff --git a/website/src/views/modules/ModuleArchiveContainer.tsx b/website/src/views/modules/ModuleArchiveContainer.tsx index 24765d9757..3dc88431b7 100644 --- a/website/src/views/modules/ModuleArchiveContainer.tsx +++ b/website/src/views/modules/ModuleArchiveContainer.tsx @@ -1,159 +1,71 @@ -import * as React from 'react'; -import { AxiosError } from 'axios'; -import { connect, MapDispatchToPropsNonObject } from 'react-redux'; -import { match as Match, Navigate, RouteComponentProps, withRouter } from 'react-router-dom'; -import deferComponentRender from 'views/hocs/deferComponentRender'; +import React, { Suspense, useEffect } from 'react'; +import { useLocation, useNavigate } from 'react-router-dom'; import { get } from 'lodash'; import type { Module, ModuleCode } from 'types/modules'; -import type { State as StoreState } from 'types/state'; -import type { Dispatch } from 'types/redux'; +import type { EntryPointComponentProps } from 'views/routes/types'; +import type { JSResource } from 'utils/JSResource'; -import { fetchModuleArchive } from 'actions/moduleBank'; -import { captureException, retryImport } from 'utils/error'; import ApiError from 'views/errors/ApiError'; import ModuleNotFoundPage from 'views/errors/ModuleNotFoundPage'; import LoadingSpinner from 'views/components/LoadingSpinner'; import { moduleArchive } from 'views/routes/paths'; +import ErrorBoundary from 'views/errors/ErrorBoundary'; -import { Props as ModulePageContentProp } from './ModulePageContent'; +import ModulePageContent from './ModulePageContent'; -type Params = { - year: string; - moduleCode: string; +type PreparedProps = { + module: JSResource; + moduleCode: ModuleCode; + archiveYear: string; }; -type OwnProps = RouteComponentProps; - -type DispatchProps = { - fetchModule: () => Promise; -}; +type Props = EntryPointComponentProps; -type Props = OwnProps & - DispatchProps & { - module: Module | null; - moduleCode: ModuleCode; - archiveYear: string; - }; +// TODO: Generalize this smart error fallback for other error boundaries +const ErrorFallback: React.FC<{ + error: Error; + moduleCode: string; +}> = ({ error, moduleCode }) => { + if (get(error, ['response', 'status'], 200) === 404) { + return ; + } -type State = { - ModulePageContent: React.ComponentType | null; - error?: Error; + // TODO: Handle other errors; we can't just assume everything's a load error + return ; }; /** * Wrapper component for the archive page that handles data fetching and error handling. * This is very similar to ModulePageContainer except it is used for the archive - * page, so it uses different code paths for canonical URL, data fetching and - * error handling - the normal page tries to check the archives if this year's - * API returns 404, while this page doesn't. + * page, so it uses different code paths for canonical URL and error handling - + * the normal page tries to check the archives if this year's API returns 404, + * while this page doesn't. */ -export class ModuleArchiveContainerComponent extends React.PureComponent { - state: State = { - ModulePageContent: null, - }; - - componentDidMount() { - this.fetchModule(); - this.fetchPageImport(); - } - - componentDidUpdate(prevProps: Props) { - if ( - prevProps.moduleCode !== this.props.moduleCode || - prevProps.archiveYear !== this.props.archiveYear - ) { - this.fetchModule(); +export const ModuleArchiveContainerComponent: React.FC = ({ + prepared: { module, moduleCode, archiveYear }, +}) => { + const location = useLocation(); + const navigate = useNavigate(); + + useEffect(() => { + // Navigate to canonical URL + const canonicalUrl = moduleArchive(moduleCode, archiveYear, module.get()?.title); + if (module.get() && location.pathname !== canonicalUrl) { + navigate({ ...location, pathname: canonicalUrl }, { replace: true }); } - } - - fetchModule = () => { - this.setState({ error: undefined }); - this.props.fetchModule().catch(this.handleFetchError); - }; - - fetchPageImport() { - // Try importing ModulePageContent thrice if we're online and - // getting the "Loading chunk x failed." error. - retryImport(() => import(/* webpackChunkName: "module" */ 'views/modules/ModulePageContent')) - .then((module) => this.setState({ ModulePageContent: module.default })) - .catch(this.handleFetchError); - } - - handleFetchError = (error: AxiosError) => { - this.setState({ error }); - captureException(error); - }; - - render() { - const { ModulePageContent, error } = this.state; - const { module, moduleCode, match, location, archiveYear } = this.props; - - if (get(error, ['response', 'status'], 200) === 404) { - return ; - } - - // If there is an error but module data can still be found, we assume module has - // been loaded at some point, so we just show that instead - if (error && !module) { - return ; - } - - // Redirect to canonical URL - if (module) { - const canonicalUrl = moduleArchive(moduleCode, archiveYear, module.title); - if (match.url !== canonicalUrl) { - return ; - } - } - - if (module && ModulePageContent) { - // Unique key forces component to remount whenever the user moves to - // a new module. This allows the internal state (eg. currently selected - // timetable semester) of to be consistent - return ( - - ); - } - - return ; - } -} - -const getPropsFromMatch = (match: Match) => { - const { year = '', moduleCode = '' } = match.params; - return { - moduleCode: moduleCode.toUpperCase(), - year: year.replace('-', '/'), - }; -}; - -const mapStateToProps = ({ moduleBank }: StoreState, ownProps: OwnProps) => { - const { moduleCode, year } = getPropsFromMatch(ownProps.match); - return { - moduleCode, - archiveYear: year, - module: get(moduleBank.moduleArchive, [moduleCode, year], null), - }; -}; - -const mapDispatchToProps = (dispatch: Dispatch, ownProps: OwnProps) => { - const { moduleCode, year } = getPropsFromMatch(ownProps.match); - return { - fetchModule: () => dispatch(fetchModuleArchive(moduleCode, year)), - }; + }, [archiveYear, location, module, moduleCode, navigate]); + + return ( + } + > + }> + + + + ); }; -const connectedModuleArchiveContainer = connect( - mapStateToProps, - // Cast required because the version of Dispatch defined by connect does not have the extensions defined - // in our Dispatch - mapDispatchToProps as MapDispatchToPropsNonObject, -)(ModuleArchiveContainerComponent); -const routedModuleArchiveContainer = connectedModuleArchiveContainer; -// const routedModuleArchiveContainer = withRouter(connectedModuleArchiveContainer); -export default deferComponentRender(routedModuleArchiveContainer); +export default ModuleArchiveContainerComponent; diff --git a/website/src/views/modules/ModulePage.entrypoint.ts b/website/src/views/modules/ModulePage.entrypoint.ts index 722bbd38b3..52c1c7cfa2 100644 --- a/website/src/views/modules/ModulePage.entrypoint.ts +++ b/website/src/views/modules/ModulePage.entrypoint.ts @@ -1,12 +1,13 @@ import type { Params } from 'react-router'; import { fetchModule } from 'actions/moduleBank'; -import { Module } from 'types/modules'; import { captureException } from 'utils/error'; import { JSResource } from 'utils/JSResource'; -import { EntryPoint } from 'views/routes/types'; +import type { Module, ModuleCode } from 'types/modules'; +import type { EntryPoint } from 'views/routes/types'; export type PreparedProps = { module: JSResource; + moduleCode: ModuleCode; }; const getPropsFromParams = (params: Params) => ({ @@ -29,9 +30,13 @@ const entryPoint: EntryPoint = { throw error; }), ); + // TODO: If the resource doesn't exist, we're looking at spraying many + // requests for the same 404 resource because `prepare` may be called + // multiple times on a single navigation. module.preloadOrReloadIfError(); return { module, + moduleCode, }; }, }; diff --git a/website/src/views/modules/ModulePageContainer.tsx b/website/src/views/modules/ModulePageContainer.tsx index cd21de07ee..8617eeabf6 100644 --- a/website/src/views/modules/ModulePageContainer.tsx +++ b/website/src/views/modules/ModulePageContainer.tsx @@ -1,9 +1,8 @@ import React, { Suspense, useEffect } from 'react'; -import type { Params } from 'react-router'; -import { useLocation, useNavigate, useParams } from 'react-router-dom'; +import { useLocation, useNavigate } from 'react-router-dom'; import { get } from 'lodash'; -import type { Module } from 'types/modules'; +import type { Module, ModuleCode } from 'types/modules'; import type { EntryPointComponentProps } from 'views/routes/types'; import type { JSResource } from 'utils/JSResource'; @@ -15,18 +14,19 @@ import ErrorBoundary from 'views/errors/ErrorBoundary'; import ModulePageContent from './ModulePageContent'; +// TODO: Can we dedupe PreparedProps definitions with those in *.entrypoint.ts? type PreparedProps = { module: JSResource; + moduleCode: ModuleCode; }; type Props = EntryPointComponentProps; -const getPropsFromParams = (params: Params) => ({ - moduleCode: (params.moduleCode ?? '').toUpperCase(), -}); - // TODO: Generalize this smart error fallback for other error boundaries -const ErrorFallback: React.FC<{ error: Error; moduleCode: string }> = ({ error, moduleCode }) => { +const ErrorFallback: React.FC<{ + error: Error; + moduleCode: ModuleCode; +}> = ({ error, moduleCode }) => { if (get(error, ['response', 'status'], 200) === 404) { return ; } @@ -51,20 +51,17 @@ const ErrorFallback: React.FC<{ error: Error; moduleCode: string }> = ({ error, * - Loading: Either requests are pending * - Loaded: Both requests are successfully loaded */ -export const ModulePageContainerComponent: React.FC = ({ prepared: { module } }) => { +export const ModulePageContainerComponent: React.FC = ({ + prepared: { module, moduleCode }, +}) => { const location = useLocation(); const navigate = useNavigate(); - const params = useParams(); - const { moduleCode } = getPropsFromParams(params); - useEffect(() => { // Navigate to canonical URL - if (module.get() && location.pathname !== modulePage(moduleCode, module.get()?.title)) { - navigate( - { ...location, pathname: modulePage(moduleCode, module.get()?.title) }, - { replace: true }, - ); + const canonicalUrl = modulePage(moduleCode, module.get()?.title); + if (module.get() && location.pathname !== canonicalUrl) { + navigate({ ...location, pathname: canonicalUrl }, { replace: true }); } }, [location, module, moduleCode, navigate]); diff --git a/website/src/views/routes/Routes.tsx b/website/src/views/routes/Routes.tsx index 4799b24aa1..9c8ea55424 100644 --- a/website/src/views/routes/Routes.tsx +++ b/website/src/views/routes/Routes.tsx @@ -9,11 +9,11 @@ import timetableEntryPoint from 'views/timetable/Timetable.entrypoint'; import todayEntryPoint from 'views/today/Today.entrypoint'; import moduleFinderEntryPoint from 'views/modules/ModuleFinder.entrypoint'; import modulePageEntryPoint from 'views/modules/ModulePage.entrypoint'; +import moduleArchiveEntryPoint from 'views/modules/ModuleArchive.entrypoint'; import type { Dispatch } from 'types/redux'; import type { EntryPoint, EntryPointPartialRouteObject, EntryPointRouteObject } from './types'; -// import ModuleArchiveContainer from 'views/modules/ModuleArchiveContainer'; // import VenuesContainer from 'views/venues/VenuesContainer'; // import SettingsContainer from 'views/settings/SettingsContainer'; // import AboutContainer from 'views/static/AboutContainer'; @@ -43,7 +43,6 @@ function entryPointRoute( }; } -// // // today // @@ -78,17 +77,39 @@ function createPartialRoutes(dispatch: Dispatch): EntryPointPartialRouteObject[] { ...entryPointRoute(appShellEntryPoint, dispatch), children: [ - { path: '/', element: }, + { + path: '/', + element: , + }, { path: '/timetable', children: [ - { path: ':semester/*', ...entryPointRoute(timetableEntryPoint, dispatch) }, - { path: '/', element: }, + { + path: ':semester/*', + ...entryPointRoute(timetableEntryPoint, dispatch), + }, + { + path: '/', + element: , + }, ], }, - { path: '/today', ...entryPointRoute(todayEntryPoint, dispatch) }, - { path: '/modules', ...entryPointRoute(moduleFinderEntryPoint, dispatch) }, - { path: '/modules/:moduleCode/*', ...entryPointRoute(modulePageEntryPoint, dispatch) }, + { + path: '/today', + ...entryPointRoute(todayEntryPoint, dispatch), + }, + { + path: '/modules', + ...entryPointRoute(moduleFinderEntryPoint, dispatch), + }, + { + path: '/modules/:moduleCode/*', + ...entryPointRoute(modulePageEntryPoint, dispatch), + }, + { + path: '/archive/:moduleCode/:archiveYear/*', + ...entryPointRoute(moduleArchiveEntryPoint, dispatch), + }, // 404 page { From cab0b55a5de5a0d83c9c7d1fec1ffdf4719eee33 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Sat, 24 Oct 2020 17:29:38 +0800 Subject: [PATCH 14/20] Replace JSResource with DevTools Resource implementation for data loading --- website/src/utils/JSResource.ts | 110 ++++++--- website/src/utils/Resource.ts | 218 ++++++++++++++++++ website/src/views/AppShell.entrypoint.ts | 40 +++- website/src/views/AppShell.tsx | 8 +- .../components/map/venueLocationResource.ts | 8 +- .../views/modules/ModuleArchive.entrypoint.ts | 27 ++- .../views/modules/ModuleArchiveContainer.tsx | 4 +- .../views/modules/ModuleFinder.entrypoint.ts | 4 +- .../views/modules/ModulePage.entrypoint.ts | 32 +-- .../src/views/modules/ModulePageContainer.tsx | 4 +- .../src/views/modules/ModulePageContent.tsx | 4 +- website/src/views/routes/types.ts | 4 +- .../views/timetable/Timetable.entrypoint.ts | 4 +- website/src/views/today/Today.entrypoint.ts | 6 +- 14 files changed, 376 insertions(+), 97 deletions(-) create mode 100644 website/src/utils/Resource.ts diff --git a/website/src/utils/JSResource.ts b/website/src/utils/JSResource.ts index eb335c60d9..c20c0b88d3 100644 --- a/website/src/utils/JSResource.ts +++ b/website/src/utils/JSResource.ts @@ -1,5 +1,39 @@ -type Loadable = Result | { default: Result }; -type Loader = () => Promise>; +type Module = { default: T }; +type Loader = () => Promise>; + +export interface JSResourceReference { + /** + * Returns the module's id + */ + getModuleId(): string; + + /** + * Gets a module if it is already loaded, undefined otherwise. + */ + getModuleIfRequired(): T | undefined; + + /** + * Loads the resource if necessary + */ + preload(): Promise; + + preloadOrReloadIfError(): void; + + /** + * This is the key method for integrating with React Suspense. Read will: + * - "Suspend" if the resource is still pending (currently implemented as + * throwing a Promise, though this is subject to change in future + * versions of React) + * - Throw an error if the resource failed to load. + * - Return the data of the resource if available. + */ + read(): T; + + /** + * Convenience function that preloads and reads this resource. + */ + fetch(): T; +} /** * A cache of resources to avoid loading the same module twice. This is important @@ -7,32 +41,38 @@ type Loader = () => Promise>; * modules, so to be able to access already-loaded modules synchronously we * must have stored the previous result somewhere. */ -const resourceMap = new Map(); +const resourceMap = new Map>(); /** - * A generic resource: given some method to asynchronously load a value - the loader() - * argument - it allows accessing the state of the resource. + * A generic resource: given some method to asynchronously load a value -- the + * loader() argument -- it allows accessing the state of the resource. + * + * The main differences between this and `Resource` are: + * - `JSResourceImpl` returns a promise on preload. + * - `Resource` allows a single resource to load values for different keys. */ -class Resource { - private error: Error | null = null; +class JSResourceImpl implements JSResourceReference { + private error: Error | undefined; + + private promise: Promise | undefined; - private promise: Promise | null = null; + private result: T | undefined; - private result: Result | null = null; + private moduleId: string; - private loader: Loader; + private loader: Loader; - constructor(loader: Loader) { + constructor(moduleId: string, loader: Loader) { + this.moduleId = moduleId; this.loader = loader; } - // TODO: Check if we should do this or implement a way to replace the resource - // instance entirely. I suspect this current approach will not trigger - // renders. - reset() { - this.error = null; - this.promise = null; - this.result = null; + getModuleId() { + return this.moduleId; + } + + getModuleIfRequired() { + return this.result; } /** @@ -42,15 +82,9 @@ class Resource { let { promise } = this; if (promise == null) { promise = this.loader() - .then((result) => { - let unmoduledResult: Result; - if (result.default) { - unmoduledResult = result.default; - } else { - unmoduledResult = result; - } - this.result = unmoduledResult; - return unmoduledResult; + .then(({ default: result }) => { + this.result = result; + return result; }) .catch((error) => { this.error = error; @@ -62,8 +96,10 @@ class Resource { } preloadOrReloadIfError() { - if (this.error !== null) { - this.reset(); + if (this.error !== undefined) { + this.error = undefined; + this.promise = undefined; + this.result = undefined; } this.preload(); } @@ -73,7 +109,7 @@ class Resource { * is resolved yet. */ get() { - if (this.result != null) { + if (this.result !== undefined) { return this.result; } return undefined; @@ -88,13 +124,13 @@ class Resource { * - Return the data of the resource if available. */ read() { - if (this.result !== null) { + if (this.result !== undefined) { return this.result; } - if (this.error !== null) { + if (this.error !== undefined) { throw this.error; } - if (this.promise === null) { + if (this.promise === undefined) { throw new Error('preload() must be called before read().'); } throw this.promise; @@ -106,8 +142,6 @@ class Resource { } } -export type JSResource = Resource; - /** * A helper method to create a resource, intended for dynamically loading code. * @@ -125,11 +159,11 @@ export type JSResource = Resource; * @param {*} moduleId A globally unique identifier for the resource used for caching * @param {*} loader A method to load the resource's data if necessary */ -export function JSResource(moduleId: unknown, loader: Loader): JSResource { +export function JSResource(moduleId: string, loader: Loader): JSResourceReference { let resource = resourceMap.get(moduleId); if (resource == null) { - resource = new Resource(loader); + resource = new JSResourceImpl(moduleId, loader); resourceMap.set(moduleId, resource); } - return resource; + return resource as JSResourceReference; } diff --git a/website/src/utils/Resource.ts b/website/src/utils/Resource.ts new file mode 100644 index 0000000000..cc1bf9487b --- /dev/null +++ b/website/src/utils/Resource.ts @@ -0,0 +1,218 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import * as React from 'react'; +import { createContext } from 'react'; + +// Cache implementation was forked from the React repo: +// https://github.com/facebook/react/blob/4e5d7faf54b38ebfc7a2dcadbd09a25d6f330ac0/packages/react-devtools-shared/src/devtools/cache.js +// which was forked from: +// https://github.com/facebook/react/blob/4e5d7faf54b38ebfc7a2dcadbd09a25d6f330ac0/packages/react-cache/src/ReactCacheOld.js +// +// This cache is simpler than react-cache in that: +// 1. Individual items don't need to be invalidated. +// 2. We didn't need the added overhead of an LRU cache. + +type PendingResult = { + status: 0; + value: Promise; +}; + +type ResolvedResult = { + status: 1; + value: Value; +}; + +type RejectedResult = { + status: 2; + value: unknown; +}; + +type Result = PendingResult | ResolvedResult | RejectedResult; + +// TODO: Reduce API surface area? +export type Resource = { + clear(): void; + invalidate(key: Key): void; + + /** + * Returns the result, if available. This can be useful to check if the value + * is resolved yet. + */ + get(input: Input): Value | undefined; + + /** + * This is the key method for integrating with React Suspense. Read will: + * - "Suspend" if the resource is still pending (currently implemented as + * throwing a Promise, though this is subject to change in future + * versions of React) + * - Throw an error if the resource failed to load. + * - Return the data of the resource if available. + */ + read(input: Input): Value; + + /** + * Loads the resource if necessary. + */ + preload(input: Input): void; + + write(key: Key, value: Value): void; +}; + +const Pending = 0; +const Resolved = 1; +const Rejected = 2; + +// TODO: Decide if we should dump this as it's only used for a dev check +const { ReactCurrentDispatcher } = + // eslint-disable-next-line no-underscore-dangle + (React as any).__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED; + +function readContext(Context: React.Context, observedBits: void | number | boolean) { + const dispatcher = ReactCurrentDispatcher.current; + if (dispatcher === null) { + throw new Error( + 'Resource.ts: read and preload may only be called from within a ' + + "component's render. They are not supported in event handlers or " + + 'lifecycle methods.', + ); + } + return dispatcher.readContext(Context, observedBits); +} + +const CacheContext = createContext(null); + +type Config = { + useWeakMap?: boolean; +}; + +const entries: Map, Map | WeakMap> = new Map(); +const resourceConfigs: Map, Config> = new Map(); + +function getEntriesForResource(resource: any): Map | WeakMap { + let entriesForResource = (entries.get(resource) as any) as Map | WeakMap; + if (entriesForResource === undefined) { + const config = resourceConfigs.get(resource); + entriesForResource = config !== undefined && config.useWeakMap ? new WeakMap() : new Map(); + entries.set(resource, entriesForResource); + } + return entriesForResource; +} + +function accessResult( + resource: any, + fetch: (fetchInput: Input) => Promise, + input: Input, + key: Key, +): Result { + const entriesForResource = getEntriesForResource(resource); + const entry = entriesForResource.get(key); + + if (entry === undefined) { + const thenable = fetch(input); + thenable.then( + (value) => { + if (newResult.status === Pending) { + const resolvedResult: ResolvedResult = newResult as any; + resolvedResult.status = Resolved; + resolvedResult.value = value; + } + }, + (error) => { + if (newResult.status === Pending) { + const rejectedResult: RejectedResult = newResult as any; + rejectedResult.status = Rejected; + rejectedResult.value = error; + } + }, + ); + const newResult: PendingResult = { + status: Pending, + value: thenable, + }; + entriesForResource.set(key, newResult); + return newResult; + } + + return entry; +} + +export function createResource( + fetch: (input: Input) => Promise, + hashInput: (input: Input) => Key, + config: Config = {}, +): Resource { + const resource = { + clear(): void { + entries.delete(resource); + }, + + invalidate(key: Key): void { + const entriesForResource = getEntriesForResource(resource); + entriesForResource.delete(key); + }, + + get(input: Input): Value | undefined { + const key = hashInput(input); + const result: Result = accessResult(resource, fetch, input, key); + switch (result.status) { + case Resolved: { + const { value } = result; + return value; + } + default: + return undefined; + } + }, + + read(input: Input): Value { + // Prevent access outside of render. + readContext(CacheContext); + + const key = hashInput(input); + const result: Result = accessResult(resource, fetch, input, key); + switch (result.status) { + case Pending: { + const suspender = result.value; + throw suspender; + } + case Resolved: { + const { value } = result; + return value; + } + case Rejected: { + const error = result.value; + throw error; + } + default: + // Should be unreachable + return undefined as any; + } + }, + + preload(input: Input): void { + // Prevent access outside of render. + readContext(CacheContext); + + const key = hashInput(input); + accessResult(resource, fetch, input, key); + }, + + write(key: Key, value: Value): void { + const entriesForResource = getEntriesForResource(resource); + + const resolvedResult: ResolvedResult = { + status: Resolved, + value, + }; + + entriesForResource.set(key, resolvedResult); + }, + }; + + resourceConfigs.set(resource, config); + + return resource; +} + +export function invalidateResources(): void { + entries.clear(); +} diff --git a/website/src/views/AppShell.entrypoint.ts b/website/src/views/AppShell.entrypoint.ts index 6183e9c0ed..7ca48318e2 100644 --- a/website/src/views/AppShell.entrypoint.ts +++ b/website/src/views/AppShell.entrypoint.ts @@ -1,30 +1,46 @@ import { captureException } from '@sentry/browser'; import { fetchModuleList } from 'actions/moduleBank'; import { JSResource } from 'utils/JSResource'; -import { EntryPoint } from 'views/routes/EntryPointContainer'; +import { Resource, createResource } from 'utils/Resource'; +import type { EntryPoint } from 'views/routes/types'; export type PreparedProps = { - moduleList: JSResource; + moduleList: Resource; + moduleListPromise: Promise; }; +// HACK: Cache the promise so that we can feed a constant value to to +// fetch timetable modules after timetables have been fetched from localStorage. +// Typed as unknown because we don't actually need the output +let cachedModuleListPromise: Promise; + const entryPoint: EntryPoint = { component: JSResource( 'AppShell', - () => import(/* webpackChunkName: "AppShell.route" */ 'views/AppShell'), + () => import(/* webpackChunkName: "AppShell" */ 'views/AppShell'), ), prepare(_params, dispatch) { - const moduleList = JSResource('moduleList', async () => { - try { - // Typed as unknown because we don't actually need the output - return (dispatch(fetchModuleList()) as unknown) as Promise; - } catch (error) { - captureException(error); - throw error; - } - }); + const moduleList = createResource( + () => { + if (!cachedModuleListPromise) { + // TODO: Defer to an idle callback? + cachedModuleListPromise = (async () => { + try { + return (dispatch(fetchModuleList()) as unknown) as Promise; + } catch (error) { + captureException(error); + throw error; + } + })(); + } + return cachedModuleListPromise; + }, + () => 'moduleList', + ); moduleList.preload(); return { moduleList, + moduleListPromise: cachedModuleListPromise, }; }, }; diff --git a/website/src/views/AppShell.tsx b/website/src/views/AppShell.tsx index 16cfc8bb14..a9c56f458d 100644 --- a/website/src/views/AppShell.tsx +++ b/website/src/views/AppShell.tsx @@ -29,7 +29,7 @@ import { trackPageView } from 'bootstrapping/matomo'; import { isIOS } from 'bootstrapping/browser'; import Logo from 'img/nusmods-logo.svg'; import { State as StoreState } from 'types/state'; -import type { JSResource } from 'utils/JSResource'; +import type { Resource } from 'utils/Resource'; import LoadingSpinner from './components/LoadingSpinner'; import FeedbackModal from './components/FeedbackModal'; import KeyboardShortcuts from './components/KeyboardShortcuts'; @@ -41,7 +41,8 @@ type Props = { // From router prepared: { - moduleList: JSResource; + moduleList: Resource; + moduleListPromise: Promise; }; // From Redux state @@ -103,9 +104,10 @@ export const AppShellComponent: React.FC = ({ () => { // Fetch the module data of the existing modules in the timetable and ensure all // lessons are filled + // TODO: Defer to an idle callback? each(timetables, (timetable, semesterString) => { const semester = Number(semesterString); - prepared.moduleList.preload().then(() => { + prepared.moduleListPromise.then(() => { // Wait for module list to be fetched before trying to fetch timetable modules // TODO: There may be a more optimal way to do this fetchTimetableModules(timetable, semester); diff --git a/website/src/views/components/map/venueLocationResource.ts b/website/src/views/components/map/venueLocationResource.ts index 08e9a2db38..a89b99acf2 100644 --- a/website/src/views/components/map/venueLocationResource.ts +++ b/website/src/views/components/map/venueLocationResource.ts @@ -1,6 +1,10 @@ +import { createResource } from 'utils/Resource'; import { getVenueLocations } from 'apis/github'; -import { JSResource } from 'utils/JSResource'; +import type { VenueLocationMap } from 'types/venues'; // FIXME: Disable getVenueLocations' built in promise memoization as it's // already memoized in Resource. Breaks resource reloads. -export default JSResource('venueLocationResource', () => getVenueLocations()); +export default createResource( + () => getVenueLocations(), + () => 'venueLocationResource', +); diff --git a/website/src/views/modules/ModuleArchive.entrypoint.ts b/website/src/views/modules/ModuleArchive.entrypoint.ts index ed13e33f10..e68a6d36c3 100644 --- a/website/src/views/modules/ModuleArchive.entrypoint.ts +++ b/website/src/views/modules/ModuleArchive.entrypoint.ts @@ -1,12 +1,13 @@ import type { Params } from 'react-router'; import { fetchModuleArchive } from 'actions/moduleBank'; import { captureException } from 'utils/error'; +import { Resource, createResource } from 'utils/Resource'; import { JSResource } from 'utils/JSResource'; import type { Module, ModuleCode } from 'types/modules'; import type { EntryPoint } from 'views/routes/types'; export type PreparedProps = { - module: JSResource; + module: Resource; moduleCode: ModuleCode; archiveYear: string; }; @@ -25,21 +26,23 @@ const getPropsFromParams = (params: Params) => { */ const entryPoint: EntryPoint = { component: JSResource( - 'ModuleArchive', - () => import(/* webpackChunkName: "ModuleArchive.route" */ './ModuleArchiveContainer'), + 'ModuleArchiveContainer', + () => import(/* webpackChunkName: "ModuleArchiveContainer" */ './ModuleArchiveContainer'), ), prepare(params, dispatch) { const { moduleCode, archiveYear } = getPropsFromParams(params); - const module = JSResource(`ModuleArchiveContainer-module-${moduleCode}-${archiveYear}`, () => - dispatch(fetchModuleArchive(moduleCode, archiveYear)).catch((error) => { - captureException(error); - // TODO: If there is an error but module data can still be found, we - // can assume module has been loaded at some point, so we can just show - // that instead - throw error; - }), + const module = createResource( + () => + dispatch(fetchModuleArchive(moduleCode, archiveYear)).catch((error) => { + captureException(error); + // TODO: If there is an error but module data can still be found, we + // can assume module has been loaded at some point, so we can just show + // that instead + throw error; + }), + () => `ModuleArchiveContainer-module-${moduleCode}-${archiveYear}`, ); - module.preloadOrReloadIfError(); + module.preload(); return { module, moduleCode, diff --git a/website/src/views/modules/ModuleArchiveContainer.tsx b/website/src/views/modules/ModuleArchiveContainer.tsx index 3dc88431b7..7e79e04b8e 100644 --- a/website/src/views/modules/ModuleArchiveContainer.tsx +++ b/website/src/views/modules/ModuleArchiveContainer.tsx @@ -4,7 +4,7 @@ import { get } from 'lodash'; import type { Module, ModuleCode } from 'types/modules'; import type { EntryPointComponentProps } from 'views/routes/types'; -import type { JSResource } from 'utils/JSResource'; +import type { Resource } from 'utils/Resource'; import ApiError from 'views/errors/ApiError'; import ModuleNotFoundPage from 'views/errors/ModuleNotFoundPage'; @@ -15,7 +15,7 @@ import ErrorBoundary from 'views/errors/ErrorBoundary'; import ModulePageContent from './ModulePageContent'; type PreparedProps = { - module: JSResource; + module: Resource; moduleCode: ModuleCode; archiveYear: string; }; diff --git a/website/src/views/modules/ModuleFinder.entrypoint.ts b/website/src/views/modules/ModuleFinder.entrypoint.ts index 6ed615f511..63817a292c 100644 --- a/website/src/views/modules/ModuleFinder.entrypoint.ts +++ b/website/src/views/modules/ModuleFinder.entrypoint.ts @@ -5,8 +5,8 @@ export type PreparedProps = unknown; const entryPoint: EntryPoint = { component: JSResource( - 'ModuleFinder', - () => import(/* webpackChunkName: "ModuleFinder.route" */ './ModuleFinderContainer'), + 'ModuleFinderContainer', + () => import(/* webpackChunkName: "ModuleFinderContainer" */ './ModuleFinderContainer'), ), prepare() { return {}; diff --git a/website/src/views/modules/ModulePage.entrypoint.ts b/website/src/views/modules/ModulePage.entrypoint.ts index 52c1c7cfa2..a4ebe28aa5 100644 --- a/website/src/views/modules/ModulePage.entrypoint.ts +++ b/website/src/views/modules/ModulePage.entrypoint.ts @@ -1,12 +1,13 @@ import type { Params } from 'react-router'; import { fetchModule } from 'actions/moduleBank'; import { captureException } from 'utils/error'; +import { Resource, createResource } from 'utils/Resource'; import { JSResource } from 'utils/JSResource'; import type { Module, ModuleCode } from 'types/modules'; import type { EntryPoint } from 'views/routes/types'; export type PreparedProps = { - module: JSResource; + module: Resource; moduleCode: ModuleCode; }; @@ -16,24 +17,25 @@ const getPropsFromParams = (params: Params) => ({ const entryPoint: EntryPoint = { component: JSResource( - 'ModulePage', - () => import(/* webpackChunkName: "ModulePage.route" */ './ModulePageContainer'), + 'ModulePageContainer', + () => import(/* webpackChunkName: "ModulePageContainer" */ './ModulePageContainer'), ), prepare(params, dispatch) { const { moduleCode } = getPropsFromParams(params); - const module = JSResource(`ModulePageContainer-module-${moduleCode}`, () => - dispatch(fetchModule(moduleCode)).catch((error) => { - captureException(error); - // TODO: If there is an error but module data can still be found, we - // can assume module has been loaded at some point, so we can just show - // that instead - throw error; - }), + const module = createResource( + () => + dispatch(fetchModule(moduleCode)).catch((error) => { + captureException(error); + // TODO: If there is an error but module data can still be found, we + // can assume module has been loaded at some point, so we can just show + // that instead + throw error; + }), + () => `ModulePageContainer-module-${moduleCode}`, ); - // TODO: If the resource doesn't exist, we're looking at spraying many - // requests for the same 404 resource because `prepare` may be called - // multiple times on a single navigation. - module.preloadOrReloadIfError(); + // TODO: Temp failures will prevent the module from ever being loaded, until + // a reload. + module.preload(); return { module, moduleCode, diff --git a/website/src/views/modules/ModulePageContainer.tsx b/website/src/views/modules/ModulePageContainer.tsx index 8617eeabf6..f3c001e7d0 100644 --- a/website/src/views/modules/ModulePageContainer.tsx +++ b/website/src/views/modules/ModulePageContainer.tsx @@ -4,7 +4,7 @@ import { get } from 'lodash'; import type { Module, ModuleCode } from 'types/modules'; import type { EntryPointComponentProps } from 'views/routes/types'; -import type { JSResource } from 'utils/JSResource'; +import type { Resource } from 'utils/Resource'; import ApiError from 'views/errors/ApiError'; import ModuleNotFoundPage from 'views/errors/ModuleNotFoundPage'; @@ -16,7 +16,7 @@ import ModulePageContent from './ModulePageContent'; // TODO: Can we dedupe PreparedProps definitions with those in *.entrypoint.ts? type PreparedProps = { - module: JSResource; + module: Resource; moduleCode: ModuleCode; }; diff --git a/website/src/views/modules/ModulePageContent.tsx b/website/src/views/modules/ModulePageContent.tsx index 55472c5641..9267571fa4 100644 --- a/website/src/views/modules/ModulePageContent.tsx +++ b/website/src/views/modules/ModulePageContent.tsx @@ -29,13 +29,13 @@ import Title from 'views/components/Title'; import ScrollToTop from 'views/components/ScrollToTop'; import { Archive, Check } from 'react-feather'; import ErrorBoundary from 'views/errors/ErrorBoundary'; -import type { JSResource } from 'utils/JSResource'; +import type { Resource } from 'utils/Resource'; import styles from './ModulePageContent.scss'; import ReportError from './ReportError'; export type Props = { - module: Module | JSResource; + module: Module | Resource; archiveYear?: string; }; diff --git a/website/src/views/routes/types.ts b/website/src/views/routes/types.ts index 0c05bb7396..ccf4eb6dda 100644 --- a/website/src/views/routes/types.ts +++ b/website/src/views/routes/types.ts @@ -1,5 +1,5 @@ import type { Params, PartialRouteObject, RouteObject } from 'react-router'; -import type { JSResource } from 'utils/JSResource'; +import type { JSResourceReference } from 'utils/JSResource'; import type { Dispatch } from 'types/redux'; export type EntryPointComponentProps = React.PropsWithChildren<{ @@ -8,7 +8,7 @@ export type EntryPointComponentProps = React.PropsWithChildren<{ }>; export type EntryPoint> = { - component: JSResource>; + component: JSResourceReference>; prepare: (matchParams: Params, dispatch: Dispatch) => PreparedProps; }; diff --git a/website/src/views/timetable/Timetable.entrypoint.ts b/website/src/views/timetable/Timetable.entrypoint.ts index 4f8c90e6e1..14405f2e3e 100644 --- a/website/src/views/timetable/Timetable.entrypoint.ts +++ b/website/src/views/timetable/Timetable.entrypoint.ts @@ -3,8 +3,8 @@ import { EntryPoint } from 'views/routes/types'; const entryPoint: EntryPoint = { component: JSResource( - 'Timetable', - () => import(/* webpackChunkName: "Timetable.route" */ 'views/timetable/TimetableContainer'), + 'TimetableContainer', + () => import(/* webpackChunkName: "TimetableContainer" */ 'views/timetable/TimetableContainer'), ), prepare() { return {}; diff --git a/website/src/views/today/Today.entrypoint.ts b/website/src/views/today/Today.entrypoint.ts index aa845e0cf5..3fbe82540d 100644 --- a/website/src/views/today/Today.entrypoint.ts +++ b/website/src/views/today/Today.entrypoint.ts @@ -6,11 +6,11 @@ export type PreparedProps = unknown; const entryPoint: EntryPoint = { component: JSResource( - 'Today', - () => import(/* webpackChunkName: "Today.route" */ './TodayContainer'), + 'TodayContainer', + () => import(/* webpackChunkName: "TodayContainer" */ './TodayContainer'), ), prepare() { - venueLocationResource.preloadOrReloadIfError(); + venueLocationResource.preload(); }, }; From b96632646eaa6b66e5c2e5899b623181e0ced05b Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Mon, 26 Oct 2020 14:10:20 +0800 Subject: [PATCH 15/20] Minor code tweaks --- website/package.json | 5 +---- .../src/views/routes/EntryPointContainer.tsx | 2 +- website/src/views/routes/PreloadingLink.tsx | 2 +- website/src/views/routes/types.ts | 1 - .../src/views/timetable/TimetableContainer.tsx | 4 ++-- website/yarn.lock | 17 ----------------- 6 files changed, 5 insertions(+), 26 deletions(-) diff --git a/website/package.json b/website/package.json index 487c6dcdc3..968b16a5ed 100644 --- a/website/package.json +++ b/website/package.json @@ -58,7 +58,6 @@ "@types/react-redux": "7.1.9", "@types/react-scrollspy": "3.3.3", "@types/redux-mock-store": "1.0.2", - "@types/use-subscription": "^1.0.0", "@types/webpack-env": "1.15.3", "@typescript-eslint/eslint-plugin": "4.5.0", "@typescript-eslint/parser": "4.5.0", @@ -171,9 +170,7 @@ "redux-persist": "6.0.0", "redux-thunk": "2.3.0", "reselect": "4.0.0", - "resolve-pathname": "^3.0.0", - "searchkit": "2.4.1-alpha.5", - "use-subscription": "^0.0.0-experimental-4ead6b530" + "searchkit": "2.4.1-alpha.5" }, "browserslist": [ "extends browserslist-config-nusmods" diff --git a/website/src/views/routes/EntryPointContainer.tsx b/website/src/views/routes/EntryPointContainer.tsx index befad800c3..29d9ad3184 100644 --- a/website/src/views/routes/EntryPointContainer.tsx +++ b/website/src/views/routes/EntryPointContainer.tsx @@ -14,7 +14,7 @@ const EntryPointContainer: React.FC = ({ entryPoint }) => { const dispatch = useDispatch(); // TODO: Figure out a way to prepare outside render. Should reuse prepare result from preload return ( - + ); diff --git a/website/src/views/routes/PreloadingLink.tsx b/website/src/views/routes/PreloadingLink.tsx index ce5fa62407..c453438c06 100644 --- a/website/src/views/routes/PreloadingLink.tsx +++ b/website/src/views/routes/PreloadingLink.tsx @@ -34,7 +34,7 @@ export const PreloadingLink = forwardRef( }, ); -export const PreloadingNavLink = forwardRef( +export const PreloadingNavLink = forwardRef( function PreloadingNavLinkComponent(props, ref) { const preloadCallbacks = usePreloadCallbacks(props); return ; diff --git a/website/src/views/routes/types.ts b/website/src/views/routes/types.ts index ccf4eb6dda..bc121297f7 100644 --- a/website/src/views/routes/types.ts +++ b/website/src/views/routes/types.ts @@ -3,7 +3,6 @@ import type { JSResourceReference } from 'utils/JSResource'; import type { Dispatch } from 'types/redux'; export type EntryPointComponentProps = React.PropsWithChildren<{ - params: Params; prepared: PreparedProps; }>; diff --git a/website/src/views/timetable/TimetableContainer.tsx b/website/src/views/timetable/TimetableContainer.tsx index 5f759f5a19..885b25d080 100644 --- a/website/src/views/timetable/TimetableContainer.tsx +++ b/website/src/views/timetable/TimetableContainer.tsx @@ -30,7 +30,7 @@ import TimetableContent from './TimetableContent'; import styles from './TimetableContainer.scss'; export type QueryParam = { - action: string; + '*': string | null; // action semester: string; }; @@ -45,7 +45,7 @@ type Props = EntryPointComponentProps; export const TimetableContainerComponent: React.FC = () => { const navigate = useNavigate(); const location = useLocation(); - const params = useParams(); + const params = useParams() as QueryParam; const action = params['*']; const semester = useMemo(() => semesterForTimetablePage(params.semester), [params.semester]); diff --git a/website/yarn.lock b/website/yarn.lock index 92cb6b05d6..9008b81a81 100644 --- a/website/yarn.lock +++ b/website/yarn.lock @@ -3280,11 +3280,6 @@ resolved "https://registry.yarnpkg.com/@types/unist/-/unist-2.0.3.tgz#9c088679876f374eb5983f150d4787aa6fb32d7e" integrity sha512-FvUupuM3rlRsRtCN+fDudtmytGO6iHJuuRKS1Ss0pG5z8oX0diNEw94UEL7hgDbpN94rgaK5R7sWm6RrSkZuAQ== -"@types/use-subscription@^1.0.0": - version "1.0.0" - resolved "https://registry.yarnpkg.com/@types/use-subscription/-/use-subscription-1.0.0.tgz#d146f8d834f70f50d48bd8246a481d096f11db19" - integrity sha512-0WWZ5GUDKMXUY/1zy4Ur5/zsC0s/B+JjXfHdkvx6JgDNZzZV5eW+KKhDqsTGyqX56uh99gwGwbsKbVwkcVIKQA== - "@types/webpack-env@1.15.3": version "1.15.3" resolved "https://registry.yarnpkg.com/@types/webpack-env/-/webpack-env-1.15.3.tgz#fb602cd4c2f0b7c0fb857e922075fdf677d25d84" @@ -13600,11 +13595,6 @@ resolve-pathname@^2.2.0: resolved "https://registry.yarnpkg.com/resolve-pathname/-/resolve-pathname-2.2.0.tgz#7e9ae21ed815fd63ab189adeee64dc831eefa879" integrity sha512-bAFz9ld18RzJfddgrO2e/0S2O81710++chRMUxHjXOYKF6jTAMrUNZrEZ1PvV0zlhfjidm08iRPdTLPno1FuRg== -resolve-pathname@^3.0.0: - version "3.0.0" - resolved "https://registry.yarnpkg.com/resolve-pathname/-/resolve-pathname-3.0.0.tgz#99d02224d3cf263689becbb393bc560313025dcd" - integrity sha512-C7rARubxI8bXFNB/hqcp/4iUeIXJhJZvFPFPiSPRnhU5UPxzMFIl+2E6yY6c4k9giDJAhtV+enfA+G89N6Csng== - resolve-url@^0.2.1: version "0.2.1" resolved "https://registry.yarnpkg.com/resolve-url/-/resolve-url-0.2.1.tgz#2c637fe77c893afd2a663fe21aa9080068e2052a" @@ -15715,13 +15705,6 @@ use-memo-one@^1.1.1: resolved "https://registry.yarnpkg.com/use-memo-one/-/use-memo-one-1.1.1.tgz#39e6f08fe27e422a7d7b234b5f9056af313bd22c" integrity sha512-oFfsyun+bP7RX8X2AskHNTxu+R3QdE/RC5IefMbqptmACAA/gfol1KDD5KRzPsGMa62sWxGZw+Ui43u6x4ddoQ== -use-subscription@^0.0.0-experimental-4ead6b530: - version "0.0.0-experimental-4ead6b530" - resolved "https://registry.yarnpkg.com/use-subscription/-/use-subscription-0.0.0-experimental-4ead6b530.tgz#94c611419a6c945b17f9926e94beaf9af148f81e" - integrity sha512-vdCAmupdX9iWfyNsBE6nLYH2XDu84RAGSRVzKJaVYaLOFd1LYHSBiZn7l5JPwxIrmP+Asbie5plTi1k+KYQSuQ== - dependencies: - object-assign "^4.1.1" - use@^3.1.0: version "3.1.1" resolved "https://registry.yarnpkg.com/use/-/use-3.1.1.tgz#d50c8cac79a19fbc20f2911f56eb973f4e10070f" From b6a1014e215cd98c35a9d79366b94c7ccdb9a11d Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Mon, 26 Oct 2020 17:53:49 +0800 Subject: [PATCH 16/20] Implement EntryPoint.disposePreparedProps; preferentially use Redux data in module pages --- website/src/utils/Resource.ts | 32 ++----------- website/src/views/AppShell.entrypoint.ts | 27 ++++++----- website/src/views/hooks/useMemoCompare.ts | 26 +++++++++++ .../views/modules/ModuleArchive.entrypoint.ts | 46 +++++++++++++------ .../views/modules/ModuleArchiveContainer.tsx | 23 ++++++++-- .../views/modules/ModuleFinder.entrypoint.ts | 2 +- .../views/modules/ModulePage.entrypoint.ts | 43 ++++++++++------- .../src/views/modules/ModulePageContainer.tsx | 20 ++++++-- .../src/views/modules/ModulePageContent.tsx | 22 ++++----- .../src/views/routes/EntryPointContainer.tsx | 21 +++++++-- website/src/views/routes/Routes.tsx | 2 +- website/src/views/routes/types.ts | 15 +++++- .../views/timetable/Timetable.entrypoint.ts | 2 +- website/src/views/today/EventMap.tsx | 2 +- website/src/views/today/EventMapInline.tsx | 2 +- website/src/views/today/Today.entrypoint.ts | 3 +- 16 files changed, 182 insertions(+), 106 deletions(-) create mode 100644 website/src/views/hooks/useMemoCompare.ts diff --git a/website/src/utils/Resource.ts b/website/src/utils/Resource.ts index cc1bf9487b..3171e5d0e8 100644 --- a/website/src/utils/Resource.ts +++ b/website/src/utils/Resource.ts @@ -1,6 +1,4 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import * as React from 'react'; -import { createContext } from 'react'; // Cache implementation was forked from the React repo: // https://github.com/facebook/react/blob/4e5d7faf54b38ebfc7a2dcadbd09a25d6f330ac0/packages/react-devtools-shared/src/devtools/cache.js @@ -31,7 +29,7 @@ type Result = PendingResult | ResolvedResult | RejectedResult; // TODO: Reduce API surface area? export type Resource = { clear(): void; - invalidate(key: Key): void; + invalidate(input: Input): void; /** * Returns the result, if available. This can be useful to check if the value @@ -61,25 +59,6 @@ const Pending = 0; const Resolved = 1; const Rejected = 2; -// TODO: Decide if we should dump this as it's only used for a dev check -const { ReactCurrentDispatcher } = - // eslint-disable-next-line no-underscore-dangle - (React as any).__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED; - -function readContext(Context: React.Context, observedBits: void | number | boolean) { - const dispatcher = ReactCurrentDispatcher.current; - if (dispatcher === null) { - throw new Error( - 'Resource.ts: read and preload may only be called from within a ' + - "component's render. They are not supported in event handlers or " + - 'lifecycle methods.', - ); - } - return dispatcher.readContext(Context, observedBits); -} - -const CacheContext = createContext(null); - type Config = { useWeakMap?: boolean; }; @@ -145,8 +124,9 @@ export function createResource( entries.delete(resource); }, - invalidate(key: Key): void { + invalidate(input: Input): void { const entriesForResource = getEntriesForResource(resource); + const key = hashInput(input); entriesForResource.delete(key); }, @@ -164,9 +144,6 @@ export function createResource( }, read(input: Input): Value { - // Prevent access outside of render. - readContext(CacheContext); - const key = hashInput(input); const result: Result = accessResult(resource, fetch, input, key); switch (result.status) { @@ -189,9 +166,6 @@ export function createResource( }, preload(input: Input): void { - // Prevent access outside of render. - readContext(CacheContext); - const key = hashInput(input); accessResult(resource, fetch, input, key); }, diff --git a/website/src/views/AppShell.entrypoint.ts b/website/src/views/AppShell.entrypoint.ts index 7ca48318e2..b526f2a328 100644 --- a/website/src/views/AppShell.entrypoint.ts +++ b/website/src/views/AppShell.entrypoint.ts @@ -14,32 +14,35 @@ export type PreparedProps = { // Typed as unknown because we don't actually need the output let cachedModuleListPromise: Promise; +let moduleListResource: Resource; + const entryPoint: EntryPoint = { component: JSResource( 'AppShell', () => import(/* webpackChunkName: "AppShell" */ 'views/AppShell'), ), - prepare(_params, dispatch) { - const moduleList = createResource( - () => { - if (!cachedModuleListPromise) { - // TODO: Defer to an idle callback? + getPreparedProps(_params, dispatch) { + if (!moduleListResource) { + moduleListResource = createResource( + () => { cachedModuleListPromise = (async () => { try { + // TODO: Defer to an idle callback? return (dispatch(fetchModuleList()) as unknown) as Promise; } catch (error) { captureException(error); throw error; } })(); - } - return cachedModuleListPromise; - }, - () => 'moduleList', - ); - moduleList.preload(); + return cachedModuleListPromise; + }, + () => 'moduleList', + ); + } + + moduleListResource.preload(); return { - moduleList, + moduleList: moduleListResource, moduleListPromise: cachedModuleListPromise, }; }, diff --git a/website/src/views/hooks/useMemoCompare.ts b/website/src/views/hooks/useMemoCompare.ts new file mode 100644 index 0000000000..606063c621 --- /dev/null +++ b/website/src/views/hooks/useMemoCompare.ts @@ -0,0 +1,26 @@ +import { useEffect, useRef } from 'react'; + +// Source: https://usehooks.com/useMemoCompare/ +export default function useMemoCompare( + next: T, + compare: (previousValue: T | undefined, proposedNextValue: T) => boolean, +): T { + // Ref for storing previous value + const previousRef = useRef(); + const previous = previousRef.current; + + // Pass previous and next value to compare function + // to determine whether to consider them equal. + const equal = compare(previous, next); + + // If not equal update previousRef to next value. + // We only update if not equal (or if never been set) so that this hook + // continues to return the same old value if compare keeps returning true. + useEffect(() => { + if (!equal || previousRef.current === undefined) { + previousRef.current = next; + } + }); + + return !equal || previous === undefined ? next : previous; +} diff --git a/website/src/views/modules/ModuleArchive.entrypoint.ts b/website/src/views/modules/ModuleArchive.entrypoint.ts index e68a6d36c3..6778de4f50 100644 --- a/website/src/views/modules/ModuleArchive.entrypoint.ts +++ b/website/src/views/modules/ModuleArchive.entrypoint.ts @@ -6,8 +6,14 @@ import { JSResource } from 'utils/JSResource'; import type { Module, ModuleCode } from 'types/modules'; import type { EntryPoint } from 'views/routes/types'; +type ModuleArchiveResource = Resource< + { moduleCode: ModuleCode; archiveYear: string }, + string, + Module +>; + export type PreparedProps = { - module: Resource; + moduleResource: ModuleArchiveResource; moduleCode: ModuleCode; archiveYear: string; }; @@ -20,6 +26,8 @@ const getPropsFromParams = (params: Params) => { }; }; +let moduleResource: ModuleArchiveResource; + /** * This is very similar to ModulePage.entrypoint except it is used for the * archive page, so it uses different code paths for data fetching. @@ -29,26 +37,34 @@ const entryPoint: EntryPoint = { 'ModuleArchiveContainer', () => import(/* webpackChunkName: "ModuleArchiveContainer" */ './ModuleArchiveContainer'), ), - prepare(params, dispatch) { + getPreparedProps(params, dispatch) { + if (!moduleResource) { + moduleResource = createResource( + ({ moduleCode, archiveYear }) => + dispatch(fetchModuleArchive(moduleCode, archiveYear)).catch((error) => { + captureException(error); + // TODO: If there is an error but module data can still be found, we + // can assume module has been loaded at some point, so we can just show + // that instead + throw error; + }), + ({ moduleCode, archiveYear }) => + `ModuleArchiveContainer-module-${moduleCode}-${archiveYear}`, + ); + } + const { moduleCode, archiveYear } = getPropsFromParams(params); - const module = createResource( - () => - dispatch(fetchModuleArchive(moduleCode, archiveYear)).catch((error) => { - captureException(error); - // TODO: If there is an error but module data can still be found, we - // can assume module has been loaded at some point, so we can just show - // that instead - throw error; - }), - () => `ModuleArchiveContainer-module-${moduleCode}-${archiveYear}`, - ); - module.preload(); + moduleResource.preload({ moduleCode, archiveYear }); return { - module, + moduleResource, moduleCode, archiveYear, }; }, + disposePreparedProps(params) { + const { moduleCode, archiveYear } = getPropsFromParams(params); + moduleResource.invalidate({ moduleCode, archiveYear }); + }, }; export default entryPoint; diff --git a/website/src/views/modules/ModuleArchiveContainer.tsx b/website/src/views/modules/ModuleArchiveContainer.tsx index 7e79e04b8e..ab52cb2f95 100644 --- a/website/src/views/modules/ModuleArchiveContainer.tsx +++ b/website/src/views/modules/ModuleArchiveContainer.tsx @@ -1,7 +1,9 @@ import React, { Suspense, useEffect } from 'react'; import { useLocation, useNavigate } from 'react-router-dom'; +import { useSelector } from 'react-redux'; import { get } from 'lodash'; +import type { State } from 'types/state'; import type { Module, ModuleCode } from 'types/modules'; import type { EntryPointComponentProps } from 'views/routes/types'; import type { Resource } from 'utils/Resource'; @@ -15,7 +17,7 @@ import ErrorBoundary from 'views/errors/ErrorBoundary'; import ModulePageContent from './ModulePageContent'; type PreparedProps = { - module: Resource; + moduleResource: Resource<{ moduleCode: ModuleCode; archiveYear: string }, string, Module>; moduleCode: ModuleCode; archiveYear: string; }; @@ -43,15 +45,21 @@ const ErrorFallback: React.FC<{ * while this page doesn't. */ export const ModuleArchiveContainerComponent: React.FC = ({ - prepared: { module, moduleCode, archiveYear }, + prepared: { moduleResource, moduleCode, archiveYear }, }) => { const location = useLocation(); const navigate = useNavigate(); + // If module already exists within our Redux store, use it instead of the + // preloaded resource (which writes to the Redux store anyway). + const module = useSelector((state: State) => + get(state.moduleBank.moduleArchive, [moduleCode, archiveYear], undefined), + ); + useEffect(() => { // Navigate to canonical URL - const canonicalUrl = moduleArchive(moduleCode, archiveYear, module.get()?.title); - if (module.get() && location.pathname !== canonicalUrl) { + const canonicalUrl = moduleArchive(moduleCode, archiveYear, module?.title); + if (module && location.pathname !== canonicalUrl) { navigate({ ...location, pathname: canonicalUrl }, { replace: true }); } }, [archiveYear, location, module, moduleCode, navigate]); @@ -62,7 +70,12 @@ export const ModuleArchiveContainerComponent: React.FC = ({ errorPage={(error) => } > }> - + ); diff --git a/website/src/views/modules/ModuleFinder.entrypoint.ts b/website/src/views/modules/ModuleFinder.entrypoint.ts index 63817a292c..123420a30c 100644 --- a/website/src/views/modules/ModuleFinder.entrypoint.ts +++ b/website/src/views/modules/ModuleFinder.entrypoint.ts @@ -8,7 +8,7 @@ const entryPoint: EntryPoint = { 'ModuleFinderContainer', () => import(/* webpackChunkName: "ModuleFinderContainer" */ './ModuleFinderContainer'), ), - prepare() { + getPreparedProps() { return {}; }, }; diff --git a/website/src/views/modules/ModulePage.entrypoint.ts b/website/src/views/modules/ModulePage.entrypoint.ts index a4ebe28aa5..ce36e5395d 100644 --- a/website/src/views/modules/ModulePage.entrypoint.ts +++ b/website/src/views/modules/ModulePage.entrypoint.ts @@ -6,8 +6,10 @@ import { JSResource } from 'utils/JSResource'; import type { Module, ModuleCode } from 'types/modules'; import type { EntryPoint } from 'views/routes/types'; +type ModuleResource = Resource<{ moduleCode: ModuleCode }, string, Module>; + export type PreparedProps = { - module: Resource; + moduleResource: ModuleResource; moduleCode: ModuleCode; }; @@ -15,32 +17,39 @@ const getPropsFromParams = (params: Params) => ({ moduleCode: (params.moduleCode ?? '').toUpperCase(), }); +let moduleResource: ModuleResource; + const entryPoint: EntryPoint = { component: JSResource( 'ModulePageContainer', () => import(/* webpackChunkName: "ModulePageContainer" */ './ModulePageContainer'), ), - prepare(params, dispatch) { + getPreparedProps(params, dispatch) { + if (!moduleResource) { + moduleResource = createResource( + ({ moduleCode }) => + dispatch(fetchModule(moduleCode)).catch((error) => { + captureException(error); + // TODO: If there is an error but module data can still be found, we + // can assume module has been loaded at some point, so we can just show + // that instead + throw error; + }), + ({ moduleCode }) => `ModulePageContainer-module-${moduleCode}`, + ); + } + const { moduleCode } = getPropsFromParams(params); - const module = createResource( - () => - dispatch(fetchModule(moduleCode)).catch((error) => { - captureException(error); - // TODO: If there is an error but module data can still be found, we - // can assume module has been loaded at some point, so we can just show - // that instead - throw error; - }), - () => `ModulePageContainer-module-${moduleCode}`, - ); - // TODO: Temp failures will prevent the module from ever being loaded, until - // a reload. - module.preload(); + moduleResource.preload({ moduleCode }); return { - module, + moduleResource, moduleCode, }; }, + disposePreparedProps(params) { + const { moduleCode } = getPropsFromParams(params); + moduleResource.invalidate({ moduleCode }); + }, }; export default entryPoint; diff --git a/website/src/views/modules/ModulePageContainer.tsx b/website/src/views/modules/ModulePageContainer.tsx index f3c001e7d0..813443564c 100644 --- a/website/src/views/modules/ModulePageContainer.tsx +++ b/website/src/views/modules/ModulePageContainer.tsx @@ -1,7 +1,9 @@ import React, { Suspense, useEffect } from 'react'; import { useLocation, useNavigate } from 'react-router-dom'; +import { useSelector } from 'react-redux'; import { get } from 'lodash'; +import type { State } from 'types/state'; import type { Module, ModuleCode } from 'types/modules'; import type { EntryPointComponentProps } from 'views/routes/types'; import type { Resource } from 'utils/Resource'; @@ -16,7 +18,7 @@ import ModulePageContent from './ModulePageContent'; // TODO: Can we dedupe PreparedProps definitions with those in *.entrypoint.ts? type PreparedProps = { - module: Resource; + moduleResource: Resource<{ moduleCode: ModuleCode }, string, Module>; moduleCode: ModuleCode; }; @@ -52,15 +54,19 @@ const ErrorFallback: React.FC<{ * - Loaded: Both requests are successfully loaded */ export const ModulePageContainerComponent: React.FC = ({ - prepared: { module, moduleCode }, + prepared: { moduleResource, moduleCode }, }) => { const location = useLocation(); const navigate = useNavigate(); + // If module already exists within our Redux store, use it instead of the + // preloaded resource (which writes to the Redux store anyway). + const module = useSelector((state: State) => state.moduleBank.modules[moduleCode]); + useEffect(() => { // Navigate to canonical URL - const canonicalUrl = modulePage(moduleCode, module.get()?.title); - if (module.get() && location.pathname !== canonicalUrl) { + const canonicalUrl = modulePage(moduleCode, module?.title); + if (module && location.pathname !== canonicalUrl) { navigate({ ...location, pathname: canonicalUrl }, { replace: true }); } }, [location, module, moduleCode, navigate]); @@ -74,7 +80,11 @@ export const ModulePageContainerComponent: React.FC = ({ errorPage={(error) => } > }> - + ); diff --git a/website/src/views/modules/ModulePageContent.tsx b/website/src/views/modules/ModulePageContent.tsx index 9267571fa4..c7674a0474 100644 --- a/website/src/views/modules/ModulePageContent.tsx +++ b/website/src/views/modules/ModulePageContent.tsx @@ -4,7 +4,7 @@ import ScrollSpy from 'react-scrollspy'; import { kebabCase, map, mapValues, values, sortBy } from 'lodash'; import { hot } from 'react-hot-loader/root'; -import { Module, NUSModuleAttributes, attributeDescription } from 'types/modules'; +import { Module, NUSModuleAttributes, attributeDescription, ModuleCode } from 'types/modules'; import config from 'config'; import { getSemestersOffered, isOffered, renderMCs } from 'utils/modules'; @@ -35,7 +35,11 @@ import styles from './ModulePageContent.scss'; import ReportError from './ReportError'; export type Props = { - module: Module | Resource; + /** Module, if we already have it. */ + module: Module | undefined; + /** Resource to suspend on if `module` is falsy. */ + moduleResource: Resource<{ moduleCode: ModuleCode; archiveYear?: string }, string, Module>; + moduleCode: ModuleCode; archiveYear?: string; }; @@ -60,17 +64,9 @@ class ModulePageContent extends React.Component { toggleMenu = (isMenuOpen: boolean) => this.setState({ isMenuOpen }); render() { - const { module: moduleOrResource, archiveYear } = this.props; - - // TODO: Deprecate module: Module; always use resource - let module: Module; - if ('read' in moduleOrResource) { - module = moduleOrResource.read(); - } else { - module = moduleOrResource; - } - - const { moduleCode, title } = module; + const { module: moduleProp, moduleResource, moduleCode, archiveYear } = this.props; + const module = moduleProp ?? moduleResource.read({ moduleCode, archiveYear }); + const { title } = module; const pageTitle = `${moduleCode} ${title}`; const semesters = getSemestersOffered(module); diff --git a/website/src/views/routes/EntryPointContainer.tsx b/website/src/views/routes/EntryPointContainer.tsx index 29d9ad3184..6e3eb9ef79 100644 --- a/website/src/views/routes/EntryPointContainer.tsx +++ b/website/src/views/routes/EntryPointContainer.tsx @@ -1,6 +1,8 @@ -import React, { memo } from 'react'; +import React, { memo, useEffect, useMemo } from 'react'; import { useDispatch } from 'react-redux'; import { Outlet, useParams } from 'react-router-dom'; +import { isEqual } from 'lodash'; +import useMemoCompare from 'views/hooks/useMemoCompare'; import type { Dispatch } from 'types/redux'; import type { EntryPoint } from './types'; @@ -12,9 +14,22 @@ const EntryPointContainer: React.FC = ({ entryPoint }) => { const EntryPointComponent = entryPoint.component.read(); const params = useParams(); const dispatch = useDispatch(); - // TODO: Figure out a way to prepare outside render. Should reuse prepare result from preload + + // Use a memoized copy of params so that we don't cause unnecessary + // renders/disposes if params didn't actually change. + const stableParams = useMemoCompare(params, isEqual); + + const prepared = useMemo(() => { + return entryPoint.getPreparedProps(stableParams, dispatch); + }, [stableParams, entryPoint, dispatch]); + + // Dispose of any prepared props if they're no longer usable. + useEffect(() => { + return () => entryPoint.disposePreparedProps?.(stableParams); + }, [entryPoint, prepared, stableParams]); + return ( - + ); diff --git a/website/src/views/routes/Routes.tsx b/website/src/views/routes/Routes.tsx index 9c8ea55424..bdefc5dc3e 100644 --- a/website/src/views/routes/Routes.tsx +++ b/website/src/views/routes/Routes.tsx @@ -38,7 +38,7 @@ function entryPointRoute( preloadCode: () => entryPoint.component.preloadOrReloadIfError(), preload(params) { entryPoint.component.preloadOrReloadIfError(); - entryPoint.prepare(params, dispatch); + entryPoint.getPreparedProps(params, dispatch); }, }; } diff --git a/website/src/views/routes/types.ts b/website/src/views/routes/types.ts index bc121297f7..b586c3fd1c 100644 --- a/website/src/views/routes/types.ts +++ b/website/src/views/routes/types.ts @@ -7,8 +7,21 @@ export type EntryPointComponentProps = React.PropsWithChildren<{ }>; export type EntryPoint> = { + /** + * A reference to the root React component of this entry point. + */ component: JSResourceReference>; - prepare: (matchParams: Params, dispatch: Dispatch) => PreparedProps; + /** + * Should be idempotent as there is a good chance it'll be called multiple + * times in quick succession on a single page navigation. + */ + getPreparedProps: (params: Params, dispatch: Dispatch) => PreparedProps; + /** + * Optionally invalidate prepared props. Typically done so that the next call + * to getPreparedProps can prepare a fresh set of data or retry any failed + * requests. + */ + disposePreparedProps?: (params: Params) => void; }; export interface EntryPointRouteObject extends RouteObject { diff --git a/website/src/views/timetable/Timetable.entrypoint.ts b/website/src/views/timetable/Timetable.entrypoint.ts index 14405f2e3e..723ebfb4b3 100644 --- a/website/src/views/timetable/Timetable.entrypoint.ts +++ b/website/src/views/timetable/Timetable.entrypoint.ts @@ -6,7 +6,7 @@ const entryPoint: EntryPoint = { 'TimetableContainer', () => import(/* webpackChunkName: "TimetableContainer" */ 'views/timetable/TimetableContainer'), ), - prepare() { + getPreparedProps() { return {}; }, }; diff --git a/website/src/views/today/EventMap.tsx b/website/src/views/today/EventMap.tsx index 0f8e30c9e9..5f8a58bf08 100644 --- a/website/src/views/today/EventMap.tsx +++ b/website/src/views/today/EventMap.tsx @@ -11,7 +11,7 @@ export type Props = { }; const EventMap = memo(({ venue }) => { - const venueLocations = venueLocationResource.fetch(); + const venueLocations = venueLocationResource.read(); if (!venue) { return ( diff --git a/website/src/views/today/EventMapInline.tsx b/website/src/views/today/EventMapInline.tsx index c118b62402..8ff40fe6a6 100644 --- a/website/src/views/today/EventMapInline.tsx +++ b/website/src/views/today/EventMapInline.tsx @@ -15,7 +15,7 @@ export type Props = { }; const EventMapInline = memo(({ venue, isOpen, className, toggleOpen }) => { - const venueLocations = venueLocationResource.fetch(); + const venueLocations = venueLocationResource.read(); const venueLocation: VenueLocation = venueLocations[venue]; if (!venueLocation || !venueLocation.location) { diff --git a/website/src/views/today/Today.entrypoint.ts b/website/src/views/today/Today.entrypoint.ts index 3fbe82540d..8feb2c6144 100644 --- a/website/src/views/today/Today.entrypoint.ts +++ b/website/src/views/today/Today.entrypoint.ts @@ -9,8 +9,9 @@ const entryPoint: EntryPoint = { 'TodayContainer', () => import(/* webpackChunkName: "TodayContainer" */ './TodayContainer'), ), - prepare() { + getPreparedProps() { venueLocationResource.preload(); + return {}; }, }; From a878f038ca188bf587e68885cda6ac8846e816a9 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Mon, 26 Oct 2020 18:30:56 +0800 Subject: [PATCH 17/20] Implement --- website/src/views/components/ScrollToTop.tsx | 25 ++++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/website/src/views/components/ScrollToTop.tsx b/website/src/views/components/ScrollToTop.tsx index 517edf5ce6..2c69c4096d 100644 --- a/website/src/views/components/ScrollToTop.tsx +++ b/website/src/views/components/ScrollToTop.tsx @@ -1,4 +1,5 @@ import React, { useEffect } from 'react'; +import { useLocation } from 'react-router-dom'; import { scrollToHash } from 'utils/react'; function scrollToTop() { @@ -7,21 +8,35 @@ function scrollToTop() { export type Props = { onComponentDidMount?: boolean; + onPathChange?: boolean; shouldScrollToHash?: boolean; }; const ScrollToTop: React.FC = ({ onComponentDidMount = false, + onPathChange = false, shouldScrollToHash = true, }) => { + useEffect( + () => { + if (onComponentDidMount && !window.location.hash) { + scrollToTop(); + } else if (shouldScrollToHash) { + scrollToHash(); + } + }, + // This effect should only be run on component mount; don't care if props + // change afterwards. + // eslint-disable-next-line react-hooks/exhaustive-deps + [], + ); + + const location = useLocation(); useEffect(() => { - // FIXME: This effect runs on mount AND when any of those props change. - if (onComponentDidMount && !window.location.hash) { + if (onPathChange) { scrollToTop(); - } else if (shouldScrollToHash) { - scrollToHash(); } - }, [onComponentDidMount, shouldScrollToHash]); + }, [onPathChange, location.pathname, location.hash]); return null; }; From 4b7f0c43b5f29f4d48a7dc6b22e8504d54f093f8 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Mon, 26 Oct 2020 19:22:50 +0800 Subject: [PATCH 18/20] Wrap EventMap* in Suspense --- website/src/views/today/DayEvents.tsx | 26 +++++++++++---------- website/src/views/today/EventMap.tsx | 6 ++--- website/src/views/today/EventMapInline.tsx | 6 ++--- website/src/views/today/Today.entrypoint.ts | 1 + website/src/views/today/TodayContainer.tsx | 7 ++++-- 5 files changed, 26 insertions(+), 20 deletions(-) diff --git a/website/src/views/today/DayEvents.tsx b/website/src/views/today/DayEvents.tsx index 28159f1e3f..6b720691d8 100644 --- a/website/src/views/today/DayEvents.tsx +++ b/website/src/views/today/DayEvents.tsx @@ -1,4 +1,4 @@ -import * as React from 'react'; +import React, { memo, Suspense } from 'react'; import { AcadWeekInfo } from 'nusmoderator'; import { isSameDay } from 'date-fns'; import classnames from 'classnames'; @@ -21,7 +21,7 @@ type Props = { readonly onOpenLesson: (date: Date, lesson: Lesson) => void; }; -const DayEvents = React.memo((props) => { +const DayEvents: React.FC = (props) => { const renderLesson = (lesson: ColoredLesson, i: number) => { const { openLesson, onOpenLesson, marker, date } = props; @@ -48,14 +48,16 @@ const DayEvents = React.memo((props) => {

{' '} {lesson.venue.startsWith('E-Learn') ? 'E-Learning' : lesson.venue} -
- onOpenLesson(date, lesson)} - /> -
+ +
+ onOpenLesson(date, lesson)} + /> +
+
); @@ -71,6 +73,6 @@ const DayEvents = React.memo((props) => { }); return
{sortedLessons.map(renderLesson)}
; -}); +}; -export default DayEvents; +export default memo(DayEvents); diff --git a/website/src/views/today/EventMap.tsx b/website/src/views/today/EventMap.tsx index 5f8a58bf08..287b9bad2b 100644 --- a/website/src/views/today/EventMap.tsx +++ b/website/src/views/today/EventMap.tsx @@ -10,7 +10,7 @@ export type Props = { venue: Venue | null; }; -const EventMap = memo(({ venue }) => { +const EventMap: React.FC = ({ venue }) => { const venueLocations = venueLocationResource.read(); if (!venue) { @@ -29,6 +29,6 @@ const EventMap = memo(({ venue }) => { const position: [number, number] = [venueLocation.location.y, venueLocation.location.x]; return ; -}); +}; -export default EventMap; +export default memo(EventMap); diff --git a/website/src/views/today/EventMapInline.tsx b/website/src/views/today/EventMapInline.tsx index 8ff40fe6a6..f4c1af2cde 100644 --- a/website/src/views/today/EventMapInline.tsx +++ b/website/src/views/today/EventMapInline.tsx @@ -14,7 +14,7 @@ export type Props = { toggleOpen: () => void; }; -const EventMapInline = memo(({ venue, isOpen, className, toggleOpen }) => { +const EventMapInline: React.FC = ({ venue, isOpen, className, toggleOpen }) => { const venueLocations = venueLocationResource.read(); const venueLocation: VenueLocation = venueLocations[venue]; @@ -38,6 +38,6 @@ const EventMapInline = memo(({ venue, isOpen, className, toggleOpen }) =>
); -}); +}; -export default EventMapInline; +export default memo(EventMapInline); diff --git a/website/src/views/today/Today.entrypoint.ts b/website/src/views/today/Today.entrypoint.ts index 8feb2c6144..141fdd7659 100644 --- a/website/src/views/today/Today.entrypoint.ts +++ b/website/src/views/today/Today.entrypoint.ts @@ -10,6 +10,7 @@ const entryPoint: EntryPoint = { () => import(/* webpackChunkName: "TodayContainer" */ './TodayContainer'), ), getPreparedProps() { + // Preload EventMap/EventMapInline data requirements venueLocationResource.preload(); return {}; }, diff --git a/website/src/views/today/TodayContainer.tsx b/website/src/views/today/TodayContainer.tsx index c3d2062405..6c1e2fbf94 100644 --- a/website/src/views/today/TodayContainer.tsx +++ b/website/src/views/today/TodayContainer.tsx @@ -1,4 +1,4 @@ -import * as React from 'react'; +import React, { Suspense } from 'react'; import { connect } from 'react-redux'; import { get, minBy, range } from 'lodash'; import NUSModerator, { AcadWeekInfo } from 'nusmoderator'; @@ -41,6 +41,7 @@ import { formatTime, getDayIndex } from 'utils/timify'; import { breakpointUp } from 'utils/css'; import { State as StoreState } from 'types/state'; import type { EntryPointComponentProps } from 'views/routes/types'; +import LoadingSpinner from 'views/components/LoadingSpinner'; import DayEvents from './DayEvents'; import DayHeader from './DayHeader'; @@ -334,7 +335,9 @@ export class TodayContainerComponent extends React.PureComponent { [styles.expanded]: this.state.isMapExpanded, })} > - + }> + +
From 6f3ea31e971e1bbc1018829d40f84acef67a2d18 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Tue, 27 Oct 2020 10:23:09 +0800 Subject: [PATCH 19/20] Convert KeyboardShortcuts to FC to allow it to work with React Router 6 --- .../views/components/KeyboardShortcuts.tsx | 205 ++++++++---------- 1 file changed, 93 insertions(+), 112 deletions(-) diff --git a/website/src/views/components/KeyboardShortcuts.tsx b/website/src/views/components/KeyboardShortcuts.tsx index 75d04454fa..1480729bad 100644 --- a/website/src/views/components/KeyboardShortcuts.tsx +++ b/website/src/views/components/KeyboardShortcuts.tsx @@ -1,32 +1,20 @@ -import * as React from 'react'; -import { withRouter, RouteComponentProps } from 'react-router-dom'; -import { Dispatch } from 'redux'; -import { connect } from 'react-redux'; +import React, { memo, useCallback, useEffect, useRef, useState } from 'react'; +import { useNavigate } from 'react-router-dom'; +import { useDispatch, useSelector } from 'react-redux'; import Mousetrap from 'mousetrap'; import { groupBy, map } from 'lodash'; -import { Mode, ThemeId, DARK_MODE } from 'types/settings'; -import { Actions } from 'types/actions'; +import { DARK_MODE } from 'types/settings'; import themes from 'data/themes.json'; import { cycleTheme, toggleTimetableOrientation } from 'actions/theme'; import { openNotification } from 'actions/app'; import { toggleMode } from 'actions/settings'; import { intersperse } from 'utils/array'; import ComponentMap from 'utils/ComponentMap'; -import { State as StoreState } from 'types/state'; +import type { State } from 'types/state'; import Modal from './Modal'; import styles from './KeyboardShortcuts.scss'; -type Props = RouteComponentProps & { - dispatch: Dispatch; - theme: ThemeId; - mode: Mode; -}; - -type State = { - helpShown: boolean; -}; - type Section = 'Appearance' | 'Navigation' | 'Timetable'; const APPEARANCE: Section = 'Appearance'; const NAVIGATION: Section = 'Navigation'; @@ -41,38 +29,52 @@ type KeyBinding = { const THEME_NOTIFICATION_TIMEOUT = 1000; -export class KeyboardShortcutsComponent extends React.PureComponent { - state = { - helpShown: false, - }; +const KeyboardShortcuts: React.FC = () => { + const [helpShown, setHelpShown] = useState(false); + const closeModal = useCallback(() => setHelpShown(false), []); - shortcuts: KeyBinding[] = []; + const mode = useSelector(({ settings }: State) => settings.mode); + const themeId = useSelector(({ theme }: State) => theme.id); + const dispatch = useDispatch(); - componentDidMount() { - const { dispatch, history } = this.props; + const navigate = useNavigate(); + + // NB: Because this is a ref, updates to `shortcuts` will not trigger a render. + const shortcuts = useRef([]); + + useEffect(() => { + function bind( + key: Shortcut, + section: Section, + description: string, + action: (e: Event) => void, + ) { + shortcuts.current.push({ key, description, section }); + Mousetrap.bind(key, action); + } // Navigation - this.bind('y', NAVIGATION, 'Go to today', () => { - history.push('/today'); + bind('y', NAVIGATION, 'Go to today', () => { + navigate('/today'); }); - this.bind('t', NAVIGATION, 'Go to timetable', () => { - history.push('/timetable'); + bind('t', NAVIGATION, 'Go to timetable', () => { + navigate('/timetable'); }); - this.bind('m', NAVIGATION, 'Go to module finder', () => { - history.push('/modules'); + bind('m', NAVIGATION, 'Go to module finder', () => { + navigate('/modules'); }); - this.bind('v', NAVIGATION, 'Go to venues page', () => { - history.push('/venues'); + bind('v', NAVIGATION, 'Go to venues page', () => { + navigate('/venues'); }); - this.bind('s', NAVIGATION, 'Go to settings', () => { - history.push('/settings'); + bind('s', NAVIGATION, 'Go to settings', () => { + navigate('/settings'); }); - this.bind('/', NAVIGATION, 'Open global search', (e) => { + bind('/', NAVIGATION, 'Open global search', (e) => { if (ComponentMap.globalSearchInput) { ComponentMap.globalSearchInput.focus(); @@ -81,16 +83,14 @@ export class KeyboardShortcutsComponent extends React.PureComponent - this.setState((state) => ({ helpShown: !state.helpShown })), - ); + bind('?', NAVIGATION, 'Show this help', () => setHelpShown(!helpShown)); // Timetable shortcuts - this.bind('o', TIMETABLE, 'Switch timetable orientation', () => { + bind('o', TIMETABLE, 'Switch timetable orientation', () => { dispatch(toggleTimetableOrientation()); }); - this.bind('d', TIMETABLE, 'Open download timetable menu', () => { + bind('d', TIMETABLE, 'Open download timetable menu', () => { const button = ComponentMap.downloadButton; if (button) { button.focus(); @@ -99,102 +99,83 @@ export class KeyboardShortcutsComponent extends React.PureComponent { - this.props.dispatch(toggleMode()); + bind('x', APPEARANCE, 'Toggle Night Mode', () => { + dispatch(toggleMode()); dispatch( - openNotification(`Night mode ${this.props.mode === DARK_MODE ? 'on' : 'off'}`, { + openNotification(`Night mode ${mode === DARK_MODE ? 'on' : 'off'}`, { overwritable: true, }), ); }); // Cycle through themes - this.bind('z', APPEARANCE, 'Previous Theme', () => { + function notifyThemeChange() { + const theme = themes.find((t) => t.id === themeId); + if (theme) { + dispatch( + openNotification(`Theme switched to ${theme.name}`, { + timeout: THEME_NOTIFICATION_TIMEOUT, + overwritable: true, + }), + ); + } + } + + bind('z', APPEARANCE, 'Previous Theme', () => { dispatch(cycleTheme(-1)); - this.notifyThemeChange(); + notifyThemeChange(); }); - this.bind('c', APPEARANCE, 'Next Theme', () => { + bind(['c', '0'], APPEARANCE, 'Next Theme', () => { dispatch(cycleTheme(1)); - this.notifyThemeChange(); + notifyThemeChange(); }); // ??? Mousetrap.bind('up up down down left right left right b a', () => { - history.push('/tetris'); + navigate('/tetris'); }); - } - closeModal = () => this.setState({ helpShown: false }); - - bind(key: Shortcut, section: Section, description: string, action: (e: Event) => void) { - this.shortcuts.push({ key, description, section }); - - Mousetrap.bind(key, action); - } + return () => { + shortcuts.current.forEach(({ key }) => Mousetrap.unbind(key)); + shortcuts.current = []; + }; + }, [dispatch, helpShown, mode, navigate, themeId]); - notifyThemeChange() { - const themeId = this.props.theme; - const theme = themes.find((t) => t.id === themeId); - - if (theme) { - this.props.dispatch( - openNotification(`Theme switched to ${theme.name}`, { - timeout: THEME_NOTIFICATION_TIMEOUT, - overwritable: true, - }), - ); - } - } - - renderShortcut = (shortcut: Shortcut): React.ReactNode => { + function renderShortcut(shortcut: Shortcut): React.ReactNode { if (typeof shortcut === 'string') { const capitalized = shortcut.replace(/\b([a-z])/, (c) => c.toUpperCase()); return {capitalized}; } + return intersperse(shortcut.map(renderShortcut), ' or '); + } - return intersperse(shortcut.map(this.renderShortcut), ' or '); - }; - - render() { - const sections = groupBy(this.shortcuts, (shortcut) => shortcut.section); - - return ( - -

Keyboard shortcuts

- - - {map(sections, (shortcuts, heading) => ( - - - - + const sections = groupBy(shortcuts.current, (shortcut) => shortcut.section); - {shortcuts.map((shortcut) => ( - - - - - ))} - - ))} -
- {heading}
{this.renderShortcut(shortcut.key)}{shortcut.description}
- - ); - } -} + return ( + +

Keyboard shortcuts

-const KeyboardShortcutsConnected = connect((state: StoreState) => ({ - mode: state.settings.mode, - theme: state.theme.id, -}))(KeyboardShortcutsComponent); + + {map(sections, (shortcutsInSection, heading) => ( + + + + + + {shortcutsInSection.map((shortcut) => ( + + + + + ))} + + ))} +
+ {heading}
{renderShortcut(shortcut.key)}{shortcut.description}
+ + ); +}; -// export default withRouter(KeyboardShortcutsConnected); -export default KeyboardShortcutsConnected; +export default memo(KeyboardShortcuts); From b69e3612d6575f893f2afbeb03352109c1955f0b Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Tue, 27 Oct 2020 10:24:10 +0800 Subject: [PATCH 20/20] Define entry point for TetrisContainer --- website/src/views/routes/Routes.tsx | 32 ++++++----- .../tetris/TetrisContainer.entrypoint.ts | 16 ++++++ website/src/views/tetris/TetrisContainer.tsx | 54 +++---------------- 3 files changed, 41 insertions(+), 61 deletions(-) create mode 100644 website/src/views/tetris/TetrisContainer.entrypoint.ts diff --git a/website/src/views/routes/Routes.tsx b/website/src/views/routes/Routes.tsx index bdefc5dc3e..9c6e929803 100644 --- a/website/src/views/routes/Routes.tsx +++ b/website/src/views/routes/Routes.tsx @@ -4,12 +4,13 @@ import { Navigate, Outlet, useRoutes } from 'react-router-dom'; import NotFoundPage from 'views/errors/NotFoundPage'; import TimetableRootRedirector from 'views/timetable/TimetableRootRedirector'; -import appShellEntryPoint from 'views/AppShell.entrypoint'; -import timetableEntryPoint from 'views/timetable/Timetable.entrypoint'; -import todayEntryPoint from 'views/today/Today.entrypoint'; -import moduleFinderEntryPoint from 'views/modules/ModuleFinder.entrypoint'; -import modulePageEntryPoint from 'views/modules/ModulePage.entrypoint'; -import moduleArchiveEntryPoint from 'views/modules/ModuleArchive.entrypoint'; +import AppShellEntryPoint from 'views/AppShell.entrypoint'; +import TimetableEntryPoint from 'views/timetable/Timetable.entrypoint'; +import TodayEntryPoint from 'views/today/Today.entrypoint'; +import ModuleFinderEntryPoint from 'views/modules/ModuleFinder.entrypoint'; +import ModulePageEntryPoint from 'views/modules/ModulePage.entrypoint'; +import ModuleArchiveEntryPoint from 'views/modules/ModuleArchive.entrypoint'; +import TetrisContainerEntryPoint from 'views/tetris/TetrisContainer.entrypoint'; import type { Dispatch } from 'types/redux'; import type { EntryPoint, EntryPointPartialRouteObject, EntryPointRouteObject } from './types'; @@ -24,7 +25,6 @@ import type { EntryPoint, EntryPointPartialRouteObject, EntryPointRouteObject } // import AppsContainer from 'views/static/AppsContainer'; // import TodayContainer from 'views/today/TodayContainer'; // import PlannerContainer from 'views/planner/PlannerContainer'; -// import TetrisContainer from 'views/tetris/TetrisContainer'; import EntryPointContainer from './EntryPointContainer'; import { RoutePreloaderProvider } from './RoutePreloaderContext'; @@ -46,7 +46,7 @@ function entryPointRoute( // // today // -// +// tetris // // @@ -75,7 +75,7 @@ function createPartialRoutes(dispatch: Dispatch): EntryPointPartialRouteObject[] // { path: '/api', element: }, { - ...entryPointRoute(appShellEntryPoint, dispatch), + ...entryPointRoute(AppShellEntryPoint, dispatch), children: [ { path: '/', @@ -86,7 +86,7 @@ function createPartialRoutes(dispatch: Dispatch): EntryPointPartialRouteObject[] children: [ { path: ':semester/*', - ...entryPointRoute(timetableEntryPoint, dispatch), + ...entryPointRoute(TimetableEntryPoint, dispatch), }, { path: '/', @@ -96,19 +96,23 @@ function createPartialRoutes(dispatch: Dispatch): EntryPointPartialRouteObject[] }, { path: '/today', - ...entryPointRoute(todayEntryPoint, dispatch), + ...entryPointRoute(TodayEntryPoint, dispatch), }, { path: '/modules', - ...entryPointRoute(moduleFinderEntryPoint, dispatch), + ...entryPointRoute(ModuleFinderEntryPoint, dispatch), }, { path: '/modules/:moduleCode/*', - ...entryPointRoute(modulePageEntryPoint, dispatch), + ...entryPointRoute(ModulePageEntryPoint, dispatch), }, { path: '/archive/:moduleCode/:archiveYear/*', - ...entryPointRoute(moduleArchiveEntryPoint, dispatch), + ...entryPointRoute(ModuleArchiveEntryPoint, dispatch), + }, + { + path: '/tetris', + ...entryPointRoute(TetrisContainerEntryPoint, dispatch), }, // 404 page diff --git a/website/src/views/tetris/TetrisContainer.entrypoint.ts b/website/src/views/tetris/TetrisContainer.entrypoint.ts new file mode 100644 index 0000000000..57fa3d65e1 --- /dev/null +++ b/website/src/views/tetris/TetrisContainer.entrypoint.ts @@ -0,0 +1,16 @@ +import { JSResource } from 'utils/JSResource'; +import { EntryPoint } from 'views/routes/types'; + +export type PreparedProps = unknown; + +const entryPoint: EntryPoint = { + component: JSResource( + 'TetrisContainer', + () => import(/* webpackChunkName: "TetrisContainer" */ './TetrisContainer'), + ), + getPreparedProps() { + return {}; + }, +}; + +export default entryPoint; diff --git a/website/src/views/tetris/TetrisContainer.tsx b/website/src/views/tetris/TetrisContainer.tsx index 1ad39b46f0..b01c9bff95 100644 --- a/website/src/views/tetris/TetrisContainer.tsx +++ b/website/src/views/tetris/TetrisContainer.tsx @@ -1,54 +1,14 @@ -import React, { lazy, memo, Suspense, useCallback, useState } from 'react'; - -import ApiError from 'views/errors/ApiError'; -import LoadingSpinner from 'views/components/LoadingSpinner'; -import ErrorBoundary from 'views/errors/ErrorBoundary'; -import type { Props as TetrisGameProps } from './TetrisGame'; - -// Source: https://github.com/facebook/react/issues/14254#issuecomment-441717770 -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function retryableLazy>( - lazyImport: () => Promise<{ default: T }>, - setComponent: (component: React.LazyExoticComponent) => void, -) { - setComponent( - lazy(() => - lazyImport().catch((err) => { - retryableLazy(lazyImport, setComponent); - throw err; - }), - ), - ); -} - -let TetrisGame: React.ComponentType; -retryableLazy( - () => import(/* webpackChunkName: "tetris" */ './TetrisGame'), - (component) => { - TetrisGame = component; - }, -); +import React, { useCallback, useState } from 'react'; +import TetrisGame from './TetrisGame'; /** - * Wrapper around TetrisGame which: - * - Lazy loads the TetrisGame component. - * - Resets the game's internal state and components by forcing a remount after - * each game via the key prop + * Wrapper around TetrisGame which resets the game's internal state and + * components by forcing a remount after each game via the key prop. */ -const TetrisContainer = memo(() => { +const TetrisContainer: React.FC = () => { const [game, setGame] = useState(0); const onResetGame = useCallback(() => setGame(game + 1), [game]); - - return ( - } - > - }> - ; - - - ); -}); + return ; +}; export default TetrisContainer;