rdkcentral / Lightning-SDK

SDK for Lightning framework
Apache License 2.0
130 stars 69 forks source link

Router: Query param change fails to make it to _onUrlParams() #274

Closed frank-weindel closed 2 years ago

frank-weindel commented 3 years ago

Starting with already being on a page with the route:

#front-door

Manually add a URL param to the hash in the browser to look like so:

#front-door?myParam=myValue

Expected: _onUrlParams' param is:

{
  Symbol(queryParams): {
    myParam: "myValue"
  }
  Symbol(store): undefined
}

Actual: _onUrlParams' param is:

{
  Symbol(store): undefined
}
frank-weindel commented 3 years ago

Issue seems to be related to this line. When the query params are parsed, instead of using the new incoming hash it is internally ignoring that hash and using the resumeHash, which matches the route without the query param. I can just remove that part of the if statement, but not knowing exactly what the intent is here I'm not sure if that will break something else.

erikhaandrikman commented 2 years ago

This was added to capture url parameters when a bootComponent is configured. When it's configured the Router will navigate to $ force it to load as any other configured route.

https://github.com/rdkcentral/Lightning-SDK/blob/93c749509bb9a5db5e2888b8eed7d76ab3ac72ec/src/Router/index.js#L85 https://github.com/rdkcentral/Lightning-SDK/blob/93c749509bb9a5db5e2888b8eed7d76ab3ac72ec/src/Router/index.js#L132

Depending on the moment you call getQueryStringParams() ; getActiveHash() can return undefined or $ ( in case of calling it in root function ) to still be able to read the correct parameters, it uses the resumeHash instead of the active hash: https://github.com/rdkcentral/Lightning-SDK/blob/93c749509bb9a5db5e2888b8eed7d76ab3ac72ec/src/Router/utils/helpers.js#L114

The problem seems to be in the condition; it should check if hash eq to $ or undefined AND resumeHash has a value

https://github.com/rdkcentral/Lightning-SDK/blob/93c749509bb9a5db5e2888b8eed7d76ab3ac72ec/src/Router/utils/helpers.js#L116

erikhaandrikman commented 2 years ago

This fix is available in release 4.8.0

frank-weindel commented 2 years ago

Thanks @erikhaandrikman