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

Incorrect Link href with basepath #76

Closed
giuem opened this issue Aug 16, 2019 · 13 comments · Fixed by #100
Closed

Incorrect Link href with basepath #76

giuem opened this issue Aug 16, 2019 · 13 comments · Fixed by #100

Comments

@giuem
Copy link

giuem commented Aug 16, 2019

Hello, I'm trying to implement basepath into my application. I followed the README's instruction. Everything works well, except <Link /> component.

For example,

<Link href="/path">link</Link>

It's fine that clicking on it will navigate to :basepath/path, but the href prop is still /path.

Now I had to use <Link /> in this way,

<Link href="/path">
  <span><a href={`${basepath}/path`}>link</a></span>
</Link>

Any idea on it?

@molefrog
Copy link
Owner

@giuem Oh yeah, that's a bummer.

You're right since wouter is based on simple useLocation subscription the Link doesn't know anything about the path substitution. It should work well in the app, however I agree, if you need to have semantic links for the SSR that would mess things up. As a temp hack I can offer you to either always use absolute urls everywhere, or write your own Link implementation that takes care of all the transformations needed.

@molefrog
Copy link
Owner

molefrog commented Aug 16, 2019

Another idea is to introduce a new prop on Link component say originalHref (any other suggestions on the name?) that will control the href. So you then could write a helper:

const linkPropsTo = (path) => ({
  href: path,
  originalHref: basePath + path
})

<Link {...linkPropsTo("/users")} />

Just an idea.

@giuem
Copy link
Author

giuem commented Aug 17, 2019

originalHref sounds great for me! Or maybe we can introduce createLink HOC:

function createLink(basepath = '') {
  function Link(props) {
    // ...
    const href = props.href || props.to;
    const originalHref = basepath + href;
    // ...
  }
  return Link;
}

const Link = createLink(basepath);

<Link href="/users" />

@omgovich
Copy link
Contributor

@molefrog Should we close the issue?
As far as I understand the problem was solved here: #94

@molefrog
Copy link
Owner

@omgovich Not yet, there are a few things left:

  • Add types that describe the basepath option
  • Document the feature in README (that's on me)

I also haven't decided yet which name to use. Been thinking that base is smaller and will help us to save a couple of extra bytes, but not sure if it's clear enough:

<Router base="/app">...</Router>

Does this look ok @omgovich @giuem @cbbfcd ?

@omgovich
Copy link
Contributor

@molefrog base is used by Vue Router and NuxtJS: https://router.vuejs.org/ru/api/#mode
So I think this is OK

I can rename basepath to base everywhere and send a PR.

@cbbfcd
Copy link
Contributor

cbbfcd commented Nov 18, 2019

i agree!!!! good idea!! 😄

@giuem
Copy link
Author

giuem commented Nov 18, 2019

Look great for me! Can't wait to use.

@molefrog
Copy link
Owner

@omgovich Ok, thanks. @cbbfcd has just sent a PR with types, we're almost there with merging it. Let's rename the parameter once types land in master.

@molefrog
Copy link
Owner

@omgovich Types have been landed in master. Can you rename the param?

@omgovich
Copy link
Contributor

@molefrog Sure)

@molefrog
Copy link
Owner

Updated the README: https://github.com/molefrog/wouter#i-deploy-my-app-to-the-subfolder-can-i-specify-a-base-path

@molefrog
Copy link
Owner

The basepath support has just landed in the latest 2.4.0 version! Great work, guys, thanks everyone for the help!

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 a pull request may close this issue.

4 participants