veliovgroup / Meteor-flow-router-title

Change document.title on the fly within flow-router
https://atmospherejs.com/ostrio/flow-router-title
BSD 3-Clause "New" or "Revised" License
25 stars 4 forks source link

Route title function isn't reactive #9

Closed nicooprat closed 6 years ago

nicooprat commented 6 years ago

Here's a example of route definition:

FlowRouter.route('/:storyId', {
  name: 'story',
  title({storyId}) {
    // This is only fired once, cursors are empty
    // Subscriptions are done in templates
    // When data is available, this doesn't run again
    const story = Stories.find(storyId).fetch()[0]
    return story && story.title
  },
  action: function(params, queryParams) {
    BlazeLayout.render(...)
  }
})

I also tried const story = Stories.find(storyId).fetch() (without [0]), or even const story = Stories.find(storyId), no luck. The title() function should run again when the cursor isn't empty anymore. Also tried with some Session.get() tests, nothing happens neither.

Thanks.

NB. This follows https://github.com/VeliovGroup/Meteor-flow-router-title/issues/6

dr-dimitru commented 6 years ago

@nicooprat should be fixed now, please update to the latest release and let me know if it works on your end.

nicooprat commented 6 years ago

Many thanks for your reactivity and your work!

I can confirm it's now reactive, but... too much :) I set a debugger in my title() function, and it runs 5 times on any route change: 2 times on the exiting route, 3 times on the entering route. Any idea why it happens?

dr-dimitru commented 6 years ago
  1. Show me your code
  2. If possible debug it with console.trace()
  3. Do you use is in a group, or nested group?
nicooprat commented 6 years ago

My code is like the one on the first comment.

Yes the route is in a group, which has a default title() function by the way (which is not run in this case, as expected).

Here's the trace:

VM33237:1 console.trace
(anonymous) @ VM33237:1
title @ routing.js:142
Tracker.Computation._compute @ tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:339
Tracker.Computation @ tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:229
Tracker.autorun @ tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:613
FlowRouterTitle._reactivate @ flow-router-title.js:23
FlowRouterTitle.titleHandler @ flow-router-title.js:92
Triggers.runTriggers @ triggers.js:102
(anonymous) @ router.js:166
waitOn @ route.js:287
route._actionHandle @ router.js:165
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3159
nextEnter @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2991
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3160
nextEnter @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2991
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3160
nextEnter @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2991
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3160
nextEnter @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2991
nextExit @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2979
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3160
nextExit @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2980
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3160
nextExit @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2980
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3160
nextExit @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2980
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3160
nextExit @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2980
Triggers.runTriggers @ triggers.js:112
route._exitHandle @ router.js:178
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3159
nextExit @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2980
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3160
nextExit @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2980
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3160
nextExit @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2980
(anonymous) @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:3160
nextExit @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2980
page.dispatch @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2995
page.replace @ ostrio_flow-router-extra.js?hash=7b042184508396ac568d63de9dee5730d1c9975f:2960
self._page.(anonymous function) @ router.js:436
go @ router.js:260
(anonymous) @ chapterView.js:100
EVp.withValue @ meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:1090
withReplaceState @ router.js:405
snap @ chapterView.js:100
(anonymous) @ lib.coffee:289
(anonymous) @ lib.coffee:272
Blaze._withCurrentView @ blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:2271
(anonymous) @ lib.coffee:271
Template._withTemplateInstanceFunc @ blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3744
wrapViewAndTemplate @ lib.coffee:265
eventMap.(anonymous function) @ lib.coffee:288
(anonymous) @ blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:2617
Blaze._withCurrentView @ blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:2271
(anonymous) @ blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:2616
(anonymous) @ blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:863
(anonymous) @ blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:901
wrapper @ blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:164
_super.bugsnag @ modules.js?hash=32f4a4c8ab3a1491c376817b1f96ec7819349195:82786
(anonymous) @ layout.js:127
_super.bugsnag @ modules.js?hash=32f4a4c8ab3a1491c376817b1f96ec7819349195:82786
(anonymous) @ modules.js?hash=32f4a4c8ab3a1491c376817b1f96ec7819349195:83759
setTimeout (async)
(anonymous) @ modules.js?hash=32f4a4c8ab3a1491c376817b1f96ec7819349195:83758
snap @ layout.js:127
vars.end @ Draggable.js:859
_parseEnd @ ThrowPropsPlugin.js:53
p._onInitTween @ ThrowPropsPlugin.js:351
TweenLite.js.p._initProps @ TweenLite.js:1375
TweenLite.js.p._init @ TweenLite.js:1334
TweenLite.js.p.render @ TweenLite.js:1503
ThrowPropsPlugin.to @ ThrowPropsPlugin.js:297
animate @ Draggable.js:1453
onRelease @ Draggable.js:1980
_super.bugsnag @ modules.js?hash=32f4a4c8ab3a1491c376817b1f96ec7819349195:82786

