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

Route title function isn't reactive #9

Closed
nicooprat opened this issue Oct 14, 2017 · 16 comments
Closed

Route title function isn't reactive #9

nicooprat opened this issue Oct 14, 2017 · 16 comments

Comments

@nicooprat
Copy link

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 #6

@dr-dimitru
Copy link
Member

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

@nicooprat
Copy link
Author

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
Copy link
Member

  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
Copy link
Author

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
Copy link
Member

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

@dr-dimitru
Copy link
Member

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
Copy link
Author

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
Copy link
Member

dr-dimitru commented Oct 18, 2017

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
Copy link
Author

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
Copy link
Member

dr-dimitru commented Oct 18, 2017

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
Copy link
Author

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
Copy link
Member

Well:

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

// will result in
Prefix > Title

@dr-dimitru
Copy link
Member

too bad we are not in the same timezone :D

1:30 am at my zone :D

@dr-dimitru
Copy link
Member

dr-dimitru commented Oct 18, 2017

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
Copy link
Author

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
Copy link
Member

But anyway, that’s not really important.

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

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

2 participants