Bug

There is an unresolvable circular dependency between Router and Route that was accidentally introduced by Router depending on Route. There was a legacy dependency of Route on Router that shouldn't have likely been there.

Package Version: v2.0.0-beta2.2

Bug

Add typings for the routing event - nav:

export interface NavEvent extends EventObject<string> {
	outlet: string | undefined;
	context: OutletContext | undefined;
}

Errors that occur while dispatching should be reported. Let's say as an error event on the router.

• Should we set up a default listener which writes such errors to console.error?
• Alternatively, if we tackle dojo/shim#21 we could take care to ensure the error becomes an unhandled rejection, and browsers will report it

Like parameters, the context type should be provided using a generic. Otherwise users will end up casting context access.

Enhancement

Upgrade to use dojo/interfaces and dojo/compose bases.

change events should only be emitted when the current value actually changes.

Is there a use case for dojo/routing to be used outside of a Dojo 2 application?

Initially the routing library was created to be standalone and provided programmatic functionality that is not used (or needed) with the later declarative Outlet routing that integrates with widgets created using dojo/widget-core.

If it is decided that dojo/routing is specifically for usage within Dojo 2 applications, we should consider refactoring the package to ensure the Outlet routing is the first class functionality from the ground up as opposed to being built on top of the initial programmatic routing solution (which meant there were some design/implementation sacrifices).

We need a README and some documentation for this repo.

@nicknisi commented on Sun Jun 03 2018

Bug

Package Version: 2.0.0

Code

An example of this can be seen at https://github.com/aldyn/dojo-tree.

Expected behavior:

Only the view associated with the current route should be displayed.

Actual behavior:

When navigating from a path (e.g. "test/1/test2/test3") to a different path, the views assigned to each path are both shown.

Currently it's null, which is crummy.

Fix dependencies as part of this.

Both in the History and in the Router, the base pathname should be configurable. E.g. the base could be https://example.com/division/project/, allowing routes to be defined for content/5. These would then match https://example.com/division/project/content/5.

Enhancement

Instead of using compose, we should change routing over to use TS 2.2 classes and mixins.

Enhancement

Is there any reason why router.append() does not return a removal handle? There appears to be no API to remove a route.

Enhancement

Should be able to define an onEnter callback for outlets that can use properties from the selected output to perform an action (such as load data for the route.)

Enhancement

To support using push state routing the Link component should handle change of route and not rely on the browser unless an target attribute has been specified

Enhancement

The initial dispatches on router.start() should be sync, currently as they are async, SSR will not be able to render the full initial page for applications with default routes.

Bug

The history manager is registered with an onChange listener in the constructor. This means that the _onChange method may be fired within the constructor, emitting the 'nav' event before listeners have been registered.

Previously, this was handled through a .start method but could be a constructor option or use a queued event manager.

Package Version: latest

Code

router.on('nav', () => {
	console.log('nav');
});

Expected behavior:

'nav' should be logged on page load.

Actual behavior:

'nav' is not logged on page load.

Enhancement

I believe that there is a use case to support "waiting" for the dispatch results to complete before executing the next route segment when a promise is returned from the dispatch function.

Currently a thenable return is supported but only used to capture and report routing errors.

Bug

Currently if the Link is not able to generate a link from the outlet specified and params provided an error is thrown.

We should consider allow this to happen gracefully and return an a element without a href.

Currently guard() can prevent a route from being selected. It should also be able to force a navigation to a different path. E.g. if the guard knows a user doesn't have access to a specific section it can redirect them to an alternative location.

When forcing navigation, the current dispatch should be aborted.

The new approach is for main.ts to export the public API, so in plain Node you can do require('dojo-routing'). However for TypeScript we'd still want you to be able to consume dojo-routing/createRoute.

• Remove interface exports from main.ts
• Export history from main.ts
• Rename _createRoute to createRoute
• Rename _createRouter to createRouter
• Update README

It'd be great if we could create a link for a route:

const router = createRouter();
const blog = createRoute({ path: '/blog' });

router.append(blog);

