veliovgroup / flow-router

🚦 Carefully extended flow-router for Meteor
https://packosphere.com/ostrio/flow-router-extra
BSD 3-Clause "New" or "Revised" License
202 stars 29 forks source link

Some templates stop rendering after upgrade 3.6.3 -> 3.7.0 #69

Closed cormip closed 5 years ago

cormip commented 5 years ago

No server-side errors triggered. No client-side errors triggered. An empty template is rendered. Downgrading back to 3.6.3 solved the problem. Details below.

Template

<template name="layoutBlankPublic">
    {{> Template.dynamic template=content}}
</template>

FlowRouter

import { FlowRouter } from 'meteor/ostrio:flow-router-extra';
import { BlazeLayout } from 'meteor/kadira:blaze-layout'

const publicRoutes = FlowRouter.group({
    name: 'public'
});

publicRoutes.route('/login', {
    name: 'login',
    action: function () {
        BlazeLayout.render('layoutBlankPublic', { content: 'login' });
    }
});

Oddly enough, a different layout template DOES work?!

Template

<template name="defaultLayout">
{{#if loggingIn}}
    {{> loading}}
{{else}}
    {{#if authenticated}}
        <div id="wrapper">
            {{> authenticatedNavigation }}
            <div id="page-wrapper" class="gray-bg">
                {{> authenticatedNavbar }}
                {{> Template.dynamic template=content}}
                {{> footer }}
            </div>
        </div>
    {{else}}
        <div id="wrapper">
            <div id="page-wrapper" class="gray-bg">
                {{> publicNavigation }}
                <p>You are not authorized to view this page.</p>
                <p>Please <a href="/login">login</a> to view this page</p>
            </div>
        </div>
    {{/if}}
{{/if}}
</template>

FlowRouter

import { FlowRouter } from 'meteor/ostrio:flow-router-extra';
import { BlazeLayout } from 'meteor/kadira:blaze-layout'

FlowRouter.route( '/dashboard', {
    name: 'dashboard',
    action: function() {
        BlazeLayout.render( 'defaultLayout', { content: 'dashboard' } );
    }
});
jankapunkt commented 5 years ago

I think you should not use BlazeLayout.render but this.render, see: https://github.com/VeliovGroup/flow-router/blob/master/docs/api/render.md

If this still does not work:

Are you shure you have properly imported the Template to render? Because a typical behaviour for Template.dynamic is to silently fail if no Template is found to dynamically render and just display nothing / update nothing on the DOM.

cormip commented 5 years ago

OK, I'll give those things a try, though this is a very old Meteor app that was structured to use the very original "folder" structure to automatically import templates based on their existence in the client folder rather than directly importing. However, it's odd that other templates render OK under 3.7.0 using BlazeLayout.render and no specific import of the template. I'll post an update once I try your suggestions.

dr-dimitru commented 5 years ago

@jankapunkt @cormip Although it's recommended to use this.render, BlazeLayout.render should work just perfectly, we have apps where BlazeLayout.render is used (old apps) and test it before every release. So far we have noticed URL not being updated upon navigation in rare cases, working on a fix now.

RomanBf commented 5 years ago

@jankapunkt @cormip Although it's recommended to use this.render, BlazeLayout.render should perfectly work, we have apps and test it

May I ask what the difference between the two is, if there is any?

dr-dimitru commented 5 years ago

@RomanBf two different Blaze-based rendering engines.

this.render features:

BlazeLayout.render features:

cormip commented 5 years ago

OK, something is definitely not right with this.render.

So far we have noticed URL not being updated upon navigation in rare cases, working on a fix now.

I am seeing this on EVERY change in navigation. I have another newer app that was running 3.7.0 without problem using BlazeLayout.render, with no imports of the templates. I switched the routes to this.render, and nothing worked correctly. Reloading the app correctly displays the template that matches the route (path), but that template never changes when navigating to other views. I tried adding imports for the layouts and templates, and it made no difference.

dr-dimitru commented 5 years ago

@cormip you got to pass layout as first argument with {{> yield}} for "main region", see docs. It's a bug workaround.

cormip commented 5 years ago

@dr-dimitru Isn't {{> yield}} a legacy Iron Router way of doing things? The Templating with "Regions" doc you referred to has a layout that seems unintuitive and backwards to me. The main body of the layout is what should be dynamic, and the sidebar and footer static templates. I don't want to refactor all my routes if this is only a temporary bug work-around.

My layout templates are structured like this:

<template name="layout">
  <aside>
    <nav>
      {{> sidebar }}
    </nav>
  </aside>

  <main>
    {{> Template.dynamic template=content}}
  </main>

  <footer>
    {{> footer }}
  </footer>
</template>

This has been working fine using BlazeLayout.render like this:

FlowRouter.route( '/dashboard', {
  name: 'dashboard',
  action: function() {
    BlazeLayout.render( 'layout', { content: 'dashboard' } )
  }
})

What is your recommended way to make this work without using {{> yield }}?

dr-dimitru commented 5 years ago

I don't want to refactor all my routes if this is only a temporary bug work-around.

Just try a single one or two routes as a test-case, or revert to latest stable release, and wait for patch with a fix.

Your examples ("old way") should be fixed with upcoming patch release

dr-dimitru commented 5 years ago

@cormip @jankapunkt please check latest v3.7.1 release, this issue should be solved.

Feel free to reopen it in case if the issue is still persists on your end.