Follow-up for #3459235: make server not redirect when requesting a client-side-only route

But … wouldn't non-hash-based routing (like we currently have) be nicer? 🤔

It's just this silly redirect, this should be fixable.

    if (!empty($react_route)) {
      // Temporary measure so refreshing a page with a component selected does
      // not result in a 404.
      // @todo Investigate how https://www.drupal.org/project/decoupled_pages solved this same problem.
      return new RedirectResponse(Url::fromRoute('experience_builder.experience_builder')->toString());
    }

Plus, https://www.drupal.org/project/decoupled_pages already solved this.

Quoting a chat between Jesse and I:

Wim: Q: how do I let the React routing stuff know what the actual URL is? So it can do its routing?
Jesse: It should just pick it up - so hopefully the URL is still fully formed, but just Drupal takes /xb/* and always sends it to /xb/

Based on that … just removing the redirect that the server does should be sufficient? 😅

I can load e.g.

</code> just fine, but then the client runs into an error and causes a full page reload, and I'm hitting this unhandled edge case:
<code>
const NameTag: React.FC<NameTagProps> = (props) => {
  const { elementId, selected } = props;
  const model = useAppSelector(selectModel);
  const component = model[elementId];
  if (!component) {
    console.error(elementId);
    return;
  }

… and that would be much simpler to solve once #3461431: Improve client side error handling is fixed I think? 😅

@larowlan

We're definitely in the 'can't use the full URL' boat here.

Why? 🤔

If this is the answer:

At present we're not even using React router, because /xb/some/thing is redirected to /xb via Drupal so there's no 'hotlinking' or anything that was the original intention of the issue.

… then I get it. But AFAICT it really is trivial to fix that. And yes, the scope I defined in #6 is what should have been a follow-up issue before #3459235: Implement front-end (React) routing library landed 😕

Anyway … here's hoping @jessebaker can adjust the UI to respect the URL — making Drupal respect it was easy 😊

🚀

Given we have @larowlan's +1, revamping the issue summary!

Woah, looks like @jessebaker already fixed it! 😄

Now this just needs tests!

✅ Tests. Updated xb-general.cy.js passes.

(In doing so, I discovered the existing test coverage was rather broken 😅)

Cypress tests are currently too brittle, probably due to the variability of the GitLab CI runners that drupal.org operates 😬

1. First CI run: a11y.cy.js failed: https://git.drupalcode.org/project/experience_builder/-/jobs/2391963
2. Second CI run: undo-redo.cy.js failed: https://git.drupalcode.org/project/experience_builder/-/jobs/2392056
3. Third CI run: xb-general.cy.js failed: https://git.drupalcode.org/project/experience_builder/-/jobs/2392110
4. Fourth CI run: success at least: https://git.drupalcode.org/project/experience_builder/-/jobs/2392137

Fixing #14 is out of scope here. Explicitly made that part of the scope of #3456020 in #3456020-15: Harden Cypress E2E test against environment variability + document Cypress test best practices.

Thanks for the review, @jessebaker! Merged in upstream commits and awaiting final CI run prior to merging…