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

The Pushstate constructor should work independent of can-route #130

Open
justinbmeyer opened this issue Nov 23, 2018 · 0 comments
Open

The Pushstate constructor should work independent of can-route #130

justinbmeyer opened this issue Nov 23, 2018 · 0 comments

Comments

@justinbmeyer
Copy link
Contributor

justinbmeyer commented Nov 23, 2018

I think that Pushstate should work independent of can-route, but it does not. For example, a codepen like this does not work:

https://codepen.io/justinbmeyer/pen/MzGyPb?editors=1011

This is because:

  1. Pushstate calls into can-route to know the root (which is going to be #!)
  2. When a link is clicked on, the routing rules are checked: if (route.rule(url) !== undefined) {

#1 can be solved by letting people pass the "root" into new RoutePushstate().

#2 is tricker. We could allow people to pass in if the "url" should be handled.

new RoutePushstate({
  root: "/",
  usePushstate(){
    return true; // All links will use pushstate
  }
})

I think we should enable this sort of flexibility now. It will make issues like canjs/canjs#4448 easier to tackle.

For backwards compatibility, we will just make the constructor default to:

new RoutePushstate({
  root: function(){
    bindingProxy.call("root")
  },
  usePushstate(){
    return true; // All links will use pushstate
  }
})

The other place it is used is when setting the value of the observable (obs.set("/some/url")).

Currently, set(){} uses route.deparam() to diff which keys are changing and optionally call replaceState instead of pushState.

Do we need this functionality? Should we expect people to call these themselves? We could have additional configuration like:

new RoutePushstate({
  historyMethod(path) {
    return CHECK_PATH ? "pushState" : "replaceState"
  }
})
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

3 participants