veliovgroup/flow-router

FlowRouter.setQueryParams() always causes full route re-render

drone1 opened this issue · 32 comments

drone1 commented

I'm having an issue:

  • Give an expressive description of what is went wrong
    FlowRouter.setQueryParams() always causes full route re-render
  • Version of flow-router-extra you're experiencing this issue
    3.7.5
  • Version of Meteor you're experiencing this issue
    2.5.5
  • Browser name and its version (Chrome, Firefox, Safari, etc.)?
    Chrome Version 97.0.4692.99 (Official Build) (64-bit)
  • Platform name and its version (Win, Mac, Linux)?
    Windows 10, WSL1

I'm trying to use FlowRouter.setQueryParams() without causing a re-render. I'd simply like my Tracker, which watches the query string via FlowRouter.getQueryParams(), to catch query string updates. Is there a particular pattern or other recommendation for doing this?

I tried throwing my call to FlowRouter.setQueryParams() into a FlowRouter.withReplaceState() callback just for the hell of it but no luck.

Thanks!

dr-dimitru commented

Hello @drone1 ,

I'm trying to use FlowRouter.setQueryParams() without causing a re-render

It depends from how you render, can you post your code here?

drone1 commented
dr-dimitru commented

@drone1 really would need to know how do you render it. Do you use this.render() or blaze-renderer package?

drone1 commented
dr-dimitru commented

@drone1 have you updated to the lates release? Perhaps it was fixed

drone1 commented
dr-dimitru commented

We would need a reproduction repo for this one, or at least post router definition

drone1 commented

OK, got some code together for you.

routes.js:

import './foo.js'

// routes.js
let count = 0
let firstRender = true

FlowRouter.route('/foo', {
	forceReRender: false,

	action: function (params, query, data) {
		console.log('test')

		this.render('foo', { count })
		count++

		if (firstRender) {
			// Only do this once:
			setInterval(
				() => FlowRouter.setQueryParams({ test: count }),	// Causes re-render
				2000
			)
			
			firstRender = false
		}
	}
})

foo.html:

<template name="foo">
	render {{count}}
</template>

foo.js:

import './foo.html'

Many thanks.

dr-dimitru commented

This isn't 100%. You might be able to solve it by using layout template with {{>yield}}

drone1 commented
drone1 commented
dr-dimitru commented

@drone1

is the explanation for exactly what yield does?

yield helps to render as passthrough data to your template, there are some improvements into rendering process in order to optimize it and avoid re-rendering of entire page the it isn't necessary. Also it cache and reuse previously loaded templates and layouts

drone1 commented

Thank you so much for explaining. Okay, I have clearly been missing out on something critical here. Maybe I missed something but I feel like this could be made much more clear in the documentation? This all seems extremely useful!

I'll be sure to test and let you know how it goes.

By the way, isn't this unreachable code? At a glance it looks like it may be.

flow-router/client/renderer.js

Line 348 in e2cbb67

current.materialized = false;

and

flow-router/client/renderer.js

Line 358 in e2cbb67

current.materialized = false;

(LMK if you want me to open a separate issue)

drone1 commented

Quick question: I'm trying to assess the amount of work this would be to refactor my application to use yield.

Is it possible to yield a child of the layout? Or does it have to be in layout (and not a child of layout)?

I've_ got a main application layout which calls Template.dynamic with another layout. And then the given page is rendered within that.

Can I use yield with this structure, or would I need to refactor? Thank you.

dr-dimitru commented

@drone1

this could be made much more clear in the documentation

Sure, feel free to send a PR

By the way, isn't this unreachable code?

Why? Changing object property to read it on the next/other cycle

Is it possible to yield a child of the layout?

you can create empty template, with just {{> yield}}

Or does it have to be in layout (and not a child of layout)?

Since "layout" is template, and "templates" are following HTML tree structure — everything is a "child" of layout

I've_ got a main application layout which calls Template.dynamic with another layout. And then the given page is rendered within that.

  1. Replace Template.dynamic with {{>yield}}
  2. Use .render() passing layout and template names, instead of changing/assigning variable passed to Template.dynamic

Hope that helps

drone1 commented

Sure, feel free to send a PR

As I have a relatively vague understanding of what yield does, I do not think I am a great person to write the documentation.

Replace Template.dynamic with {{>yield}}
Use .render() passing layout and template names, instead of changing/assigning variable passed to Template.dynamic

