Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[State Management] State syncing utilities and ScopedHistory #58137

Closed
Dosant opened this issue Feb 20, 2020 · 3 comments
Closed

[State Management] State syncing utilities and ScopedHistory #58137

Dosant opened this issue Feb 20, 2020 · 3 comments

Comments

@Dosant
Copy link
Contributor

Dosant commented Feb 20, 2020

Need to do some refactoring and testing to make sure sync state utilities work with new ScopedHistory api provided in core.

ScopedHistory introduced in core in this pr #56705
New api addresses issues possible issues described in #53692

There will need to be some modifications to existing utilities to work correctly with this code. For example, sync state will need to use this provided history instance and use relative URLs rather than absolute URLs. In my brief experiments, it seemed this will actually simplify sync state's implementation quite a bit.

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@Dosant Dosant self-assigned this Feb 20, 2020
@joshdover
Copy link
Contributor

When I was originally developing and testing ScopedHistory I did some of the updates to get state sync working (though this is not all of them). The main change here was making everything work with relative URLs which simplified things quite a bit. Here is the diff I saved for future reference:

diff --git a/examples/state_containers_examples/public/app.tsx b/examples/state_containers_examples/public/app.tsx
index 319680d07f9..5e72669449e 100644
--- a/examples/state_containers_examples/public/app.tsx
+++ b/examples/state_containers_examples/public/app.tsx
@@ -20,7 +20,7 @@
 import { AppMountParameters } from 'kibana/public';
 import ReactDOM from 'react-dom';
 import React from 'react';
-import { createHashHistory, createBrowserHistory } from 'history';
+import { createHashHistory } from 'history';
 import { TodoAppPage } from './todo';
 
 export interface AppOptions {
@@ -35,13 +35,10 @@ export enum History {
 }
 
 export const renderApp = (
-  { appBasePath, element }: AppMountParameters,
+  { appBasePath, element, history: platformHistory }: AppMountParameters,
   { appInstanceId, appTitle, historyType }: AppOptions
 ) => {
-  const history =
-    historyType === History.Browser
-      ? createBrowserHistory({ basename: appBasePath })
-      : createHashHistory();
+  const history = historyType === History.Browser ? platformHistory : createHashHistory();
   ReactDOM.render(
     <TodoAppPage
       history={history}
@@ -54,8 +51,8 @@ export const renderApp = (
         const currentAppUrl = stripTrailingSlash(history.createHref(history.location));
         if (historyType === History.Browser) {
           // browser history
-          const basePath = stripTrailingSlash(appBasePath);
-          return currentAppUrl === basePath && !history.location.search && !history.location.hash;
+          // const basePath = stripTrailingSlash(appBasePath);
+          return currentAppUrl === '' && !history.location.search && !history.location.hash;
         } else {
           // hashed history
           return currentAppUrl === '#' && !history.location.search;
diff --git a/src/plugins/kibana_utils/public/state_management/url/kbn_url_storage.ts b/src/plugins/kibana_utils/public/state_management/url/kbn_url_storage.ts
index 1dd204e7172..cc00289e605 100644
--- a/src/plugins/kibana_utils/public/state_management/url/kbn_url_storage.ts
+++ b/src/plugins/kibana_utils/public/state_management/url/kbn_url_storage.ts
@@ -153,7 +153,7 @@ export const createKbnUrlControls = (
   let shouldReplace = true;
 
   function updateUrl(newUrl: string, replace = false): string | undefined {
-    const currentUrl = getCurrentUrl();
+    const currentUrl = getCurrentUrl(history);
     if (newUrl === currentUrl) return undefined; // skip update
 
     const historyPath = getRelativeToHistoryPath(newUrl, history);
@@ -164,7 +164,7 @@ export const createKbnUrlControls = (
       history.push(historyPath);
     }
 
-    return getCurrentUrl();
+    return getCurrentUrl(history);
   }
 
   // queue clean up
@@ -186,7 +186,10 @@ export const createKbnUrlControls = (
 
   function getPendingUrl() {
     if (updateQueue.length === 0) return undefined;
-    const resultUrl = updateQueue.reduce((url, nextUpdate) => nextUpdate(url), getCurrentUrl());
+    const resultUrl = updateQueue.reduce(
+      (url, nextUpdate) => nextUpdate(url),
+      getCurrentUrl(history)
+    );
 
     return resultUrl;
   }
@@ -234,8 +237,9 @@ export function getRelativeToHistoryPath(absoluteUrl: string, history: History):
     const stripLeadingHash = (_: string) => (_.charAt(0) === '#' ? _.substr(1) : _);
     const stripTrailingSlash = (_: string) =>
       _.charAt(_.length - 1) === '/' ? _.substr(0, _.length - 1) : _;
-    const baseName = stripLeadingHash(stripTrailingSlash(history.createHref({})));
-    return path.startsWith(baseName) ? path.substr(baseName.length) : path;
+    // const baseName = stripLeadingHash(stripTrailingSlash(history.createHref({})));
+    // return path.startsWith(baseName) ? path.substr(baseName.length) : path;
+    return stripLeadingHash(stripTrailingSlash(path));
   }
   const isHashHistory = history.createHref({}).includes('#');
   const parsedUrl = isHashHistory ? parseUrlHash(absoluteUrl)! : parseUrl(absoluteUrl);
diff --git a/src/plugins/kibana_utils/public/state_management/url/parse.ts b/src/plugins/kibana_utils/public/state_management/url/parse.ts
index 95041d0662f..9228f3efc6a 100644
--- a/src/plugins/kibana_utils/public/state_management/url/parse.ts
+++ b/src/plugins/kibana_utils/public/state_management/url/parse.ts
@@ -18,12 +18,13 @@
  */
 
 import { parse as _parseUrl } from 'url';
+import { History } from 'history';
 
 export const parseUrl = (url: string) => _parseUrl(url, true);
 export const parseUrlHash = (url: string) => {
   const hash = parseUrl(url).hash;
   return hash ? parseUrl(hash.slice(1)) : null;
 };
-export const getCurrentUrl = () => window.location.href;
-export const parseCurrentUrl = () => parseUrl(getCurrentUrl());
-export const parseCurrentUrlHash = () => parseUrlHash(getCurrentUrl());
+export const getCurrentUrl = (history: History) => history.createHref(history.location);
+export const parseCurrentUrl = (history: History) => parseUrl(getCurrentUrl(history));
+export const parseCurrentUrlHash = (history: History) => parseUrlHash(getCurrentUrl(history));

@Dosant Dosant added v7.8.0 and removed v7.7.0 labels Mar 27, 2020
@Dosant Dosant moved this from To triage to 7.8 in kibana-app-arch Mar 27, 2020
@Dosant Dosant moved this from 7.8 to In progress in kibana-app-arch Apr 6, 2020
@Dosant
Copy link
Contributor Author

Dosant commented Apr 6, 2020

Needs #62016 fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
kibana-app-arch
  
Done in current release
Development

No branches or pull requests

3 participants