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

router.resolve does not use current route by default #2385

Comments

@decademoon
Copy link
Contributor

decademoon commented Sep 14, 2018

Version

3.0.1

Reproduction link

http://jsfiddle.net/df4Lnuw6/698/

Steps to reproduce

  • Click the link.
  • Click the button.

What is expected?

No warning to be shown.

What is actually happening?

A warning is shown saying that the id param was not provided. The docs mention that the current route will be used if omitted.

@clintonyeb
Copy link

clintonyeb commented Sep 16, 2018

In current implementation, this is how the resolve method is implemented https://github.com/vuejs/vue-router/blob/dev/src/index.js#L177 :

resolve (
    to: RawLocation,
    current?: Route,
    append?: boolean
  ): {
    location: Location,
    route: Route,
    href: string,
    // for backwards compat
    normalizedTo: Location,
    resolved: Route
  } {
    const location = normalizeLocation(
      to,
      current || this.history.current,
      append,
      this
    )
    const route = this.match(location, current)
    const fullPath = route.redirectedFrom || route.fullPath
    const base = this.history.base
    const href = createHref(base, fullPath, this.mode)
    return {
      location,
      route,
      href,
      // for backwards compat
      normalizedTo: location,
      resolved: route
    }
}

I am thinking the issue is coming from from line 195:
const route = this.match(location, current)

I am proposing this new implementation:

resolve (
    to: RawLocation,
    current?: Route,
    append?: boolean
  ): {
    location: Location,
    route: Route,
    href: string,
    // for backwards compat
    normalizedTo: Location,
    resolved: Route
  } {
    current = current || this.history.current

    const location = normalizeLocation(
      to,
      current,
      append,
      this
    )
    const route = this.match(location, current)
    const fullPath = route.redirectedFrom || route.fullPath
    const base = this.history.base
    const href = createHref(base, fullPath, this.mode)
    return {
      location,
      route,
      href,
      // for backwards compat
      normalizedTo: location,
      resolved: route
    }
}

Will love to receive feedbacks..

@posva
Copy link
Member

posva commented Sep 16, 2018

yeah, I remember I did this but somehow never committed the code nor created a PR...

@clintonyeb
Copy link

Thanks :)

posva added a commit that referenced this issue Mar 23, 2019
posva added a commit that referenced this issue Mar 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment