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

Pages via URL #2283

Merged
merged 7 commits into from
Mar 14, 2017
Merged

Pages via URL #2283

merged 7 commits into from
Mar 14, 2017

Conversation

marla-singer
Copy link
Contributor

@marla-singer marla-singer commented Mar 14, 2017

Closes #2222
Closes #2281

  • Replace Meteor.user()._id; to Meteor.userId(). It did the stack overflow
  • Add FlowRouter.wait(). It's waiting for the subscription ready. I found this way in this article. Look at sentence UPDATE May 26th 2016

@marla-singer
Copy link
Contributor Author

@apinf/developers Ready for review

// There are specific cases that this reruns

// Make sure the roles subscription is ready & FlowRouter hasn't initialized already
if (Roles.subscription.ready() && !FlowRouter._initialized) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a more FlowRouter specific way to wait for the Roles subscription? E.g. an onInitialize or beforeInitialize hook?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FlowRouter limits reactive data sources to a single run; when it is first called.

It might not be the case that we can check for the Roles subscription in a different way. I just want to double check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brylie If you can find another way - you're welcome. I added the link to article in description box that I found

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After scanning the FlowRouter docs, I can't come up with a different proposal for this code.

One thing that would be helpful, since this is a very unique situation, is to add a multi-line comment describing the need for the Tracker.autorun function. What problem is it solving?

// There are specific cases that this reruns

// Make sure the roles subscription is ready & FlowRouter hasn't initialized already
if (Roles.subscription.ready() && !FlowRouter._initialized) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After scanning the FlowRouter docs, I can't come up with a different proposal for this code.

One thing that would be helpful, since this is a very unique situation, is to add a multi-line comment describing the need for the Tracker.autorun function. What problem is it solving?

@brylie brylie self-assigned this Mar 14, 2017
@marla-singer
Copy link
Contributor Author

@brylie Add the comment. Is it full enough?

@brylie brylie merged commit d44d1c7 into develop Mar 14, 2017
@brylie brylie deleted the bugfix/directly-url branch March 14, 2017 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants