-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Fix <Link to="string"> #5489
Fix <Link to="string"> #5489
Conversation
Why not keep things inside the render? Does createLocation have some sort of side effect? |
We could have the funcionality in |
New implementation will parse the string to create an actual location object. This also means that a to string with no pathname will resolve using the current location. The "href" value is also now stored in state, only recalculating it when the "to" prop changes.
Would love to see this land! 😊 |
I'd definitely like to keep the |
I reverted to computing the |
I think this is good too. In she goes... |
YAS 🎉 Was this released? |
179: Update dependency react-router-dom to v4.3.0 r=magopian a=renovate[bot] This Pull Request updates dependency [react-router-dom](https://github.com/ReactTraining/react-router) from `v4.2.2` to `v4.3.0` <details> <summary>Release Notes</summary> ### [`v4.3.0`](https://github.com/ReactTraining/react-router/blob/master/CHANGELOG.md#v430httpsgithubcomReactTrainingreact-routercomparev420v430) [Compare Source](remix-run/react-router@v4.3.0-rc.3...a27bc56) > Jun 6, 2018 * Use the `pretty` option in generatePath ([#​6172] by @​sibelius) * aria-current has incorrect value "true" ([#​6118] by @​brandonrninefive) * Redirect with parameters ([#​5209] by @​dlindenkreuz) * Fix with missing pathname: `<Link to="?foo=bar">` ([#​5489] by @​pshrmn) * Escape NavLink path to allow special characters in path. ([#​5596] by @​esiegel) * Expose `generatePath` ([#​5661] by @​rybon) * Use named import of history module. ([#​5589] by @​RoboBurned) * Hoist dependencies for smaller UMD builds ([#​5720] by @​pshrmn) * Remove aria-current from navLink when inactive ([#​5508] by @​AlmeroSteyn) * Add invariant for missing "to" property on `<Link>` ([#​5792] by @​selbekk) * Use Prettier on the code ([e6f9017] by @​mjackson) * Fix pathless route's match when parent is null ([#​5964] by @​pshrmn) * Use history.createLocation in `<StaticRouter>` ([#​5722] by @​pshrmn) [#​6172]: `https://github.com/ReactTraining/react-router/pull/6172` [#​6118]: `https://github.com/ReactTraining/react-router/pull/6118` [#​5209]: `https://github.com/ReactTraining/react-router/pull/5209` [#​5489]: `https://github.com/ReactTraining/react-router/pull/5489` [#​5596]: `https://github.com/ReactTraining/react-router/pull/5596` [#​5661]: `https://github.com/ReactTraining/react-router/pull/5661` [#​5589]: `https://github.com/ReactTraining/react-router/pull/5589` [#​5720]: `https://github.com/ReactTraining/react-router/pull/5720` [#​5508]: `https://github.com/ReactTraining/react-router/pull/5508` [#​5792]: `https://github.com/ReactTraining/react-router/pull/5792` [e6f9017]: remix-run/react-router@e6f9017 [#​5964]: `https://github.com/ReactTraining/react-router/pull/5964` [#​5722]: `https://github.com/ReactTraining/react-router/pull/5722` --- ### [`v4.3.0-rc.3`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.3) [Compare Source](remix-run/react-router@v4.3.0-rc.2...v4.3.0-rc.3) - Fix broken UMD builds. - Add sideEffects: false for webpack tree shaking (#​6082 by @​taylorc93) --- ### [`v4.3.0-rc.2`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.2) [Compare Source](remix-run/react-router@v4.3.0-rc.1...v4.3.0-rc.2) - Bump `hoist-non-react-statics` for React 16.3. - Missing `generatePath` in `react-router-dom` package. --- ### [`v4.3.0-rc.1`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.1) [Compare Source](remix-run/react-router@v4.2.2...v4.3.0-rc.1) > Mar 26, 2018 - Redirect with parameters ([#​5209] by @​dlindenkreuz) - Fix with missing pathname: `<Link to="?foo=bar">` ([#​5489] by @​pshrmn) - Escape NavLink path to allow special characters in path. ([#​5596] by @​esiegel) - Expose `generatePath` ([#​5661] by @​rybon) - Use named import of history module. ([#​5589] by @​RoboBurned) - Hoist dependencies for smaller UMD builds ([#​5720] by @​pshrmn) - Remove aria-current from navLink when inactive ([#​5508] by @​AlmeroSteyn) - Add invariant for missing "to" property on `<Link>` ([#​5792] by @​selbekk) - Use Prettier on the code ([e6f9017] by @​mjackson) - Fix pathless route's match when parent is null ([#​5964] by @​pshrmn) - Use history.createLocation in `<StaticRouter>` ([#​5722] by @​pshrmn) [#​5209]: `https://github.com/ReactTraining/react-router/pull/5209` [#​5489]: `https://github.com/ReactTraining/react-router/pull/5489` [#​5596]: `https://github.com/ReactTraining/react-router/pull/5596` [#​5661]: `https://github.com/ReactTraining/react-router/pull/5661` [#​5589]: `https://github.com/ReactTraining/react-router/pull/5589` [#​5720]: `https://github.com/ReactTraining/react-router/pull/5720` [#​5508]: `https://github.com/ReactTraining/react-router/pull/5508` [#​5792]: `https://github.com/ReactTraining/react-router/pull/5792` [e6f9017]: remix-run/react-router@e6f9017 [#​5964]: `https://github.com/ReactTraining/react-router/pull/5964` [#​5722]: `https://github.com/ReactTraining/react-router/pull/5722` --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com). Co-authored-by: Renovate Bot <[email protected]>
178: Update dependency react-router to v4.3.0 r=magopian a=renovate[bot] This Pull Request updates dependency [react-router](https://github.com/ReactTraining/react-router) from `v4.2.0` to `v4.3.0` <details> <summary>Release Notes</summary> ### [`v4.3.0`](https://github.com/ReactTraining/react-router/blob/master/CHANGELOG.md#v430httpsgithubcomReactTrainingreact-routercomparev420v430) [Compare Source](remix-run/react-router@v4.3.0-rc.3...v4.3.0) > Jun 6, 2018 * Use the `pretty` option in generatePath ([#​6172] by @​sibelius) * aria-current has incorrect value "true" ([#​6118] by @​brandonrninefive) * Redirect with parameters ([#​5209] by @​dlindenkreuz) * Fix with missing pathname: `<Link to="?foo=bar">` ([#​5489] by @​pshrmn) * Escape NavLink path to allow special characters in path. ([#​5596] by @​esiegel) * Expose `generatePath` ([#​5661] by @​rybon) * Use named import of history module. ([#​5589] by @​RoboBurned) * Hoist dependencies for smaller UMD builds ([#​5720] by @​pshrmn) * Remove aria-current from navLink when inactive ([#​5508] by @​AlmeroSteyn) * Add invariant for missing "to" property on `<Link>` ([#​5792] by @​selbekk) * Use Prettier on the code ([e6f9017] by @​mjackson) * Fix pathless route's match when parent is null ([#​5964] by @​pshrmn) * Use history.createLocation in `<StaticRouter>` ([#​5722] by @​pshrmn) [#​6172]: `https://github.com/ReactTraining/react-router/pull/6172` [#​6118]: `https://github.com/ReactTraining/react-router/pull/6118` [#​5209]: `https://github.com/ReactTraining/react-router/pull/5209` [#​5489]: `https://github.com/ReactTraining/react-router/pull/5489` [#​5596]: `https://github.com/ReactTraining/react-router/pull/5596` [#​5661]: `https://github.com/ReactTraining/react-router/pull/5661` [#​5589]: `https://github.com/ReactTraining/react-router/pull/5589` [#​5720]: `https://github.com/ReactTraining/react-router/pull/5720` [#​5508]: `https://github.com/ReactTraining/react-router/pull/5508` [#​5792]: `https://github.com/ReactTraining/react-router/pull/5792` [e6f9017]: remix-run/react-router@e6f9017 [#​5964]: `https://github.com/ReactTraining/react-router/pull/5964` [#​5722]: `https://github.com/ReactTraining/react-router/pull/5722` --- ### [`v4.3.0-rc.3`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.3) [Compare Source](remix-run/react-router@v4.3.0-rc.2...v4.3.0-rc.3) - Fix broken UMD builds. - Add sideEffects: false for webpack tree shaking (#​6082 by @​taylorc93) --- ### [`v4.3.0-rc.2`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.2) [Compare Source](remix-run/react-router@v4.3.0-rc.1...v4.3.0-rc.2) - Bump `hoist-non-react-statics` for React 16.3. - Missing `generatePath` in `react-router-dom` package. --- ### [`v4.3.0-rc.1`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.1) [Compare Source](remix-run/react-router@v4.2.0...v4.3.0-rc.1) > Mar 26, 2018 - Redirect with parameters ([#​5209] by @​dlindenkreuz) - Fix with missing pathname: `<Link to="?foo=bar">` ([#​5489] by @​pshrmn) - Escape NavLink path to allow special characters in path. ([#​5596] by @​esiegel) - Expose `generatePath` ([#​5661] by @​rybon) - Use named import of history module. ([#​5589] by @​RoboBurned) - Hoist dependencies for smaller UMD builds ([#​5720] by @​pshrmn) - Remove aria-current from navLink when inactive ([#​5508] by @​AlmeroSteyn) - Add invariant for missing "to" property on `<Link>` ([#​5792] by @​selbekk) - Use Prettier on the code ([e6f9017] by @​mjackson) - Fix pathless route's match when parent is null ([#​5964] by @​pshrmn) - Use history.createLocation in `<StaticRouter>` ([#​5722] by @​pshrmn) [#​5209]: `https://github.com/ReactTraining/react-router/pull/5209` [#​5489]: `https://github.com/ReactTraining/react-router/pull/5489` [#​5596]: `https://github.com/ReactTraining/react-router/pull/5596` [#​5661]: `https://github.com/ReactTraining/react-router/pull/5661` [#​5589]: `https://github.com/ReactTraining/react-router/pull/5589` [#​5720]: `https://github.com/ReactTraining/react-router/pull/5720` [#​5508]: `https://github.com/ReactTraining/react-router/pull/5508` [#​5792]: `https://github.com/ReactTraining/react-router/pull/5792` [e6f9017]: remix-run/react-router@e6f9017 [#​5964]: `https://github.com/ReactTraining/react-router/pull/5964` [#​5722]: `https://github.com/ReactTraining/react-router/pull/5722` --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com). Co-authored-by: Renovate Bot <[email protected]>
This change causing the href calculations to be |
@manju16832003 Can you provide sample code (or ideally a sandbox) to reproduce the error? |
|
@manju16832003 You should only be including the <Link to="/ContactList.html">Contact List</Link> |
@pshrmn Interesting. We ran into the same thing as @manju16832003 too. I guess I never picked up in the documentation that Link couldn't handle external links (which up until this point, it could). Maybe this could be made more explicit in the documentation? While obviously Link is intended for internal navigation, I'm sure there are other users who may be running into similar things too -- I could easily see a use case impacted by this in a breaking way, like say a re-usable component with a Link inside that sometimes may be provided an internal URL, and sometimes an external URL. Easy enough to fix if one is aware, but something I could see coming up as a surprise for some folks. Either way, thanks for the updates! |
Suppose the location path is |
@Skandar-Ln you are using a relative |
I just wanted to point out that like @manjufy, we were using the component more or less universally and this is a breaking change. I also agree with @Android3000: the release notes for 4.3.0 should definitely be clear about this change along with a note the general docs. |
New implementation will parse the string to create an actual location object. This also means that a to string with no pathname will resolve using the current location.
The
href
value is also now stored in state, only recalculating it when theto
prop changes.Edit: If anyone is looking at this, but hasn't read through #5488, the reason for this PR is that when a user renders a
<Link>
whose stringto
does not include a pathname, the rendered anchor'shref
does not include a pathname (<Link to='?test=ing'>Test</Link>
-><a href='?test=ing'>Test</a>
). This results in conflicts with the output of the<StaticRouter>
since that explicitly makeshref
s absolute paths by enforcing that the string begins with a forward slash. Since React Router works with location's whosepathnames
are absolute, I felt it was more appropriate for the<Link>
to produce absolute URLs than it was for the<StaticRouter>
to have a special use case.