Generated routes ignore base paths when using pushState

[suamikim]

I'm submitting a bug report

• Library Version:
  CLI 0.26.1
  Framework 1.1.0

Please tell us about your environment:

• Operating System:
  Windows 10

• Node Version:
  6.9.4

• NPM Version:
  4.1.1

• Browser:
  Chrome 57 & FF 52 but I guess it happens in other browsers as well

• Language:
  TypeScript 2.2.1

Current behavior:
route-href bindings produce links with a leading / which makes them relative to the current domains root. This breaks generated links when using a base tag which is not /.

Here are the details:

• I'm using the routers pushState, set it's root option to /subpath and the base tag in index.html to <base href="/subpath/">
• ✔️ All links of my navigation view which are composed with <a href.bind="row.href"> in a loop over router.navigation use relative paths without a leading / which are working just fine. E.g. the href user leads to www.mypage.com/subpath/user.
• ❌ Links generated with <a route-href="route.bind: _curRouteName; params.bind: { userId: user.id }"> result in routes with leading / which makes them relative to the current domains root instead of my set base path and leads to the issues:
  • ✔️ When clicking those links, the correct URL is opened in the current tab (e.g. www.mypage.com/subpath/user/12)
  • ❌ When only hovering over the link, the browser shows an incorrect URL which is missing the base path: www.mypage.com/user/12
  • ❌ Consequently, when opening the link in a new window or tab, it opens the incorrect URL which is not working...

Browser behavior feels like a bug to me but I'm not sure since all tested browser behave exactly the same (Chrome 57, FF 52, IE 11 & Edge 38).

Despite the fact that browser behavior is really weird here, I'd suggest removing the leading / in order to make the generated URLs consistent with the ones used in the generated navigation view.

[suamikim]

I just realized that I've manually set the href attributes of my routes to relative URLs without the leading / which is why the links in the navigation view are "correct".

If I omit the custom href attributes from the routes, the generated links do also have the leading / and act the same way as the generated links from route-href...

Nonetheless this behavior is breaking my app and needs to be fixed somehow. If simply omitting the / by default would break existing code I'd suggest to make it optional.

Also, can I somehow hook into the URL generation already in order to manually remove the leading / when needed in my project?

[suamikim]

Any ideas on whether this is a browser bug or not?
Also I'd really appreciate some input on how to override Aurelias URL generation to omit the leading / manually.

[Chinchilla]

No, as far as I can see the browser is behaving exactly as it should, given the generated href.

My quick temporary fix, trying to avoid unintended side effects, was to edit route-href.js and add:

// TODO - Temporary, make href relative
if (href.charAt(0) == "/") {
  href = href.substr(1);
}

just after the line

var href = _this.router.generate(_this.route, _this.params);

in processChange()

[suamikim]

@Chinchilla

Directly editing files within the external dependencies (like node_modules/aurelia-templating-router/src/route-href.js) is very dangerous since the changes will be overwritten the next time npm install or npm update provides a new version of aurelia-templating-router.

Therefore such overrides should be encapsulated in your own code repository. I've managed to do so in the following way:

// mycode/src/overrides/app-router.ts
import {AppRouter} from 'aurelia-router';

export function apply() {
	/**
	 * Override the original AppRouter.generate function by removing leading "/"
	 * See https://github.com/aurelia/templating-router/issues/57
	 */
	(function fixGeneratedRoutes(generateOrig: Function) {
		AppRouter.prototype.generate = function(name: string, params?: any, options?: any): string {
			let href = generateOrig.call(this, name, params, options);
			
			if (href.charAt(0) === '/') {
				href = href.substr(1);
			}
			
			return href;
		}
	})(AppRouter.prototype.generate);
}

// mycode/src/main.ts
// ...
import * as AppRouterOverride from './overrides/app-router';
// ...
AppRouterOverride.apply();
// ...

I'd still appreciate any thoughts and improvement ideas on the way I implemented this override.

[Chinchilla]

@suamikim

My (quick and temporary) fix was aimed at fixing the problem I was experiencing and minimizing unintended side effects.

I agree with you that it's better to encapsulate the code in your own repository, but my concern over your fix is that it overrides the AppRouter.generate function, and I didn't have time to check that this function is only called in this circumstance.

My change, or using your override method to replace the processChange function in route-href.js, would limit the scope of the change to fix only the problem I'm experiencing without affecting other calls to AppRouter.generate

[davismj]

@AshleyGrant brought this to my attention, and I'm looking into it. If anyone has a working example that would save me the time of spinning up an instance for debugging purposes.

[sklinger]

Seems like the same problem I reported in aurelia/framework#793 - I linked a sample repository there.

[sklinger]

@davismj I think I have found at least part of the problem. It looks like a misinterpretation of a relative path starting with a slash together with a set root/base URL.

See https://github.com/aurelia/history-browser/blob/b2150087d02502bb5ee6366295456f884472a8d6/src/index.js#L144:
The root is always prepended to the fragment, even if it starts with a slash. I think it should be checked if the fragment starts with the root path in that case instead of prepending it.

The second problem I found: It appears the links generated by the following snipped (under the condition that root is set), should either generate links not starting with a slash, or prepend the root path:

<ul>
	<li repeat.for="row of router.navigation">
		<a href.bind="row.href">${row.title}</a>
	</li>
</ul>

For example with a root set to "/my-app/" - it generates links like "/page1". Yet I think they should be either "page1" or "/my-app/page1".

[AshleyGrant]

@davismj, @sklinger is knocking it out of the park!

[RomkeVdMeulen]

This problem still persists. Has there been any progress? Has this been solved in vNext?

[EisenbergEffect]

Pinging @davismj for an update on vCurrent
And @jwx for vNext

[RomkeVdMeulen]

Pinging @davismj and @fkleuver again for an update on this long standing issue.

[fkleuver]

I'd be reluctant to try to fix this in v1 right now, out of fear of introducing breaking changes that the tests may not catch.

I've added this issue to the v2 backlog to make sure we at least solve it there.