bloomreach / spa-sdk

Apache License 2.0
15 stars 16 forks source link

React SDK breaks Preview as it does not create a new Page after destroying the first Page when a page prop is also provided #15

Closed mikroware closed 11 months ago

mikroware commented 1 year ago

We updated our React SPA from 19.0.2 to 21.0.0 to get the SSR functionalities back, however it seems like the Experience Manager is broken now.

This is in the browser console:

[SPA] [INFO] Enabled token-based setup.
[SPA] [INFO] Using Page Model API 1.0.
There is no channel detected in the page metadata.
[SPA] [INFO] Running in preview mode.

The 3rd line comes from the angular.js file. So I assume something in the manager between iframe and API did break in the update?

Additionally: I can see the request to the page model being done with the token in the custom authorizationHeader we defined in the configuration and in the CMS. This setup worked for us on 19.0.2.

joerideg commented 1 year ago

Hi @mikroware , I'm sorry to hear you are having trouble after the upgrade. I just checked our example apps and they seem to be working as expected in our test environment, so lets dive a little deeper into your setup.

Could you please enable 'verbose' logging in chrome dev tools and output the logs of the page load so I could inspect the order of messages, judging by the logs you already shared I suspect that the page.sync event is fired before the page is fully rendered for some reason as the log would indicate that the page model (as parsed by the experience manager after page.sync is fired) does not have a meta or id, which means that page variable is likely undefined.

Could you please also share the configuration object you are passing into the initialize call and BrPage component?

mikroware commented 1 year ago

Hi @joerideg

Thank you for your answer! We indeed have a little bit of a customized setup. Which did work fine on previous versions.

You are suggesting that the page.sync could be fired before the page is fully rendered. I am not sure where this page.sync is called, but it could definitely be before our page render. Basically what we are doing is calling initialize ourselves in order to chain on the promise for custom loading and errors handling. The received page model is stored as state and passed to the BrPage together with the exact configuration we called initialize with.

Our configuration object is probably nothing special:

{
    endpoint: `${brHost}${route}/resourceapi`, // Building our root, brHost is environment dependent, route is the language
    authorizationHeader: 'HST-Authorization', // This is also in our API config
    httpClient: api, // Api is a custom axios instance with pre- and post-hooks for header handling
    path: `${path}${location.search}`,
}
joerideg commented 1 year ago

So page.sync is called in the BrPage component in the componentDidUpdate hook.

Would you be able to output the javascript console logs in chrome (or another browser) with the dev tools logging set to 'verbose' that should output a bunch of extra logs including the events being fired and processed between the SPA and the experience manager.

In the mean time I will generate a clean react 18 project and see if I can reproduce the problem with the information you gave me so far.

joerideg commented 1 year ago

Hi @mikroware ,

I've generated a minimum setup with React 18 and Bloomreach here. You can see in the App.tsx that i'm separately generating the page object through initialize and passing it to the BrPage component. This example works with our preview as far as I can see, there are no errors or unexpected behavior. If you do clone it and try it out make sure you change the endpoint config.

Feel free to create an MR to make the example app reflect your setup more accurately.

mikroware commented 1 year ago

Hi @joerideg,

Thank you for the effort! I cloned your project and changed a few things to make it run on our local hosted cms instance. When opening the console, I see exactly the same 4 log lines as I posted in my initial post. The code also suggests there should be a BrManageMenuButton, but I don't see it appear either.

What I changed was:

mikroware commented 1 year ago

Is there anything else I could do to make the issue more clear? I cannot find any other debug configuration than debug: true which is already on. So the logs I showed are all I got.

joerideg commented 1 year ago

Alright @mikroware, I followed your changes and while debugging the app it seems that when setting a proxy like that it breaks the integration with the preview. I realized that the endpoint parameter string is required to represent a full URL, specifically including the origin.

For the preview integration we set up an event message bus that uses window.postMessage and we set the targetOrigin to the origin of the endpoint for security.

I suggest you set your endpoint parameter to a full url. I assume you want different urls for localhost vs live and production, you should be able to use an environment variable for that.

For example I could set the endpoint in that app I build to endpoint: import.meta.env.VITE_BRXM_ENDPOINT as string and set that VITE_BRXM_ENDPOINT environment variable (read the docs for vite env variables for the gotcha's if you are going to try it out) and it should allow me to set the variable between different environments.

You mentioned that it worked before but I think there might have been some slightly different setup because this check and code has not changed since 2021-01-04 according to git blame.

I will close the issue for now, feel free to request to reopen it if you feel this is premature.

mikroware commented 1 year ago

Vite was used in the minimum setup repo which you created. However, we don't use vite yet ourselves. We plan to use it with SSR soon, but currently we are still on CRA and http-proxy-middleware.

Additonally, in our full setup, we do prepend the endpoint with the current host. And in production we already set the full url with the FQDN. If I remember correctly, we had problems with the experience manager before which sounds like what you described, hence we already moved to full urls and also had to move to the same domain.


Just did some more testing in our application, this time in our SSR feature branch, with vite. Here we also use the same full url and basically the same setup.

What appears to be the case, is that the problem is fixed when removing strictmode. Strictmode caused the functional component which does call initialize to be mounted twice. And thus also call initialize twice. When moving the strictmode down the render tree (under BrPage), it started working.

Your minimum setup repo also has strictmode down the tree, but still as parent of the BrPage. When being a parent of the BrPage, the problem persists for me. When making the strictmode a child of the BrPage, it all starts working.

jwgmeligmeyling commented 1 year ago

@joerideg could you respond to the above findings that still persist for us in the latest release?

joerideg commented 1 year ago

Hi @mikroware and @jwgmeligmeyling I'm sorry for my lack of reply, I should have at least made some acknowledgement of your finding.

I did indeed confirm that Strict mode calls initialize twice and that in doing so it will destroy the 'previous' Page object in memory, but not create a new one because a 'Page ' was already provided.

This seems like a setup/situation we have not taking into account in the react-sdk and we need to fix. I'm reopening the issue and am already working to fix the problem.

joerideg commented 1 year ago

I've updated the issue title to reflect the problem to fix

joerideg commented 1 year ago

@mikroware while creating a ticket in our internal JIRA and trying to formulate the reproduction path I fail to actually do so.

I thought that destroying the Page after the first render, and not creating a new Page object on the second render was the/a problem but it is actually not so. After all you already have the Page object from the server side (or a useEffect hook of some sort) passed in through the page prop. Therefore it will use that model to hydrate the page instead of doing a new request.

Again in my local setup I see that the preview works just fine using the webapp in the repo as well as our 'next.js' example app.

Since I am not able to reproduce your issue it might make sense to try and do some live video call where you could show me some of your code/setup? Is there an email or other channel I can reach you on to set this up? Thanks!

mikroware commented 1 year ago

We could do a call, definitely! You can reach me at mirko.dunnewind@athenastudies.nl. And of course, you can also come by our office for a coffee; we are in the same street.

For purpose of illustrating, I did fork your testing front-end and did some minimal edits (which I described above) to make it work with our local cms. This setup does expose the problem for me already. And is fixed when I move down the StrictMode as a child of BrPage. See: https://github.com/mikroware/react-18-bloomreach

joerideg commented 11 months ago

@mikroware as discussed, we are waiting for a more information or other clients experiencing the same issues. For now I'm closing this issue.