Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request: add a viewUrl property to the routes #590

Open
EisenbergEffect opened this issue Apr 24, 2018 · 13 comments
Open

Request: add a viewUrl property to the routes #590

EisenbergEffect opened this issue Apr 24, 2018 · 13 comments
Assignees
Milestone

Comments

@EisenbergEffect
Copy link
Contributor

@gordon-matt commented on Sun Apr 22 2018

As discussed in an email with @EisenbergEffect, could we please have a viewUrl property in the routes? At the moment it appears that Aurelia is simply taking the moduleId and appending a .js extension to find the model and an .html extension to find the view.

In order to use Razor routes, I make use of ViewLocator.prototype.convertOriginToViewUrl as follows:

ViewLocator.prototype.convertOriginToViewUrl = function (origin) {

    //console.log("origin: " + JSON.stringify(origin));
    var viewUrl = null;
    var idx = origin.moduleId.indexOf('aurelia-app');

    // The majority of views should be under /wwwroot/aurelia-app
    if (idx != -1) {
        viewUrl = origin.moduleId.substring(idx + 11).replace(".js", '');
    }
    // JSPM packages may need to load HTML files (example: aurelia-kendoui-bridge)
    // TODO: This is not perfect.. (what if the view we want to show is normal HTML, but not in jspm_packages folder?)
    else if (origin.moduleId.indexOf('jspm_packages') !== -1) {
        viewUrl = origin.moduleId.replace(".js", '.html');
    }
    else {
        // This is for any js files in top-level of /wwwroot directory (should point to HomeController).
        var split = origin.moduleId.split("/");
        viewUrl = split[split.length - 1].replace(".js", '');
    }

    console.log('View URL: ' + viewUrl);
    return viewUrl;
}

If you look at it carefully, you will see it’s not quite perfect. I would actually prefer to not have to do this at all.. we could have Aurelia still look for .html files by default, but if there’s a viewUrl property in the route definition, then use that instead.. That way, modules that are not being served by ASP .NET controllers and have actual HTML files (for example: aurelia-kendo-bridge and others) won’t be affected... and it will allow me to have the .js file location decoupled from the .html file location, so I can put them wherever I want and additionally, I can then use embedded javascript as well. I did that with Durandal by passing the embedded script path to the RequireJS config.. but I currently don’t see a way of doing this in Aurelia.. and I absolutely need embedded js files in my architecture in some cases.


@davismj commented on Mon Apr 23 2018

@EisenbergEffect can you move this to router please?


@stsje commented on Mon Apr 23 2018

I'm not sure a new viewUrl property will be a good name when/if the new component property is implemented. aurelia/templating-router#75
At least consider the above PR before implementing viewUrl :)!

@EisenbergEffect
Copy link
Contributor Author

EisenbergEffect commented Apr 24, 2018

Note: I think we already support this 😃 I forgot. Try setting the moduleId property of the route config to an html file path. Looking in the templating-router's route-loader, I see special code there intended to handle that scenario. @gordon-matt Give it a try and let me know if it's as you would expect.

@gordon-matt
Copy link

@EisenbergEffect When you say "Set it to an .html file path", I see 2 issues:

  1. Does that mean it needs to have an ".html" extension? Don't forget I am using ASP .NET routes and they are extension-less.

  2. Even if that works, how would I then tell Aurelia where to find the .js file (if I want it in a different location)? Don't forget I need to be able to also have embedded view models in some cases as well. For example: "My.Namespace.Scripts.some-view-model.js".. locating the embedded view is something I handle myself with an EmbeddedFileProvider, but the point is I want to be able to tell Aurelia to use that embedded path for the .js and a different path for the MVC action (to get the .cshtml content generated by Razor).

@EisenbergEffect
Copy link
Contributor Author

In that case, you probably want to build a custom RouteLoader. It's designed to be pluggable. You can start with our TemplatingRouteLoader and then modify for your needs. I don't think we can implement this in a backend-agnostic way. It sounds like you've got a very particular setup.

@gordon-matt
Copy link

Having a viewUrl property on the routes would be backend agnostic, because your implementation would be as simple as something like:

if (viewUrl) {
    // use this instead of moduleId
}
else {
    // Do what you normally do (add .html extension to moduleId)
}

This would work perfectly as far as I can tell. Am I missing something?

@stsje
Copy link

stsje commented Apr 24, 2018

Looking through the code, it looks like you can already tell it a view to use
https://github.com/aurelia/templating-router/blob/498234e1370366fe82c2de424b1c0c7cb71a077b/src/route-loader.js#L25

    let instruction = {
      viewModel: viewModel,
      childContainer: childContainer,
      view: config.view || config.viewStrategy,
      router: router
    };

So you could maybe just add a view property to your routes, or a custom view strategy?

The properties are not in the documentation.

@gordon-matt
Copy link

@stsje Thanks, I will give that a try a little later today and report back here.

