search

rdkcentral / Lightning-SDK

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

Can't access queryParams with router #238

Closed chiefcll closed 3 years ago

chiefcll commented 3 years ago

Seems like queryParams are stored behind a symbol...

https://github.com/rdkcentral/Lightning-SDK/blob/master/src/Router/utils/provider.js#L76

So if I have a route like /myroute?importantParam=hi

How should I access that parameter? Why not put it in page.params?

chiefcll commented 3 years ago

My current solution for others is:

before: (page) => {
        page.queryParams = page.params[Router.symbols.queryParams];
        return Promise.resolve();
      }
erikhaandrikman commented 3 years ago

we chose to store the queryParams behind a unique Symbol property to prevent potential collision between dynamic route parts and querystring parameters.

Let say we have this route configuration: home/:categoryId/:playbackId and you point your browser to: foo.bar/?plackbackId=12#home/14/15 we would end up with duplicate keys for playbackId in the params object.

Your solution for grabbing the parameters is the correct one since the Router exposes the Symbols mapping: https://github.com/rdkcentral/Lightning-SDK/blob/0accb42cf6479e711a5c604d6ccc140050b44b7d/src/Router/index.js#L487

erikhaandrikman commented 3 years ago

Can we close this issue?

chiefcll commented 3 years ago

I would argue that this is a poor developer experience for accessing queryParams and should not be behind a symbol. I don't think SDK protect me from using the same name in route param and query param (and who would do that anyhow?) - Needless to say I won't argue strongly for fixing it but a lot of developers who will need to use queryParams will be frustrated by this.

erikhaandrikman commented 3 years ago

I agree that the public api is not intuitive enough. We will rethink the interface and probably provide something like Router.getQueryParams() to make it a bit more obvious.

As for dynamic name collisions between route blueprints and query string parameters; Since the dynamic nature of the Router it's technically possible to extend the route configuration run-time. This mixed with the router's support for dynamic module import you can work towards an implementation where 3rd parties can ship their own (let's say) Ui modules with route configuration attached. In this situation the 3rd party developer introduces their own dynamic names and they could fairly easily collide with one of the Ui's launching parameters.

erikhaandrikman commented 3 years ago

Fixed in v4.5.0 (https://github.com/rdkcentral/Lightning-SDK/blob/master/CHANGELOG.md#v450)