On Fri, Sep 6, 2019 at 4:45 PM Dushan Silva <dush...@wso2.com> wrote:
> Hi Kasun, > > >> - We already have a query string parser/formatter library *qs >> >> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/package.json#L42> >> *shall >> we use that library in here[1 >> >> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L52> >> ][2 >> >> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L56>] >> too ? is there any limitations with that library ? >> >> no limitation, since query-string was already available in our package I > used it, we can use qs too. I think both can be used here. > There is a minor advantage of using query-string instead of qs where we can parse the location.search prop directly into query string without removing the ? *const parsed = queryString.parse(location.search);* But for qs we either have to replace the ? with ' ' like we have done in [1] or specifically mention to ignore the ? like *var prefixed = qs.parse('?a=b&c=d', { ignoreQueryPrefix: true });* [1] https://github.com/wso2/carbon-apimgt/blob/master/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/ProtectedApp.jsx#L152 Thanks, Tanya > > Shall we move the BrowserRouter.jsx file to >> */store-new/source/src/app/components/Base/* directory, Since this file >> is a React component, it should come under components directory and there >> we could move it to Base components directory? > > Shall we add the props validation in the original BrowserRouter >> <https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/BrowserRouter.js#L19-L25>[4] >> implementation? > > > +1, Will do that > > > > On Fri, Sep 6, 2019 at 11:16 AM Kasun Thennakoon <kasu...@wso2.com> wrote: > >> Hi Dushan, >> >> First, great work on implementing the multi-tenancy support to Store >> SPA, This improvement will allow users to directly access a tenant >> store,share tenant store URL etc. >> I have a few suggestions on the implementation. >> >> - We already have a query string parser/formatter library *qs >> >> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/package.json#L42> >> *shall >> we use that library in here[1 >> >> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L52> >> ][2 >> >> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L56>] >> too ? is there any limitations with that library ? >> - Shall we move the BrowserRouter.jsx file to >> */store-new/source/src/app/components/Base/* directory, Since this >> file is a React component, it should come under components directory and >> there we could move it to Base components directory? >> - Shall we add the props validation in the original BrowserRouter >> >> <https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/BrowserRouter.js#L19-L25>[4] >> implementation? >> - And also I think we now need to keep an eye on the original >> (react-router-dom) BrowserRouter.js >> >> <https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/BrowserRouter.js#L19-L25> >> implementation, >> To absorb their changes to our component. >> >> >> [1]: >> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L52 >> [2]: >> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L56 >> [3]: >> https://github.com/dushansilva/carbon-apimgt/tree/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/components/Base >> [4]: >> https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/BrowserRouter.js#L19-L25 >> >> Thanks >> ~KasunTe >> >> On Thu, Sep 5, 2019 at 3:29 PM Dushan Silva <dush...@wso2.com> wrote: >> >>> Hi all, >>> I have been working on $Subject. While working on it we came across the >>> following requirement. >>> >>> In the previous store implementation the tenant domain is passed on the >>> url as a query param and when either the page is refresh or if the url is >>> directly accessed with the query param tenant specific details are loaded. >>> However in the current react based single page implementation all the >>> routing is handle via react router. Here if we initially plan on keeping >>> the tenant domain in the react context api as a global setting. Once the >>> app is loaded routing can be handled easily by taking the tenant domain >>> from the context api. >>> >>> The problem is when the app is refreshed or if a url is visited >>> directly, at this occasion the context api is empty and the user will >>> always be redirect to the tenant listing. To handle this issue after >>> considering some input from the react community [1] , we came up with the >>> solution of writing an interceptor for the react router. This would require >>> us to extend the currently react router implementation and its history >>> implementation to plugin our custom interceptor. An advantage of the >>> interceptor approach is that this saves the developer the hassle handling >>> both tenant and normal cases each and every route path already added and >>> paths to be added in the future. The flow will be as follows. >>> >>> When the store is initially loaded react router should be initialized >>> [2]. Rather than using the default Router component by react router we will >>> be using our custom component BrowserRouter [3]. In this we place the >>> interceptor to be invoked when we are navigating around (push or replace) >>> [4]. The interceptor [5] will take the tenant domain from the context and >>> append it to the path. So in each route (when the user navigates to >>> different pages this will be appended as a query param. >>> >>> To retrieve the tenant specific data from the REST API , we will be >>> getting the tenant domain from the query param and setting it to the >>> x-wso2-tenant header [6]. >>> >>> Any thoughts and feedback on this much appreciated. >>> >>> >>> [1] - >>> https://stackoverflow.com/questions/57757263/how-to-write-an-interceptor-for-react-router/57757921#57757921 >>> [2] - >>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/App.jsx#L96 >>> [3] - >>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L15 >>> [4] - >>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L28 >>> [5] - >>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L43 >>> [6] - >>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/data/APIClient.jsx#L163 >>> -- >>> Best Regards >>> Dushan Silva >>> Software Engineer >>> >>> *WSO2, Inc. * >>> >>> lean . enterprise . middleware >>> Mob: +94 774 979042 >>> >> >> >> -- >> *Kasun Thennakoon* | Senior Software Engineer | WSO2 Inc. >> (m) +94 711661919 | (w) +94 11 214 5345 | (e) kasu...@wso2.com >> GET INTEGRATION AGILE >> Integration Agility for Digitally Driven Business >> > > > -- > Best Regards > Dushan Silva > Software Engineer > > *WSO2, Inc. * > > lean . enterprise . middleware > Mob: +94 774 979042 > _______________________________________________ > Architecture mailing list > Architecture@wso2.org > https://mail.wso2.org/cgi-bin/mailman/listinfo/architecture > -- *Tanya Madurapperuma* | Technical Lead | WSO2 Inc. (m) +94718184439 | (e) ta...@wso2.com <http://wso2.com/signature>
_______________________________________________ Architecture mailing list Architecture@wso2.org https://mail.wso2.org/cgi-bin/mailman/listinfo/architecture
