Merge pull request #244 from rdkcentral/feature/router

Feature/router

Author: michielvandergeest

CHANGELOG.md

@@ -1,5 +1,18 @@
 # Changelog
 
+## v4.4.0
+
+*18 june 2021*
+
+- Router updates
+    - Added showing loading page between shared instances with `on()` provider
+    - Fixed wrong historyState object on shared instance
+    - Fixed hash to route mismatch on hash with trailing slash
+    - Fixed showing bootPage before unknown hash
+    - Fixed focus issues with shared state on routes with same page type
+    - Fixed memoryleak on shared routes with lazyCreate disabled
+   
+    
 ## v4.3.3
 
 *7 may 2021*

src/Router/index.js

@@ -25,6 +25,7 @@ import {
   isString,
   getQueryStringParams,
   symbols,
+  cleanHash,
 } from './utils/helpers'
 
 import {
@@ -121,6 +122,12 @@ const start = () => {
   }
 
   if (routeExists(bootKey)) {
+    if (hash && !isDirectLoad) {
+      if (!getRouteByHash(hash)) {
+        navigate('*', { failedHash: hash })
+        return
+      }
+    }
     navigate(
       bootKey,
       {
@@ -198,7 +205,7 @@ export const navigate = (url, args = {}, store) => {
 }
 
 const queue = (hash, args = {}, store) => {
-  hash = hash.replace(/^#/, '')
+  hash = cleanHash(hash)
   if (!navigateQueue.has(hash)) {
     for (let request of navigateQueue.values()) {
       request.cancel()
@@ -217,7 +224,7 @@ const queue = (hash, args = {}, store) => {
  * @returns {Promise<void>}
  */
 const handleHashChange = async override => {
-  const hash = (override || getHash()).replace(/^#/, '')
+  const hash = cleanHash(override || getHash())
   const queueId = decodeURIComponent(hash)
   let request = navigateQueue.get(queueId)
 
@@ -299,6 +306,13 @@ const resolveHashChange = request => {
     }
     // if there is a component attached to the route
     if (component) {
+      // to prevent shared state issues between same routes
+      // we force page to root state
+      const activePage = getActivePage()
+      if (activePage) {
+        activePage._setState('')
+      }
+
       if (isPage(component, stage)) {
         load(request).then(() => {
           app._refocus()

src/Router/model/Request.js

@@ -22,19 +22,67 @@ import { createRegister } from '../utils/register'
 import Log from '../../Log'
 
 export default class Request {
-  constructor(hash, navArgs, storeCaller) {
+  constructor(hash = '', navArgs, storeCaller) {
+    /**
+     * Hash we navigate to
+     * @type {string}
+     * @private
+     */
     this._hash = hash
+
+    /**
+     * Do we store previous hash in history
+     * @type {boolean}
+     * @private
+     */
     this._storeCaller = storeCaller
+
+    /**
+     * Request and navigate data
+     * @type {Map}
+     * @private
+     */
     this._register = new Map()
+
+    /**
+     * Flag if the instance is created due to
+     * this request
+     * @type {boolean}
+     * @private
+     */
     this._isCreated = false
+
+    /**
+     * Flag if the instance is shared between
+     * previous and current request
+     * @type {boolean}
+     * @private
+     */
     this._isSharedInstance = false
+
+    /**
+     * Flag if the request has been cancelled
+     * @type {boolean}
+     * @private
+     */
     this._cancelled = false
 
+    /**
+     * if instance is shared between requests we copy state object
+     * from instance before the new request overrides state
+     * @type {null}
+     * @private
+     */
+    this._copiedHistoryState = null
+
     // if there are arguments attached to navigate()
     // we store them in new request
     if (isObject(navArgs)) {
       this._register = createRegister(navArgs)
     } else if (isBoolean(navArgs)) {
+      // if second navigate() argument is explicitly
+      // set to false we prevent the calling page
+      // from ending up in history
       this._storeCaller = navArgs
     }
     // @todo: remove because we can simply check
@@ -114,4 +162,12 @@ export default class Request {
   get isCancelled() {
     return this._cancelled
   }
+
+  set copiedHistoryState(v) {
+    this._copiedHistoryState = v
+  }
+
+  get copiedHistoryState() {
+    return this._copiedHistoryState
+  }
 }

src/Router/utils/helpers.js

@@ -69,6 +69,10 @@ export const isPromise = method => {
   return isObject(result) && isFunction(result.then)
 }
 
+export const cleanHash = (hash = '') => {
+  return hash.replace(/^#/, '').replace(/\/+$/, '')
+}
+
 export const getConfigMap = () => {
   const routerSettings = Settings.get('platform', 'router')
   const isObj = isObject(routerSettings)

src/Router/utils/history.js

@@ -49,7 +49,7 @@ export const updateHistory = request => {
   if (store) {
     const toStore = hash.replace(/^\//, '')
     const location = locationInHistory(toStore)
-    const stateObject = getStateObject(getActivePage())
+    const stateObject = getStateObject(getActivePage(), request)
     const routerConfig = getRouterConfig()
 
     // store hash if it's not a part of history or flag for
@@ -102,8 +102,14 @@ export const replaceHistoryState = (state = null, hash) => {
   }
 }
 
-const getStateObject = page => {
-  if (page && isFunction(page.historyState)) {
+const getStateObject = (page, request) => {
+  // if the new request shared instance with the
+  // previous request we used the copied state object
+  if (request.isSharedInstance) {
+    if (request.copiedHistoryState) {
+      return request.copiedHistoryState
+    }
+  } else if (page && isFunction(page.historyState)) {
     return page.historyState()
   }
   return null

src/Router/utils/loader.js

@@ -31,8 +31,8 @@ import {
 } from './router'
 
 import Log from '../../Log'
-import { isBoolean, isComponentConstructor, symbols } from './helpers'
-import { getProvider, hasProvider, isPageExpired, types, addPersistData } from './provider'
+import { isBoolean, isComponentConstructor, isFunction, symbols } from './helpers'
+import { getProvider, hasProvider, isPageExpired, dataHooks, addPersistData } from './provider'
 import { createComponent } from './components'
 import { executeTransition } from './transition'
 import { getActiveWidget } from './widgets'
@@ -120,7 +120,7 @@ const loader = async request => {
     }
   }
 
-  // If type is not a constructor
+  // If page is Lightning Component instance
   if (!isConstruct) {
     request.page = type
     // if we have have a data route for current page
@@ -136,6 +136,12 @@ const loader = async request => {
     // and check platform settings in we want to re-use instance
     if (route.path === currentRoute) {
       request.isSharedInstance = true
+      // since we're re-using the instance we must attach
+      // historyState to the request to prevent it from
+      // being overridden.
+      if (isFunction(request.page.historyState)) {
+        request.copiedHistoryState = request.page.historyState()
+      }
     }
   } else {
     request.page = createComponent(stage, type)
@@ -162,7 +168,7 @@ const loader = async request => {
       request.provider = provider
       request.providerType = loadType
 
-      await types[request.isSharedInstance ? 'shared' : loadType](request)
+      await dataHooks[loadType](request)
 
       // we early exit if the current request is expired
       if (hash !== getLastHash()) {

src/Router/utils/provider.js

@@ -24,7 +24,7 @@ import emit from './emit'
 
 export let previousState
 
-export const types = {
+export const dataHooks = {
   on: request => {
     previousState = app.state || ''
     app._setState('Loading')
@@ -41,10 +41,6 @@ export const types = {
     }
     return Promise.resolve()
   },
-  // on route share instance
-  shared: request => {
-    return execProvider(request)
-  },
 }
 
 const execProvider = (request, emitProvided) => {

src/Router/utils/router.js

@@ -25,6 +25,7 @@ import {
   isFunction,
   isPage,
   symbols,
+  cleanHash,
 } from './helpers'
 import { step, navigateQueue } from '../index'
 import { createRoute, getOption } from './route'
@@ -204,8 +205,7 @@ const setup = config => {
     init(config)
   }
   config.routes.forEach(r => {
-    // strip leading slash
-    const path = r.path.replace(/\/+$/, '')
+    const path = cleanHash(r.path)
     if (!routeExists(path)) {
       const route = createRoute(r)
       routes.set(path, route)
@@ -366,9 +366,21 @@ const cleanUp = (page, request) => {
 
   let doCleanup = false
 
+  // if this request is executed due to a step back in history
+  // and we have configured to destroy active page when we go back
+  // in history or lazyDestory is enabled
   if (isFromHistory && (destroyOnBack || lazyDestroy)) {
     doCleanup = true
-  } else if (lazyDestroy && !keepAlive) {
+  }
+
+  // clean up if lazyDestroy is enabled and the keepAlive flag
+  // in navigation register is false
+  if (lazyDestroy && !keepAlive) {
+    doCleanup = true
+  }
+
+  // if the current and new request share the same route blueprint
+  if (activeRoute === request.route.path) {
     doCleanup = true
   }