Skip to content

Add access to history state #2397

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

Closed
wants to merge 3 commits into from
Closed

Conversation

jnields
Copy link

@jnields jnields commented Sep 18, 2018

This allows developers access to history.state, which you can use for associating extra data to the URL without updating the visible URL, in history mode. It does not work for hash mode.

@posva
Copy link
Member

posva commented Sep 19, 2018

Why are there 31 files modified? 😨
I appreciate your efforts on this but we don't want to add invisible state like this to html history because we have seen it becomes a pattern difficult to maintain in apps. We may add some way yo add data to the object passed to popstate but directly having a state in the route is exactly the pattern that has been used in other routers and led to any maintenance problems

@jnields
Copy link
Author

jnields commented Sep 19, 2018

@posva - eslint wasn't running on test files so I just added it for that and did a --fix. It's a separate commit, so feel free to drop it if you like!

Is this really such a maintainability concern for you? Whether or not using history.state makes an app maintainable seems like the user's concern!

And as it stands right now I am using a home-brewed router for access to history.state to keep track of analytics data.

It's an appropriate place for this, and there are several other completely valid use cases for it as well (which is why it was added to the native API!)

I don't think it makes maintainability of this project much more difficult - so why prevent users from accessing it? It's preventing adoption in at least one case ...

@jnields
Copy link
Author

jnields commented Sep 19, 2018

There is some work to finalize this - it could use some more tests and thinking through edge cases - but if you give me the green light I'll go ahead and add them and get this ready. The one commit I added is the bulk of it, and as you can see it wouldn't be that much

@emanuelmutschlechner
Copy link
Contributor

@jnields I think the router shouldn't be responsible for managing state which is not required for routing.

It's possible to add extra state without changes in the router:

const Foo = { template: '<div>foo</div>' }
const Bar = { template: '<div>bar</div>' }

const routes = [
    { 
        path: '/foo',
        component: Foo,
        state: { a: 1 }
    },
    {
        path: '/bar',
        component: Bar,
        state: { b: 2 }
    },
]

const router = new VueRouter({
    routes
})

function urlToState(url, originalState) {
    const route = undefined; // TODO match url with routes.path, 
                                            // e.g. using regex or router.resolve
    const somethingElse = false;
    if (route) {
        return route.state || {}
    } else if (somethingElse) {
        return { random: 123 }
    }
    return { default: 999 }
}

function modifyState(data, url) {
    const state = urlToState(url, data)
    return {
        ...data,
        ...state,
    };
}

const { pushState, replaceState } = window.history;
window.history.pushState = (data, title, url) => pushState.call(window.history, modifyState(data, url), url);
window.history.replaceState = (data, title, url) => replaceState.call(window.history, modifyState(data, url), url);

@rzane
Copy link

rzane commented Jan 31, 2019

Is there any possibility that Vue Router will support this behavior? I assumed this was provided by Vue Router (because it's provided by every other client-side routing package that I've used), and I was really disappointed to find that it's not supported.

Because Vue Router controls all interaction with the history API, I'm concerned that if I use history.pushState directly, I'd break compatibility Vue Router. So now, it feels like Vue Router has actually taken away my access to stateful routing.

I also don't think that monkey-patching history.pushState is a viable solution.

@posva
Copy link
Member

posva commented Aug 23, 2019

Things have got a little bit outdated but I want to get a second look at this and want to know about the different use cases that require access to the state like applying transition on using the data on scrollBehavior. The discussion can be found at #2243

I need to make sure people don't use this to put request data that should go somewhere else and maybe even come up with something new for that

@posva posva closed this Aug 23, 2019
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.

4 participants