Aurelia Router should support ignoreUnknownRoutes-like feature

[plwalters]

The router for Durandal included an option to ignoreUnknownRoutes IIRC. If a link is clicked on and Aurelia doesn't have a matching route the request should be able to continue on to the server by having Aurelia completely ignore unknown routes.

Perhaps this could take a function to override the default behavior so the user can configure more thoroughly.

[davismj]

@PWKad A couple questions:

• If navigation to /my/url occurs and /my/url isn't configured, the request falls through to a normal navigation request by the browser for /my/url, correct?
• This shouldn't have any effect unless on a pushState app, correct?
• What would the "default behavior" be? What could you override it with?

[plwalters]

Right now the router hijacks any request that comes through when push state is active at the same base URL IIRC and the only way to allow the server to handle the request so the router doesn't swallow it is to map unknown routes and return a fake promise that never resolves, which is ugly.

Instead the router should allow an option to not throw if a matching route isn't found which could be called something like ignoreUnknownRoutes I thought this was a feature from Durandal IIRC but I may be mistaken but it's needed for push state apps where the request might be serviced by the backend instead.

[pndewit]

@PWKad Thanks, your workaround works! 👍

If anyone is interested, I use the following piece of code inside my configureRouter(routerConfiguration, router) function:

routerConfiguration.mapUnknownRoutes(() => {
    if (location.pathname.startsWith('/my-route')) {
        return './app/not-found.html';
    } else {
        // REALLY ugly workaround (introduces memory leak) to ignore unknown routes outside "/my-route"
        this.dummy_promise = this.dummy_promise || new Promise(() => {});
        return this.dummy_promise;
    }
});

this.dummy_promise is used (as @PWKad explained) as fake promise which never resolves. This fake promise is then reused for next calls to prevent the app from creating a new Promise object for each navigation call to an unknown route. This will only create another pointer in memory to that same fake promise (which of course is still a memory leak), but only a minor one.

[davismj]

@mgraf1

#558 doesn't seem to solve this issue

(1) created a push state enabled application
(2) added a link to a local path file (build/paths.js)
(3) click the link, the router quietly navigates to build/paths.js, but the request does not fall through to the server

[plwalters]

I added a new PR as a work in progress to address this but as you mentioned it should fall through. https://github.com/aurelia/router/pull/561/files

[Alexander-Taran]

I guess #561 is not merged because of conflicts.
can't close issue then?

[plwalters]

@Alexander-Taran The issue was addressed and an issue was found in the PR so the work was reverted without ever being tested or worked on further, going to close this and other issues around the router to get the issues down. PushState is borderline unusable without a hacky workaround so I would advise not using pushstate unless you absolutely have to because it makes "embedding" your aurelia app in to a larger server rendered app much more difficult.

[rquast]

@PWKad #527 (comment) - but I need pushState.
You can't do opengraph or SSR that requires fragment information because it doesn't go to the server.
I guess I'll have to hack my download links for now to make them work.

[bigopon]

IMO, this should be solved in browser history via a new API on it. Could potentially be an API on abstract class History from aurelia-history too. Something like this:

class History {
  ignore(...routes: string[]): void;
}

So when History implementation detects any changes that falls within ignore patterns, it simply ... ignores it and doesn't notify the router.

[bigopon]

Another option is to have an additional check in loadUrl at

router/src/app-router.ts

Lines 62 to 70 in 9ba6ef3

	loadUrl(url: string): Promise<NavigationInstruction> {
	return this
	._createNavigationInstruction(url)
	.then(instruction => this._queueInstruction(instruction))
	.catch(error => {
	logger.error(error);
	restorePreviousLocation(this);
	});
	}

If there is a config for ignoreUnkownRoutes on app router, then it does a check with RouteRecognizer to see if a URL is supported before processing further.

Something like this

  loadUrl(url: string): Promise<NavigationInstruction> {
+   if (this.ignoreUnknownRoutes) {
+     if (!this._recognizer.recognize(url) && !this._childRecognizer.recognize(url)) {
+       return Promise.resolve(null);
+     }
+   }
    return this
      ._createNavigationInstruction(url)
      .then(instruction => this._queueInstruction(instruction))
      .catch(error => {
        logger.error(error);
        restorePreviousLocation(this);
      });
  }

Thoughts? @EisenbergEffect @davismj @fkleuver @jwx

[jwx]

IMO, this should be solved in browser history via a new API on it. Could potentially be an API on abstract class History from aurelia-history too. Something like this:

class History {
  ignore(...routes: string[]): void;
}

So when History implementation detects any changes that falls within ignore patterns, it simply ... ignores it and doesn't notify the router.

@bigopon I don't think I agree that it should be added to History. In my opinion, History should be responsible for detecting and performing navigations. Its API should reflect this by making it possible to get reports of navigations, including their relevant data, and to perform navigations (and/or update relevant data). How to act on those navigations, or when to do navigations, should be decided by something else, in this case Router. If Router however decides to ignore a report about a navigation as early as possible, it should still be possible to get a solution that doesn't get into Router's routing and all the lovely details that come with that. :)

[bigopon]

@jwx Without touching the lovely details of routing of Router, I see one possible API:

ignoreUnknownRoutes(pattern: Array<Regex | ((route: string) => boolean)>): void;

Then in user code, probably just supply some manual process:

  configureRouter(config: RouterConfiguration, router: Router) {
    config.ignoreUnknownRoutes([ /\*pdf$/ ]);
    // or
    config.ignoreUnknownRoutes([ route => route.indexOf('.pdf') !== -1 ]);
  }

[plwalters]

@bigopon I PR'd almost that same code a year and a half ago - https://github.com/aurelia/router/pull/561/files - to loadUrl and it was rejected and all code immediately reverted by the listed technical maintainer of the repo @davismj because he didn't have time to test or review it. I think the approach solves the problem from a pragmatic perspective and handles this case that I have run in to all the time in larger Aurelia projects.

I would recommend getting Matthews buy-in first before working on anything so you don't end up wasting your time to have it rejected later ;)

I'm not going to close this issue again just to have it re-opened or whatever but I am going to unsubscribe so please don't @ me ;)

[bigopon]

@PWKad thanks for the reminder. There are some small differences between 2 versions, The earlier tries to create an instruction, and then check if it should proceed. The later tries to check if it should proceed before trying to create an instruction.

I would say the later is safer and less side-effect prone.

[davismj]

for more context: #558

I'm all for it! It just didn't work. I tried the code but it wouldn't let me download a file from the server.

[plwalters]

"it just didn't work" - yes it did, I don't appreciate your first comment after a year and a half of saying you wanted to wait until you could test it more being that it didn't work. I asked you not to revert the PR.