bloomreach / spa-sdk

Apache License 2.0
15 stars 16 forks source link

Page in BrPage Component not being updated #22

Open IsabellaZaby opened 11 months ago

IsabellaZaby commented 11 months ago

Hi,

we noticed an issue in our project using the react-sdk and spa-sdk. When we pass the page as a prop it sometimes happen on a route change that the page is not being updated even though we pass the new page object to the component. We investigated a little further and realised that the page is only set once into the state in the constructor (see link) and not updated in componentDidMount directly (only through initialize).

https://github.com/bloomreach/spa-sdk/blob/4436a89797c72c1f5085c9ead3bef9c0e8d67b75/packages/react-sdk/src/page/BrPage.tsx#L64

As far as I understood the destroy and initialize should take care of it but unfortunately it does not work. Could you please take a look? Thank you.

beetlerom commented 11 months ago

@joerideg FYI

joerideg commented 11 months ago

hi @IsabellaZaby ,

I can check it out, could you please tell me what your context is:

IsabellaZaby commented 11 months ago

Hi @joerideg

We do have a useMemo on the configuration with config.path as a dependency to update it when the path changes. I checked it again: we do pass the new configuration to the BrPage component as well as the new page we fetched on the server but the BrPage component from the sdk is not updating anymore, since it initialised with the old page and configuration.

joerideg commented 11 months ago

Hi @IsabellaZaby , could you please update to the latest and greatest SDK versions (22.0.2 as of writing) and let me know if that resolves your issue? You can read on how to migrate here: https://bloomreach.github.io/spa-sdk/docs/migration-guide/

IsabellaZaby commented 11 months ago

Hi @joerideg ,

did the upgrade – unfortunately the issue is still there.

joerideg commented 11 months ago

Alright, is it possible for you to create an isolated reproduction path, using stackblitz for example, so I can take a closer look? Updating the page seems to be working for our tested use cases (CSR, SSR using Next.js), so its probably specific to your setup.

IsabellaZaby commented 10 months ago

Hi, sorry for the delay I was on vacation. No unfortunately I cannot easily create it for you in stackblitz since our SSR Framework is not open source. The only thing I can provide is our hook where we fetch the page (which creates a request to our Node.js which then does the actual request to the resource api) and our BrPage component which is a wrapper for the SDK-BrPage component. But I am not sure that will help you cause as said we do provide the new configuration and page to the SDK-BrPage component but it doesn't update.

joerideg commented 5 months ago

@IsabellaZaby are you still experiencing this issue? One thought I had was that you might be able to recreate the issue, whether its SSR or not, as the SDK is agnostic as to where it renders. Maybe use plain react or Next.js to reproduce the issue?