router.link(blog) === '/blog';

Routes should know who they've been appended to (and they cannot be appended more than once):

const create = createRoute({ path: '/new' });
blog.append(create);

router.link(create) === '/blog/new';

Routes may contain parameters:

const show = createRoute({ path: '/{id}' });
blog.append(show);

router.link(show, { id: '5' }) === '/blog/5';

Perhaps the router can keep track of the currently selected routes and their raw parameters, to avoid having to repeat parameters. Then, if it can determine which selected route directly contains the route it needs to create the link for:

const edit = createRoute({ path: '/edit' });
post.append(edit);

router.dispatch({}, '/blog/5');

router.link(edit) === '/blog/5/edit';
router.link(show, { id: '6' }) === '/blog/6';
router.link(show) === '/blog/5';

Throw if parameters are missing along the hierarchy. Assume it's OK if two routes in the hierarchy use the same parameter and fill in the same value. Make sure not to repeat query parameters.

Thoughts?

Bug

• Links are not currently generated with query params.
• When a queryParam immediately follows a param in the route configuration it is not parsed correctly. For example a path of /foo/{bar}?{qux} is parsed as a path param named bar?{qux}.

When using createHashHistory#set() a Router#dispatch() will be triggered twice (if wired) because the createHashHistory#set() emits a change, as well as the _onHashchange listener triggering a change itself. This is currently evident in todo-mvc.

Enhancement

A routing component that can hook into registered routes against a router instance that will be available via the registry. The registry will be either global or locally scoped, however for routing, using the global registry will be more common.

The creation of the application routes will be configuration driven using a simple nested structure:

const applicationRoutes = [
    {
        path: 'foo',
        children: [
            {
                path: 'bar',
            },
            {
                path: 'qux'
             }
        ]
    },
   {
        path: 'baz'
    }
];

This configuration would support the following routes:

1. /foo
2. /baz
3. /foo/bar
4. /foo/qux

Routing will provide a utility functions to create a RouterContext, register these routes against a specific router instance and allow the consumer to explicitly register the RouterContext in the desired registry.

// assuming the routes configuration above
const router = new Router(/* with history manager of choice */);
const context: RouterContext = createRouterContext(router);

registerRoutes(routes, router);

//ROUTING_KEY - symbol exported by routing
registry.define(ROUTING_KEY, Injector(RouterInjector, context));

Additionally easy to provide a further function that encapsulates all this logic and simply returns the router instance.

The routes will be controlled by using the higher order component Routeable for the component that requires routing. Routable accepts and an options argument with path, onEnter & onExit properties.

• path
  • the path chunk of the route. e.g Routeable(MyWidget, { path: 'foo' });
• onEnter
  • callback to execute when a route is entered. The route and the params will be passed as arguments
• onExit
  • callback to execute when route is exited. The route will be passed as an argument.