(We're getting off-top but this is very helpful to me - thanks!): Thank you for these suggestions. I've done this, but my application is still calling "action" on routes which should be "re-used" (forceReRender: false) -- am I doing something wrong?

app renders layout renders template

Currently, after your suggestion, app renders layout using {{> yeield }}

But since most of the heavy lifting happens in 'template', do I need to refactor and merge app and layout or can I put a yield in app and in layout?

I have tried it, and it did not work. Use of yield is currently not supported in this way, yes? I am just trying to make sure.

To return to the topic at hand, I have updated the example to use yield and FlowRouter.setQueryParams() still causes a full re-render. Count increases.

foo.html:

<template name="layout">
	{{> yield}}
</template>

<template name="foo">
	render {{count}}
</template>

routes.js:

// routes.js
let count = 0
let firstRender = true

FlowRouter.route('/foo', {
	forceReRender: false,

	action: function (params, query, data) {
		console.log('test')

		this.render('layout', 'foo', { count })
		count++

		if (firstRender) {
			// Only do this once:
			setInterval(
				() => FlowRouter.setQueryParams({ test: count }),	// Causes re-render
				2000
			)

			firstRender = false
		}
	}
})

Thanks.

dr-dimitru commented

@drone1

  1. action method will be always called on URL change, this templating technique is only regarding how's actual .render() of a Blaze template
  2. How would you explain "full rerendering"? What is it exactly in your case? (perhaps your screen record is the fastest way to explain)
drone1 commented
dr-dimitru commented

@drone1

app renders layout renders template
Currently, after your suggestion, app renders layout using {{> yeield }}

not sure how this would be possible, I believe template will override the app template. In short — place {{>yield}} inside whatever template is passed as a first argument into .render() method

But since most of the heavy lifting happens in 'template', do I need to refactor and merge app and layout or can I put a yield in app and in layout?

I suggest to figure out "re-rendering issue" on empty template before making things complex

I have tried it, and it did not work. Use of yield is currently not supported in this way, yes? I am just trying to make sure.

Having more than one {{>yield}}, shouldn't work (in theory)

To provide further support I'd need to have clear understanding what do you mean by full re-rendering

drone1 commented

yield helps to render as passthrough data to your template, there are some improvements into rendering process in order to optimize it and avoid re-rendering of entire page the it isn't necessary. Also it cache and reuse previously loaded templates and layouts

What did you mean by avoid re-rendering of the entire page the it isn't necessary? That's what I mean by full re-rendering. :)

dr-dimitru commented

@drone1 I need to see visual or minimal reproduction code. Why do you think it's fully re-rendered? Do you see elements removed from DOM and placed back again?

drone1 commented

I was able to optimize my application by caching results of waitOn(). I will start a new issue if I find something odd, but this made a huge difference. So I'm good for now.

So, back to the topic of this issue. :)

Reproduction code for this issue is in my comment above.

The routing still increases the 'count' every 2 seconds (see setInterval), because calling FlowRouter.setQueryParam() causes the page to re-render.

Would you not expect this? There is nothing reactively getting the query parameter.

dr-dimitru commented

Reproduction code for this issue is in my #94 (comment).

The routing still increases the 'count' every 2 seconds (see setInterval), because calling FlowRouter.setQueryParam() causes the page to re-render.

action() hook is expected to run on every URL change

Would you not expect this? There is nothing reactively getting the query parameter.

Black box, there are no details about your template an its structure

was able to optimize my application by caching results of waitOn(). I will start a new issue if I find something odd, but this made a huge difference. So I'm good for now.

You had waitOn and never mentioned it...

drone1 commented

action() hook is expected to run on every URL change

So now you literally just answered the initial question: If this.render() is called within action(), FlowRouter.setQueryParams() will cause a page to re-render. Is that correct?

Black box, there are no details about your template an its structure

I don't think we are talking about the same thing here. It's not a black box -- I have included full reproduction source code for you in this thread twice.

waitOn I have in my application, not in the reproduction code I have included twice pertaining to the specific topic this issue was created for, which is FlowRouter.setQueryParams() causing pages to re-render. Perhaps you can try running the reproduction code and seeing if it's behaving as you might expect. Thanks!

dr-dimitru commented

So now you literally just answered the initial question: If this.render() is called within action(), FlowRouter.setQueryParams() will cause a page to re-render. Is that correct?

No, action() hook is always called on URL change

I have included twice pertaining to the specific topic this issue was created for, which is FlowRouter.setQueryParams() causing pages to re-render

Does it mean that if you call .go() method or click a link — there are no re-rendering, is that what you mean?

dr-dimitru commented

@drone1
Feel free to close it in case if the issue is solved on your end.

dr-dimitru commented

Hello @drone1, have you found a way to solve this on your end?

dr-dimitru commented

Closed due to silence at issue owner end.
Feel free to reopen it in case if the issue still persists on your end.

drone1 commented
dr-dimitru commented

@drone1 templating docs are self-explanatory

dr-dimitru commented

@drone1 I've faced similar issue recently and realized that this statement is only true when data passed to the template not changed:

// without this option template won't be re-rendered
// upon navigation between different "item" routes
// e.g. when navigating from /item/1 to /item/2