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

[v6] Fix Link and navigate for absolute paths with basename #7462

Merged
merged 1 commit into from
Aug 11, 2021

Conversation

bogdansoare
Copy link
Contributor

Fixes #7216
Adds test for correct link behaviour under basename

@webKity

This comment has been minimized.

@ItsWendell

This comment has been minimized.

@bogdansoare
Copy link
Contributor Author

@webKity I don't think we should support basename with router nesting for now, @timdorr what do you think ?
@ItsWendell unfortunately I don't know exactly since I'm not a core contributor, I would like this being merged as soon as possible

@Frozen-byte
Copy link

Frozen-byte commented Aug 12, 2020

I'd like to see this merged quickly, too.
The bug #7216 prevents me from a clean v6 upgrade.

@benjasHu

This comment has been minimized.

@ivan-dalmet

This comment has been minimized.

ivan-dalmet added a commit to ivan-dalmet/react-router that referenced this pull request Oct 16, 2020
@caseyjhol

This comment has been minimized.

@beautyfree

This comment has been minimized.

@stale
Copy link

stale bot commented Dec 27, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
You can add the fresh label to prevent me from taking any action.

@stale stale bot added the stale label Dec 27, 2020
@Frozen-byte
Copy link

pushup, still ongoing problem.

@stale stale bot removed the stale label Dec 28, 2020
@keremciu
Copy link

would be great to see this one merged! :)

@mmartinsky
Copy link

This would be really helpful for me as well.

@taranvohra
Copy link

Would really appreciate if this one is merged

@ivanjonas
Copy link
Contributor

To me, this is a critical fix for an unusable version of react-router. Many thanks to the team for reviewing and merging this 🙏

@XiaofengXie16
Copy link

Any updates on this? This is also critical for my use case.

@chaance chaance changed the title Fix Link and navigate for absolute paths with basename [v6] Fix Link and navigate for absolute paths with basename Jul 28, 2021
@phoenixcoded20
Copy link

Any ETA on this, when it's going to be merged? It's really helpful if we have some date.

@jgoux
Copy link

jgoux commented Aug 5, 2021

I have a different use case and the current behaviour actually suits my need. 😅

I'm using a Slot/Fill library to send components into my Layout. Because of that, if I want to use relative Link inside the implementation of the components sent into the Layout I need to "recreate" a routing context. Right now it's a bit hacky but it does the job :

const { pathname } = useResolvedPath("");

  return (
    <>
      {props.tabs && (
        <Fill name="Layout.Tabs">
          <Routes basename={pathname}>
            <Route path="*" element={props.tabs} />
          </Routes>
        </Fill>
      )}
   </>
 )  

So my components sent to the Layout are still context aware from where they were sent. ⭐

Now with this change, I won't be able to use absolute path as I would do in a normal situation, because of my usage of basename here.

I now this PR is right and that it is the intended behaviour, but I think my use case is a valid one as well and I'd love having some alternative once this is fixed. 👍
Maybe just a way to opt-out from the basename when using Link would be enough to cover my need.

@chaance chaance added the bug label Aug 11, 2021
@chaance chaance merged commit ea4fc5e into remix-run:dev Aug 11, 2021
@trangnt1011
Copy link

Hi, can I know when you guys will release this bug fixing? This would be really helpful for me as well.

@timdorr
Copy link
Member

timdorr commented Aug 11, 2021

@trangnt1011 There is no scheduled release plan. Just wait a bit and these things will be released. We're still in beta on v6, so you should switch back to the stable release if you're not comfortable with waiting.

@harshadray
Copy link

Is this released?

@timdorr
Copy link
Member

timdorr commented Oct 12, 2021

@harshadray Yes, this was in the v6 beta.1 release: https://github.com/remix-run/react-router/releases/tag/v6.0.0-beta.1

@harshadray
Copy link

Hey @timdorr Thanks for a prompt response. I checked this but I am using useRoutes as suggested here: https://github.com/remix-run/react-router/blob/dev/docs/advanced-guides/migrating-5-to-6.md?ref=morioh.com&utm_source=morioh.com#move-basename-from-router-to-routes

Is there any way around to apply basename there? Previously we were able to apply to basename prop there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet