MagentoRouteHandler: correct handling of unknown routes

[niklaswolf]

Description

MagentoRouteHandler now correctly returns NotFound if the urlResolver-Query returns null.
Further description see issue #1454

Related Issue

Closes #1454 .

Verification Steps

1. Edit RootComponents/CMS/cms.js:

import React, {Component, Fragment} from 'react';
import CategoryList from '../../components/CategoryList';
import {Link} from "react-router-dom";

export default class CMS extends Component {
    render() {
        return <Fragment>
            <CategoryList title="Shop by category" id={2} />
            <Link to={'/junk.html'}>Click me</Link>
        </Fragment>;
    }
}

2. Click the link in Venia
3. See that the error message reads "That page could not be found. Please try again." instead of "Something went wrong. Please try again."

Proposed Labels for Change Type/Package

• major (e.g x.0.0 - a breaking change)
• minor (e.g 0.x.0 - a backwards compatible addition)
• patch (e.g 0.0.x - a bug fix)

Checklist:

• I have updated the documentation accordingly, if necessary.
• I have added tests to cover my changes, if necessary.

[magento-cicd2]

All committers have signed the CLA.

[vercel]

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://pwa-studio-git-fork-mothership-gmbh-1451-magentoroutehandler.mmansoor.now.sh

[PWAStudioBot]

	Messages
📖	We are currently working on automating the PR metadata checks. Until that time, you may see failures related to labels/description/linked issues/etc even if you have fixed the problem. Failures will persist until the next push (assuming they are fixed).

Generated by 🚫 dangerJS against 2fa41c5

[supernova-at]

Code change is nice and concise, and thanks for adding a unit test!

Could you please provide some verification steps though?

Admittedly I'm new to this code but when I navigate to a route that doesn't exist (/junk.html) I do get a the 404 treatment:

Here it is against the latest develop: https://magento-venia-develop.now.sh/junk.html

When I navigate to /junk (without the .html), I get a much worse un-styled error message:

Not Found

Here that is agains the latest develop: https://magento-venia-develop.now.sh/junk.

[niklaswolf]

@supernova-at You don't see this error in these cases, because you do initial requests. The initial requests get resolved directly in the upward server, so you get the "not found" shell.

Only non-initial route changes get processed by the magento route handler (see linked issue #1454 ). So to force this error, you'd have to include a broken route link somewhere into the app, e.g. I included a broken link on the home page
RootComponents/CMS/cms.js

import React, {Component, Fragment} from 'react';
import CategoryList from '../../components/CategoryList';
import {Link} from "react-router-dom";

export default class CMS extends Component {
    render() {
        return <Fragment>
            <CategoryList title="Shop by category" id={2} />
            <Link to={'/junk.html'}>Click me</Link>
        </Fragment>;
    }
}

When you click the link, you'll now see a 404 "That page could not be found. Please try again." from the ErrorView component. Without the changes, it's a 500 "Something went wrong. Please try again."

Hit me up when you have further issues testing (I'm also available in the community slack ;))

[zetlen]

I'm almost 100% sure this is great, and thank you for contributing. Just had one question.

[zetlen]

This seems to rely on a literal null, and I'm not sure that's guaranteed. Any reason not to just test it for truthiness here?

[niklaswolf]

@zetlen In all of my tests null was returned, if there was no match, that's why I decided to make it rely on literal null. And I wanted to have these changes as unobstrusive as possible. But I'm also fine with just checking for truthiness. Should I change this?

[zetlen]

Yeah, would you mind?

Suggested change

	if (resolvedRoute === null) {
	if (!resolvedRoute) {

[niklaswolf]

@zetlen Applied your suggested change

[supernova-at]

Looks good to me - thank you!