From e552effd96d4184dbc904522400fa4e1ff4667da Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Tue, 10 Mar 2020 13:09:26 +0200 Subject: [PATCH] Remove `route.resolve` feature (#4607) * Stop using route.resolve feature (pages should load all the data themselves) * Remove route.resolve feature Co-authored-by: Arik Fraimovich --- .../app/components/ApplicationArea/Router.jsx | 19 +------- client/app/pages/queries/QuerySource.jsx | 13 +++--- client/app/pages/queries/QueryView.jsx | 9 ++-- .../queries/components/wrapQueryPage.jsx | 44 +++++++++++++++++++ 4 files changed, 55 insertions(+), 30 deletions(-) create mode 100644 client/app/pages/queries/components/wrapQueryPage.jsx diff --git a/client/app/components/ApplicationArea/Router.jsx b/client/app/components/ApplicationArea/Router.jsx index d1465bbba5..e90c7b02a2 100644 --- a/client/app/components/ApplicationArea/Router.jsx +++ b/client/app/components/ApplicationArea/Router.jsx @@ -1,4 +1,4 @@ -import { isFunction, map, fromPairs, extend, startsWith, trimStart, trimEnd } from "lodash"; +import { isFunction, startsWith, trimStart, trimEnd } from "lodash"; import React, { useState, useEffect, useRef } from "react"; import PropTypes from "prop-types"; import UniversalRouter from "universal-router"; @@ -30,18 +30,6 @@ export function stripBase(href) { return false; } -function resolveRouteDependencies(route) { - return Promise.all( - map(route.resolve, (value, key) => { - value = isFunction(value) ? value(route.routeParams, route, location) : value; - return Promise.resolve(value).then(result => [key, result]); - }) - ).then(results => { - route.routeParams = extend(route.routeParams, fromPairs(results)); - return route; - }); -} - export default function Router({ routes, onRouteChange }) { const [currentRoute, setCurrentRoute] = useState(null); @@ -86,10 +74,7 @@ export default function Router({ routes, onRouteChange }) { router .resolve({ pathname }) .then(route => { - return isAbandoned || currentPathRef.current !== pathname ? null : resolveRouteDependencies(route); - }) - .then(route => { - if (route) { + if (!isAbandoned && currentPathRef.current === pathname) { setCurrentRoute({ ...route, key: generateRouteKey() }); } }) diff --git a/client/app/pages/queries/QuerySource.jsx b/client/app/pages/queries/QuerySource.jsx index 7b00800c65..3f1e6c7e24 100644 --- a/client/app/pages/queries/QuerySource.jsx +++ b/client/app/pages/queries/QuerySource.jsx @@ -21,6 +21,7 @@ import QueryVisualizationTabs from "./components/QueryVisualizationTabs"; import QueryExecutionStatus from "./components/QueryExecutionStatus"; import SchemaBrowser from "./components/SchemaBrowser"; import QuerySourceAlerts from "./components/QuerySourceAlerts"; +import wrapQueryPage from "./components/wrapQueryPage"; import QueryExecutionMetadata from "./components/QueryExecutionMetadata"; import useQuery from "./hooks/useQuery"; @@ -412,21 +413,17 @@ QuerySource.propTypes = { query: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types }; +const QuerySourcePage = wrapQueryPage(QuerySource); + export default [ routeWithUserSession({ path: "/queries/new", - render: pageProps => , - resolve: { - query: () => Query.newQuery(), - }, + render: pageProps => , bodyClass: "fixed-layout", }), routeWithUserSession({ path: "/queries/:queryId([0-9]+)/source", - render: pageProps => , - resolve: { - query: ({ queryId }) => Query.get({ id: queryId }), - }, + render: pageProps => , bodyClass: "fixed-layout", }), ]; diff --git a/client/app/pages/queries/QueryView.jsx b/client/app/pages/queries/QueryView.jsx index bbb8e2ed7a..f511e3d486 100644 --- a/client/app/pages/queries/QueryView.jsx +++ b/client/app/pages/queries/QueryView.jsx @@ -9,7 +9,6 @@ import routeWithUserSession from "@/components/ApplicationArea/routeWithUserSess import EditInPlace from "@/components/EditInPlace"; import Parameters from "@/components/Parameters"; -import { Query } from "@/services/query"; import DataSource from "@/services/data-source"; import { ExecutionStatus } from "@/services/query-result"; import getQueryResultData from "@/lib/getQueryResultData"; @@ -18,6 +17,7 @@ import QueryPageHeader from "./components/QueryPageHeader"; import QueryVisualizationTabs from "./components/QueryVisualizationTabs"; import QueryExecutionStatus from "./components/QueryExecutionStatus"; import QueryMetadata from "./components/QueryMetadata"; +import wrapQueryPage from "./components/wrapQueryPage"; import QueryViewButton from "./components/QueryViewButton"; import QueryExecutionMetadata from "./components/QueryExecutionMetadata"; @@ -211,10 +211,9 @@ function QueryView(props) { QueryView.propTypes = { query: PropTypes.object.isRequired }; // eslint-disable-line react/forbid-prop-types +const QueryViewPage = wrapQueryPage(QueryView); + export default routeWithUserSession({ path: "/queries/:queryId([0-9]+)", - render: pageProps => , - resolve: { - query: ({ queryId }) => Query.get({ id: queryId }), - }, + render: pageProps => , }); diff --git a/client/app/pages/queries/components/wrapQueryPage.jsx b/client/app/pages/queries/components/wrapQueryPage.jsx new file mode 100644 index 0000000000..6fde14dd3c --- /dev/null +++ b/client/app/pages/queries/components/wrapQueryPage.jsx @@ -0,0 +1,44 @@ +import React, { useState, useEffect, useRef } from "react"; +import PropTypes from "prop-types"; +import LoadingState from "@/components/items-list/components/LoadingState"; +import { Query } from "@/services/query"; + +export default function wrapQueryPage(WrappedComponent) { + function QueryPageWrapper({ queryId, onError, ...props }) { + const [query, setQuery] = useState(null); + const onErrorRef = useRef(); + onErrorRef.current = onError; + + useEffect(() => { + let isCancelled = false; + const promise = queryId ? Query.get({ id: queryId }) : Promise.resolve(Query.newQuery()); + promise + .then(result => { + if (!isCancelled) { + setQuery(result); + } + }) + .catch(error => onErrorRef.current(error)); + + return () => { + isCancelled = true; + }; + }, [queryId]); + + if (!query) { + return ; + } + + return ; + } + + QueryPageWrapper.propTypes = { + queryId: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), + }; + + QueryPageWrapper.defaultProps = { + queryId: null, + }; + + return QueryPageWrapper; +}