Hope it helps.

dr-dimitru commented 6 years ago

What browser that log from? Can't differ call from call.

dr-dimitru commented 6 years ago

Yes the route is in a group, which has a default title() function by the way (which is not run in this case, as expected).

Please explain

nicooprat commented 6 years ago

It's from Chrome 61. How could I get a better trace?

I knew I wasn't clear :) Here's a sample of my code:

const allRoutes = FlowRouter.group({
  name: 'allRoutes',
  title() {
    // This is not run, as expected
    return 'Default title'
  }
}

allRoutes.route('/:storyId', {
  name: 'story',
  title({storyId}) {
    // Runs a lot of times
    const story = Stories.find(storyId).fetch()[0]
    return story && story.title
  }
})
dr-dimitru commented 6 years ago

Are you meant to use titlePrefix instead of title in a group? Because title in a group will be used as default title for routes where is no title.

nicooprat commented 6 years ago

No it's meant to be like this, I have some routes where I don't set the title, so the default one is the group is set. I just meant to say that I'm actually using groups, but in my understanding it's not the cause of the issue, as it behaves as expected.

By the way, a titleSuffix option would be great too, as I always need to append the name of the app to titles. I think it's a pretty common use case.

dr-dimitru commented 6 years ago

By the way, a titleSuffix option would be great too, as I always need to append the name of the app to titles. I think it's a pretty common use case.

Then why don't you use titlePrefix 🤣 ?

const account = FlowRouter.group({
  prefix: '/account',
  title: "Account",
  titlePrefix: 'Account > '
});

How could I get a better trace?

Mb using warn expanding trace and taking screenshot.

No it's meant to be like this, I have some routes where I don't set the title, so the default one is the group is set. I just meant to say that I'm actually using groups, but in my understanding it's not the cause of the issue, as it behaves as expected.

If it's used in groups it will re-computed for each title in each group of nested groups and its routes

nicooprat commented 6 years ago

I was actually speaking of appending text, not prepending. Like « prefix > page - app title ». Unless I misunderstood its use?

I’ll get that trace as soon as possible, afk for now, too bad we are not in the same timezone :D

I’ll try without this group default title then.

dr-dimitru commented 6 years ago

Well:

route's title: 'Title'
group's titlePrefix: 'Prefix > '

// will result in
Prefix > Title
dr-dimitru commented 6 years ago

too bad we are not in the same timezone :D

1:30 am at my zone :D

dr-dimitru commented 6 years ago

I was actually speaking of appending text, not prepending. Like « prefix > page - app title ». Unless I misunderstood its use?

Got you now... yeah, not possible now. Not sure if will be ever possible, I can detect first definition to add prepending text, but won't ever know if that one is the last one, thanks to the ability to nest groups

nicooprat commented 6 years ago

Understood. Not sure if it’s really importan thought, that could even be something static, as I can’t really think of another use than for the global app title. But anyway, that’s not really important.

dr-dimitru commented 6 years ago

But anyway, that’s not really important.

And it will be difficult to keep title in the recommended range of 70 chars