When a Route "matches" the current segment of the route it will render the "wrapped" component (and it's children).

Potentially usage example (assuming the route configuration about):

//MyWidget.ts

export interface MyWidgetProperties extends WidgetProperties {
    label: string;
}

export MyWidget extends WidgetBase<MyWidgetPropertoes> {
    protected render(): DNode {
        return properties.label;
    }
}

// App.ts
import { Routeable } from '@dojo/routing/Routeable';
import { MyWidget } from './MyWidget';


export RouteableMyWidgetFoo = Routeable(MyWidget, { path: 'foo' });
export RouteableMyWidgetBar = Routeable(MyWidget, { path: 'bar' });
export RouteableMyWidgetBaz = Routeable(MyWidget, { path: 'baz' });
export RouteableMyWidgetQux = Routeable(MyWidget, { path: 'qux' });

class App extends WidgetBase {
    render() {
        return v('div', [
            w(RouteableMyWidgetFoo, { label: 'foo' }, [
                w(RouteableMyWidgetFoo, { label: 'foo' }), `foo/foo` not registered so will not ever match
                w(RouteableMyWidgetBar, { label: 'bar' }),
                w(RouteableMyWidgetQux, { label: 'qux' }),
            ]),
            w(RouteableMyWidgetBaz, { label: 'baz' })
        ]);
    }
}

Router currently composes routes. An example configuration for comments a generalized comments handler might look like:

const commentsGroup = new RouteGroup({
    path: 'comments/{id}',
    defaultRoute: new DefaultRoute({
        enter() {}
    })
});
const articlesGroup = new RouteGroup({
    path: 'articles/{id}/',
    routes: [
        commentsGroup,
    ]
});
const userGroup = new RouteGroup({
    path: 'users/{id}',
    routes: [
        commentsGroup,
    ]
});

The current router will call enter() on commentsGroup and require it to handle the entire route transition. This means that commentsGroup will need to be able to resolve the entire chain and be able to fetch data for articles or users based on the path context. In this model the leaves must have complete knowledge of its entire context and as paths become longer and are reused the leaf-route's implementation becomes more complex.

Instead, I think it makes sense to allow composed routes to also compose data from parent -> child. We could do this by appending an additional method or allowing enter() to return data in a promise. In the above case if a user were to go to /articles/{id}/comments/{id} then the router would call:

1. articlesGroup.passthrough() => Promise({ model: CommentsModel })
2. commentsGroup.enter({ model: CommentsModel })

if a user were to go to /users/{id}/comments/{id} the router would call:

1. usersGroup.passthrough() => Promise({ model: CommentsModel })
2. commentsGroup.enter({ model: CommentsModel })

By transversing through the routes and collecting data we can decouple (or loosely couple) CommentsGroup from the previous routes.

Take another example: building an Express router. Express passes a http request, result, and next parameter to each of its routes. By composing routes and data together we could have a root route that initializes parameters and passes them through. For example a HTTPRequest to /users/{id}/comments/{id} in it's most complex form could look like:

1. rootHttpRouteGroup.passthrough() => Promise({ req: HTTPRequest, res: HTTPResult, stopPropigation: Boolean })
2. usersRouteGroup.passthrough() => Promise({ req, res, ..., model: UserStore })
3. userRouteGroup.passthrough() => Promise({ req, res, ..., model: User extends Commenter })
4. commentGroup.enter({ req, res, model: Commenter }) => Promise()

In the above example the Commenter type would define a property with a CommentStore and commentGroup.enter() would be able to look at the request and see what type of request it is (POST, GET) and either add to the comment store or get from the comment store. Without the ability to pass data through the route graph things quickly get complex to break apart at the leaf routes.

Enhancement

In certain scenarios it is helpful to be able to define a single Outlet that will render for multiple (or all outlets) and just inject params when from url when they exists.

Currently usage restricts the mapping of a an Outlet component to a single outlet from the routing configuration.

// example routing config
const routingConfig = [
    {
        path: '/foo',
        outlet: 'foo'
    }
];
const myOutlet = Outlet(MyWidget, 'foo');

But there are use-case where being able to create an outlet component that maps to multiple outlets within the routing config would be helpful, this could be done by accepting an array string[] as well as a single string. This will probably need a change to the Router#getOutlet function to accept the same types and when it's an array return the matching outlet if there is one.

// example routing config
const routingConfig = [
    {
        path: '/foo',
        outlet: 'foo'
    }
    {
        path: '/bar',
        outlet: 'bar'
    }
    {
        path: '/baz',
        outlet: 'baz'
    }
];
const myOutlet = Outlet(MyWidget, [ 'foo', 'bar', 'baz' ]);

Would need to think about what to do if more than one of the specified outlets match? And if that's even a scenario that should be supported.

Now that routes can request redirects we should limit how many redirects are allowed. According to this old StackOverflow post Chrome's limit is 20, which seems reasonable.

This should only be done in Router#start(). Every time a dispatch does not result in a redirect the counter should be reset.

It would be helpful to be able to call set and replace on the history manager via the router

In addition if a history object is received by the router then it could perform an initial dispatch using history.current

Review the generated API documentation for routing at http://dojo.io/api/. If any of the information is not accurate, please make the necessary adjustments to the API documentation within this repo.