@gordon-matt
Copy link

Well, I've tried the view property and it sort of works, but not quite. It seems to only work for routes that have nav: true. Also, I can't use it for app.js to use /app as the url, because obviously it's inside app.js that I start defining the other routes. I could live with this however if there wasn't the other bug I mentioned with only items that have nav: true making use of view. Anyway, I will get into that in a moment...

Firstly, I want to remind you that my app.js uses dynamic routing (for top-level routes) as follows:

import { HttpClient } from 'aurelia-http-client';
import { PLATFORM } from 'aurelia-pal';
import $ from 'jquery';

export class App {
    async configureRouter(config, router) {
        config.title = 'Aurelia';

        self = this;
        self.router = router;

        let http = new HttpClient();
        let response = await http.get("/get-spa-routes");

        $(response.content).each(function (index, item) {
            self.router.addRoute({
                route: item.route,
                name: item.name,
                moduleId: PLATFORM.moduleName(item.moduleId),
                title: item.title,
                nav: item.nav,
                view: item.view
            });
        });

        self.router.refreshNavigation();
    }
}

Notice the view property has been added as per suggestion from @stsje.

Next, I had to change the ViewLocator.prototype.convertOriginToViewUrl function as follows:

ViewLocator.prototype.convertOriginToViewUrl = function (origin) {
    let idx = origin.moduleId.indexOf('aurelia-app/app.js');
    if (idx !== -1) {
        return "app";
    }
    else {
        idx = origin.moduleId.indexOf('aurelia-app/nav-bar.js');
        if (idx !== -1) {
            return "nav-bar";
        }
        else {
            return origin.moduleId.replace(".js", '.html'); // Default
        }
    }
}

I hoped it would only be app.js that I would need to do this for, but as you can see I added nav-bar here as well, because that's not a defined route in the route config. Anyway, I have a child router for my implementation of the "Contact Manager" tutorial as follows:

import { inject, PLATFORM } from 'aurelia-framework';
import { WebAPI } from './web-api';

@inject(WebAPI)
export class ContactManager {
    constructor(api) {
        this.api = api;
    }

    configureRouter(config, router) {
        config.title = 'Contact Manager';
        config.map([
            { route: '', redirect: 'contact-manager/no-selection' },
            { route: 'contact-manager/no-selection', name: 'contact-manager/no-selection', moduleId: PLATFORM.moduleName('./no-selection'), title: 'Select', view: 'contact-manager/no-selection' },
            { route: 'contact-manager/list', name: 'contact-manager/list', moduleId: PLATFORM.moduleName('./list'), title: 'Contacts List', view: 'contact-manager/list' },
            { route: 'contact-manager/details/:id', name: 'contact-manager/details', moduleId: PLATFORM.moduleName('./details'), title: 'Contact Details', view: 'contact-manager/details' }
        ]);

        this.router = router;
    }
}

So when I browse to the contact manager, it tries to load the contact list from /aurelia-app/contact-manager/list.html instead of using the view property. It seems this must be a bug, because only the routes with nav: true honor the view property. At least, that's how it appears to be.

@stsje
Copy link

stsje commented Apr 24, 2018

So it's not just the router you want to change the view convention for, it's every component in your project? That's something the @useView decorator can do, but if you don't want to add a decorator to every component in your project, then I'm not sure where to override the convention.

@gordon-matt
Copy link

Yes perhaps I was not clear.. I am using ASP .NET Core 2.0 and want to use Razor views (.cshtml) to output the HTML. I already have a working project which you can find here: https://github.com/gordon-matt/aurelia-razor-netcore2-skeleton. You can take a look at my main.js in there and see what I am doing in the ViewLocator.prototype.convertOriginToViewUrl() function if you like.. I am proposing that instead of all of that unnecessary (and imperfect) code, we could simply specify the view URL in the route definition itself.. I mean, why couple the .js and .html files together so tightly? Of course that should be the default, but why not make it a simple thing to decouple them, so the js and html can be in totally different locations... and doesn't need to have an .html extension for the view... To me the solution seems simple, but perhaps it's not so easy to do this "under the hood"?

@Alexander-Taran
Copy link
Contributor

this should be a 'plugin'
too many options in core would be bad

@gordon-matt
Copy link

@Alexander-Taran It appears the option is already there (there's a view property on the routes), but it's not documented and not working in all cases (only when nav: true is set.

@Alexander-Taran
Copy link
Contributor

nav:true is a true mystery (-:

@davismj davismj self-assigned this May 12, 2018
@davismj davismj added this to the 1.7.0 milestone May 12, 2018
@pndewit
Copy link
Contributor

pndewit commented Oct 15, 2018

@gordon-matt I don't know if anything changed or I'm just not using everything you are, but the view property is working for me without specifying the nav property.

@stsje Thanks for looking into this, you saved my day! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants