Proper building of query string inside EditingRenderMiddleware

[Antonytm]

Description / Motivation

I want to override default resolvePageUrl and add GET parameter.
I use the way described in the official documentation:
https://doc.sitecore.com/xp/en/developers/hd/200/sitecore-headless-development/override-default-protocols-using-the-editingrendermiddleware-in-next-js-jss-apps.html?utm_medium=rss&utm_source=dev.sitecore.net

const handler = new EditingRenderMiddleware({
  resolvePageUrl: (serverUrl: string, itemPath: string) => `${serverUrl}${itemPath}?someMyKey=${myKeyValue}&`,
}).getHandler();

Why do I need this parameter: I need to indicate to some components that the request came from EE/Horizon.
When I add only someMyKey=${myKeyValue} it doesn't work because of adding ?timestamp=${Date.now()}. The result string will have 2 ? signs, which is not right. I can do some tricks and add ?someMyKey=${myKeyValue}&(with ampersand an at the end). It will give me 2 parameters: someMyKey and ?timestamp.

As timestamp parameter is added only to avoid cache, it doesn't matter how it will be named: timestamp or ?timestamp. It makes this change very minor. But if we can have a proper GET parameter name, why don't have it?

Testing Details

1. yarn install
2. yarn build
3. Copy only editing-render-middleware.js
4. Add logging of URL (to make sure that it was changed in the proper way)
5. Try different actions in Experience Editor (adding and removing different components)
6. Review logs: look for "timestamp"

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

[ambrauer]

Hi @Antonytm! The change makes sense to me. While we never anticipated query strings in the resolvePageUrl output, we should definitely be defensive and handle this case.

Thanks for the contribution 😊

However, in your case I'm curious why not use other means to detect editing context? We have the pageEditing prop available on the Sitecore Context (you can access either through a React hook or HOC, see here for example) and also isEditorActive for detection in the browser.

[Antonytm]

@ambrauer

Business feature

1. We want to have accordion/tabs/menu/etc. Anything that has 2 states show and hide
2. We want to have placeholders inside this shown/hidden area
3. It is controlled by code like this

{someVariable && { <div>
   <Placeholder name='some-placeholder' rendering={rendering} />
</div>}

4. We want this placeholder to work properly in EE and Horizon

---

Technical details

We need render endpoint to behave differently. It should always render all components. Otherwise adding components to the page will not work.

We can achieve it when both these conditions are true:

1. It is editing context
2. It is Next.js on server context

Initially, I wanted to achieve it by using isEditorActive and process.browser (Next.js-way to define server or client).
But I isEditorActive didn't work for me. It was always false on the server but was true on the client. I haven't checked sitecoreContext.pageEditing, because I thought it is the same, just alias for HOC.
I didn't look at its implementation and thought it was by design.

Now I looked into code, made tests and see that isEditorActive() != sitecoreContext.pageEditing

    if (isServer()) {
      return false;
    }

and my output on the server-side:

xxxxx     | route: /presentation/partial-designs/header?serverEditingContextKey=gitjmwphpa&%3Ftimestamp=1656064569429
xxxxx     | isEditoreActive: false
xxxxx     | sitecoreContext.pageEditing: true

Yes, I see that I made an overkill. Thank you for pointing me out.

---

In order to save time for developers, who will face same question in future:

Features request:

1. Add to the documentation that isEditorActive will work only in browser
2. Split isEditorActive into 2 functions isEditorActiveClientSide and isEditorActiveServerSide (or alternative naming isEditorActiveBrowser and isEditorActiveServer). (I understand that it is a breaking change, we can leave isEditorActive, mark it as deprecated and point to isEditorActiveClientSide implementation.)

1st I think should be necessary. 2nd is probably too edge case, but it is up to you.

I will be able to help you with implementation if you think these changes will be valuable.

[ambrauer]

@Antonytm I agree, we should be clear in the docs that isEditorActive only works in the browser. I've added a backlog item for us to address this in an upcoming release. Great feedback, thanks!

Option 2 isn't really feasible since how you access the Sitecore Context in each framework differs.