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

View composers #2907

Open
tremby opened this issue Feb 24, 2016 · 13 comments
Open

View composers #2907

tremby opened this issue Feb 24, 2016 · 13 comments
Labels

Comments

@tremby
Copy link

tremby commented Feb 24, 2016

I posted on Stack Overflow today with a question about whether Express provides any way to automatically have data calculated and provided to a view context, per view. Apparently not. The feature is known as a "view composer" in Laravel.

The use case would be for things like data needed in every page view using a particular layout -- instead of including the same function call to get the data in every res.render call in every route which calls a view using that layout, and remembering to add it to new routes each time, add the data to the view composer associated with the layout view containing the widget in question. Or for a widget in a view partial used sporadically across the site which requires some database lookup or other calculation, that calculation is done in the view's composer rather than the developer having to remember which pages have the widget and which don't, and having the calculation or reference to it in all of the relevant route files. And remembering to remove that from the view context if the widget is removed.

It's also nice in the case where the views and the business logic are written by different people, like when the designer is the front end developer. The designer may decide to add or remove such a widget at some point, and now the relevant data is calculated and available, or no longer calculated when no longer needed, without the route having to be edited.

I'm wondering if this is a feature others would be interested in, and how feasible it might be. Since there are myriad view engines for Express, and presumably they all handle partials and layouts in different ways, I expect this would work as a pre-render callback which is registered through Express.

That is, the developer writes something like:

app.set('view engine', 'twig');
app.set('view composer', function (view, context) {
  // Whatever logic the developer wants, given the view name
  // (or filename/path?), adding to or transforming the context
  // such as this trivial example...
  switch (view) {
    case 'layout':
      context.loggedIn = auth.isUserLoggedIn();
      break;
    case 'widgets/date':
      context.date = new Date();
      break;
  }
});

This callback would then be called (at the responsibility of the view engine, presumably) at some point before each view file is rendered. The context can therefore be added to, with whatever data the view needs.

Obviously with suitable logic the developer could break these out into separate files to keep things organized.

Or alternatively there could be a standard for an event fired at an equivalent suitable time, and with similar arguments, which could be listened for by the developer.

Any thoughts?

@Twipped
Copy link

Twipped commented Mar 1, 2016

Typically in the express ecosystem you wouldn't handle this via a view level condition, but by creating a middleware that adds the needed data to res.locals (which the view would receive) and including that middleware in the routes that need it.

@tremby
Copy link
Author

tremby commented Mar 1, 2016

Well that's exactly what I'd like to avoid. I don't want to have to keep track of which routes lead to views which (possibly many includes down the chain) include a particular partial.

I don't see it as a view level condition as such. It would add the possibility for logic to run as a direct result of particular views being included. It'd be more efficient than running this logic on all routes, and easier than keeping track of which routes it's necessary for.

Is what I suggest against the express way of doing things, in some way?

@Twipped
Copy link

Twipped commented Mar 2, 2016

Not necessarily, no. However I believe the plan is to actually reduce the footprint of Express' view engine and possibly even remove it in the future (someone else can chime in if that's wrong). What you're proposing would probably be better handled via a third party middleware that overrides response.render()

@wesleytodd
Copy link
Member

I think what @ChiperSoft is talking about is this: pillarjs/discussions#2

I agree that overriding response.render() is the probably best approach here. You can do literally exactly what you are doing in the above code example with a third-party module. Here is a quick example:

app.use(function (req, res, next) {
  var _oldRender = res.render;
  res.render = function (view, locals, cb) {
    switch (view) {
      case 'layout':
        locals.loggedIn = auth.isUserLoggedIn();
        break;
      case 'widgets/date':
        locals.date = new Date();
        break;
    }
    return _oldRender(view, locals, cb);
  };
  next();
});

Would that solve your issue?

EDIT: code fixes

@tremby
Copy link
Author

tremby commented Mar 2, 2016

@wesleytodd: I'm trying out your code, but it's erroring.

As is, I get an error TypeError: Cannot read property 'req' of undefined at node_modules/express/lib/response.js:939:17, which is the second line in this snippet:

res.render = function render(view, options, callback) {
  var app = this.req.app;

I tried changing the var _oldRender = res.render; line to var _oldRender = res.render.bind(this); and now I get TypeError: Cannot read property 'app' of undefined at the same line of response.js.

@tremby
Copy link
Author

tremby commented Mar 2, 2016

Never mind; what we wanted was .bind(res). Evaluating now...

@wesleytodd
Copy link
Member

Not sure what your this context is, but in the example above I dont think** you need to bind anything. It has been a while since I have actually used this in an app, so keep in mind that I just wrote that example off the cuff, so you might need to modify it a bit to do what you really want. :)

@wesleytodd
Copy link
Member

oops, just saw your other response, and yeah, I see what is happening. Your change is correct.

@tremby
Copy link
Author

tremby commented Mar 2, 2016

I'm afraid this doesn't do what I want.

I've modified it to just print out the view names as they are rendered. In one particular case it is printing login, since that is the page being rendered, but then prints nothing else: no partial names, no layout name. Obviously the render method of the res object isn't called again for each partial and for the parent layout. I'm not sure if this would be any different for other templating engines.

@wesleytodd
Copy link
Member

If you want the stuff that Laravel provides I think you might need to look to another view engine. I think there was another issue about something similar to this recently, but I am having trouble finding it. I believe that handlebars can do this...

@tremby
Copy link
Author

tremby commented Mar 2, 2016

I'm aware that view engines would have to provide some support for a feature like this. My main point in this thread is that it would be nice to have a standard for this which the view engines which want to support it could then follow.

@wesleytodd
Copy link
Member

My guess would be that this is considered out of the scope of Express. Especially with the conversation happening in the pillarjs org about abstracting the whole view layer out of express core, but someone with more knowledge is welcome to come correct me :)

@hacksparrow
Copy link
Member

I agree with @wesleytodd, this is better discussed at pillarjs/discussions#2. Might be a good feature to have in a future version of the view system.

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

No branches or pull requests

4 participants