Skip to content

Commit

Permalink
Remove route.resolve feature (#4607)
Browse files Browse the repository at this point in the history
* Stop using route.resolve feature (pages should load all the data themselves)

* Remove route.resolve feature

Co-authored-by: Arik Fraimovich <[email protected]>
  • Loading branch information
kravets-levko and arikfr authored Mar 10, 2020
1 parent 75cc6b3 commit e552eff
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 30 deletions.
19 changes: 2 additions & 17 deletions client/app/components/ApplicationArea/Router.jsx
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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() });
}
})
Expand Down
13 changes: 5 additions & 8 deletions client/app/pages/queries/QuerySource.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 => <QuerySource {...pageProps} />,
resolve: {
query: () => Query.newQuery(),
},
render: pageProps => <QuerySourcePage {...pageProps} />,
bodyClass: "fixed-layout",
}),
routeWithUserSession({
path: "/queries/:queryId([0-9]+)/source",
render: pageProps => <QuerySource {...pageProps} />,
resolve: {
query: ({ queryId }) => Query.get({ id: queryId }),
},
render: pageProps => <QuerySourcePage {...pageProps} />,
bodyClass: "fixed-layout",
}),
];
9 changes: 4 additions & 5 deletions client/app/pages/queries/QueryView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";

Expand Down Expand Up @@ -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 => <QueryView {...pageProps} />,
resolve: {
query: ({ queryId }) => Query.get({ id: queryId }),
},
render: pageProps => <QueryViewPage {...pageProps} />,
});
44 changes: 44 additions & 0 deletions client/app/pages/queries/components/wrapQueryPage.jsx
Original file line number Diff line number Diff line change
@@ -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 <LoadingState className="flex-fill" />;
}

return <WrappedComponent query={query} onError={onError} {...props} />;
}

QueryPageWrapper.propTypes = {
queryId: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
};

QueryPageWrapper.defaultProps = {
queryId: null,
};

return QueryPageWrapper;
}

0 comments on commit e552eff

Please sign in to comment.