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

Active continues to be true when a route no longer matches the path #38

Closed
cdata opened this issue Mar 8, 2016 · 5 comments
Closed

Comments

@cdata
Copy link
Contributor

cdata commented Mar 8, 2016

Once a route matches, the global path may change such that the route's route property does not change. This results in active remaining true for routes that do not match the global path.

@cdata
Copy link
Contributor Author

cdata commented Mar 8, 2016

The problem appears to be related to tail, which is not nulled-out or emptied out when it no longer corresponds to the current global path.

@cdata
Copy link
Contributor Author

cdata commented Mar 8, 2016

Ah yes, this: https://github.com/PolymerElements/carbon-route/blob/master/carbon-route.html#L176-L177

To clear or not to clear: that is the question.

@cdata
Copy link
Contributor Author

cdata commented Mar 8, 2016

P.S. I believe that this is the cause of the state leak that I referred to here. I suspect it goes like this: the objects are not cleared, so the queryParams changes - due to late-firing events from the YouTube element - bounce back up the tree via tail and cause the query string to go from empty to populated unexpectedly.

@rictic
Copy link
Collaborator

rictic commented Mar 8, 2016

Interesting. I think this makes for a pretty good argument that the default behavior should be nulling out properties when a route no longer matches, and the case where you want to keep some information alive in an element should be the special case.

Making active work wouldn't be too hard I suspect, the tricky bit would be blocking queryParams in inactive routes while maintaining their state. I think you'd have to copy the queryParams object, which seems complex and inefficient.

@sjmiles

@cdata
Copy link
Contributor Author

cdata commented Mar 8, 2016

the default behavior should be nulling out properties when a route no longer matches

I think this makes a lot of sense for the first release. Maintaining state in inactive trees has a lot of subtle side-effects, especially when those trees may affect that state asynchronously.

I think you'd have to copy the queryParams object, which seems complex and inefficient

I agree, also you need to manually propagate changes (when appropriate) if you take this approach.

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