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

teamId/orgId missing from Segment page events #1086

Closed
jordanh opened this issue Jun 16, 2017 · 4 comments
Closed

teamId/orgId missing from Segment page events #1086

jordanh opened this issue Jun 16, 2017 · 4 comments
Assignees
Labels

Comments

@jordanh
Copy link
Contributor

jordanh commented Jun 16, 2017

Issue - Bug

Investigate why "Loaded a Page" events metadata seem to be missing for some paths (such as /meeting). Likely related to recent routing changes.

@jordanh jordanh added the bug label Jun 16, 2017
@jordanh jordanh added this to the Epic 5: Card Editor milestone Jun 16, 2017
@jordanh jordanh self-assigned this Jun 16, 2017
@jordanh
Copy link
Contributor Author

jordanh commented Jun 16, 2017

Here's the problem: https://github.com/ParabolInc/action/blob/030809d17a8b614e9c36993ca13f3b6e05651ed2/src/universal/containers/Action/ActionContainer.js#L51

Before the upgrade to react-router-v4 the url params used to be available to the segment page event. Now, that parameter is no longer being passed.

@mattkrick is there a way that ActionContainer can still have access to the object of url parameters for a route? (e.g. if /meeting/:teamId -> { teamId: "abc123" })

@mattkrick
Copy link
Member

Absolutely, it should be on the location prop. I'll make a little hotfix shortly

@mattkrick
Copy link
Member

ah, now i see what you mean: https://github.com/ParabolInc/action/pull/967/files#diff-8593bb6594d23b08eb1eb9045737df40

we aren't given params by the parent component because v4 is completely modular, so it relies on the child to tell it what those params mean & that child doesn't load before the parent.

so, couple solutions:

  • create an object that represents the sitemap. then, given a path, we can extract the params. side benefit is that folks new to the project can get a rough idea of the layout.
  • make params a reactive variable (ie stick it in a redux store or similar).
  • wrap child routes in a HOC that grabs the previous path from context, matches it to the new path + params & sends it off. this is a more modular approach, but more repetitive since each route would need this logic.

@mattkrick
Copy link
Member

i dont like the 1st option because it creates a 2nd source of truth.
2 & 3 don't work out of the box because the modular nature of v4 means that a route gets matched, then rematched, then maybe rematched again. there is no way to tell if the match occurs at the bottom of the tree (1-way data flow & all that jazz, parents don't know about children).
So, what we can do is create a boolean (say bottom) that we pass in to AsyncRoute. Then, that'll get passed on to Bundle. Then, in the bundle, we can call the update function. That means Bundle will need dispatch, which isn't ideal, but not terrible. That way, going to /me doesn't send 3 calls to the endpoint.

@ghost ghost assigned mattkrick Jun 19, 2017
jordanh added a commit that referenced this issue Jun 29, 2017
@ghost ghost removed the pr review label Jun 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants