Replies: 45 comments 1 reply
-
|
It would also be helpful here if query strings behave correctly like When the page is known an improvement to the API for pathname/query like |
Beta Was this translation helpful? Give feedback.
-
|
Could there be a manifest only server-side? And when a Link is clicked, the server answers with which bundle/page to render? Probably slower, but we avoid shipping a manifest that may not scale. |
Beta Was this translation helpful? Give feedback.
-
|
@martpie if we explored the manifest route (instead of static analysis), we'd probably make the manifest be downloaded asynchronously and not block application hydration. Routing events before that manifest arrived would revert to server-side rendering IMO, not blocking while fetching the manifest. |
Beta Was this translation helpful? Give feedback.
-
|
(x-posting from slack) Hey all 👋 found this issue while doing some research and wanted to chime in re: routing manifest. I’m sharing this in the hopes it’s relevant to help decide if we want a routing manifest or not: At my company, we’re transitioning to Next.js page by page, which means some Next.js pages need to link to non-Nextjs pages. Whenever a page gets migrated to Next.js, having to go back and wrap any tags that link to that page with a If we could build a component that can decide if it needs to wrap an tag in a Having that routing manifest gives us a way to check if an url matches any routes, sort of like this: We also have a shared design system that needs to be rendered by both Next.js and non-Next.js pages – meaning, if we have e.g. a navigation component, that navigation component can’t render This makes the API for this component more complex, because expecting an array of URLs doesn’t suffice: ^ Again, if there would be a way to check if an url matches any routes, we could keep the API of the Hopefully this is useful input 🙂 |
Beta Was this translation helpful? Give feedback.
-
|
I'd like to add a suggestion which would work without static analysis or a manifest or even a separate It could be as simple as making the <Link href={{ pathname: "/foo/[bar]", query: { bar: 1, baz: 2 } }}><a>...</a></Link>Currently this renders If this was handled out of the box, the rendered link would be Another example to illustrate the issue where it's quite tedious to construct the const LinkCurrentRouteToLocale = ({ locale, ...rest }) => {
const { pathname, query } = useRouter();
// pathname being /[locale]/foo or similar
return <Link href={{ pathname, query: { ...query, locale } }} {...rest} />
} |
Beta Was this translation helpful? Give feedback.
-
|
@herrstucki Not sure if this is exactly this issue, but I'm running into exactly this scenario as well (mixing up regular query params with the "dynamic" ones). This becomes especially cumbersome if you'd have a page with one or more "dynamic route" params which has to support one or more variable query params (think in the direction of utm tags). The query params just seem to "blow up" at some point 😅 |
Beta Was this translation helpful? Give feedback.
-
|
I'd love this! Building a PoC for moving over to Next and use the filesystem based routing we very quickly ran into this problem since we have a lot of dynamic/unknown links from the backend which would be hard to change. We do have a custom, hacky, userland solution based on some Next-internals in the works.. Where are you leaning implementation-wise? As I see it, static analysis seems hard for dynamic links coming from APIs, and while not the case for us, shipping the entire manifest might be problematic for very large applications, even if it's lazy? I haven't thought this through all the way, but could one approach be to track all the Just out of curiosity, is this something you would be interested in external help on or is it mainly something you need to incubate on in relation to the bigger picture? 😄 |
Beta Was this translation helpful? Give feedback.
-
|
Hm, I just realized that approach would only work with ssr, not static so that's a bummer. Pretty complex solution too compared to just shipping a full manifest lazily which should probably be fine for the vast majority of cases, and you can still optimize that away by using |
Beta Was this translation helpful? Give feedback.
-
|
I "solved" this problem in a closed-source project, I think in a somewhat similar way to what Next would do to implement it by itself, just not really as nice and polished. This is what we did: In the app's export const getPagePaths = req => (req
? getSortedRoutes(Object.keys(req.app.locals.nextApp.pagesManifest || {/* in dev */}))
: window.__NEXT_DATA__.props.pagePaths);We then pass them into a React context that is consumed by our custom link component. That component gets an anchor tag as a child, the corresponding import {
getRouteMatcher, getRouteRegex, getSortedRoutes, isDynamicRoute,
} from 'next/dist/next-server/lib/router/utils';to decide whether to
({ children, href, pagePaths }) => {
const matchingPagePath = pagePaths.find(
path => getRouteMatcher(getRouteRegex(path))(href),
);
if (matchingPagePath) {
if (isDynamicRoute(matchingPagePath)) {
// Next Link requires href and as for dynamic routes
return <Link href={matchingPagePath} as={href}>{children}</Link>;
}
return <Link href={href}>{children}</Link>;
}
return children;
}I hope it helps those of you who need a comprehensive solution that does not require doing extra work everywhere |
Beta Was this translation helpful? Give feedback.
-
|
Please be aware that importing from |
Beta Was this translation helpful? Give feedback.
-
|
Yes, thanks @Timer, I mentioned that in the final paragraph 😄 |
Beta Was this translation helpful? Give feedback.
-
|
Would it be possible to ship a solution similar to what @jeysal code is doing? I mean is it OK with library assumptions? |
Beta Was this translation helpful? Give feedback.
-
|
@jayu the solution posted by @jeysal should be avoided in any production facing app. It will break without warning. We want to solve this first-class in Next.js soon, however, there's no ETA. |
Beta Was this translation helpful? Give feedback.
-
|
I just want to relativize that statement a bit and explain why we went for it anyway to help others make the judgement whether it may be worth it for them since this appears to be a quite popular issue. I believe, however, that we need to look at each case individually, and I would classify this particular instance as a relatively clean solution that relies on existing concepts in Next instead of implementing everything in a potentially diverging way and is relatively low-risk. That said, I see two kinds of breaking changes that are likely to occur on the internal APIs we're using (others may be possible but seem unlikely):
As an example of the former, let's imagine Next.js introduces a new way to declare dynamic routes using curly braces in a The latter may mean (re)moving the file being imported from, renaming the exports, or removing the logic entirely in a refactor of the whole routing mechanics. However, I believe that most real world applications have technical debt and architecture choices posing a way greater risk to the continued stability and maintainability of the software than this workaround. Like those, the risk posed by this workaround can also be absorbed to a degree by integration/e2e tests. For us, the risk posed by this workaround appeared to be lower than that posed by the choice to introduce additional work for every Link we add, especially because we have abstractions built on top of Anyway, I think I was in a bit of a philosophical mood and this became more of a general braindump of my thoughts on this topic than directly related to this issue. |
Beta Was this translation helpful? Give feedback.
-
|
We've seen many times before that people tried importing internals, in basically all cases this resulted in:
Hence why we recommend not importing internals like @Timer was saying. |
Beta Was this translation helpful? Give feedback.
-
|
Here's a nice custom <Link page="/users/[id]" params={{ id: user.id }}>
{user.name}
</Link>
<Link page="/blog/[...slug]" params={{ slug: ['coffee', 'frenchpress'] }}>
View Here
</Link>It's a a lot nicer |
Beta Was this translation helpful? Give feedback.
-
|
It'd be great to have some way to use this feature programmatically as well. For example, with a new function like await Router.parse("/news/1234") // { pathname: "/news/[id]", params: { id: "1234" } }A function like this would come in handy in a bunch of situations. In particular, it'd make it a lot easier to move an existing react-router based CRA app into Next.js. Re: |
Beta Was this translation helpful? Give feedback.
-
|
I don't have time to package it up nicely, but I've been using this since dynamic routes came out and it's worked for everything I've needed. The way I create links and do navigation is I created a <Go.Link segments={["news", {id: 1234}]}>
<a>Link text</a>
</Go.Link>
And for pushing routes, I use:
Go.push(["news", {id: 1234}])Basically, a string gets converted directly to part of the path. An object converts its "key" to the Here's the code in TypeScript. May need to be cleaned but in case it's helpful, thought I'd post: import kindOf from "kind-of"
import NextLink from "next/link"
import Router from "next/router"
import React from "react"
import invariant from "tiny-invariant"
type AtLeastOne<T, U = { [K in keyof T]: Pick<T, K> }> = Partial<T> & U[keyof U]
/**
* Note:
* Lots of valuable information in `next/router`
* <https://github.com/zeit/next.js/blob/canary/packages/next/client/router.ts>
*
* properties
* `['pathname', 'route', 'query', 'asPath', 'components']`
* methods
* `['push', 'replace', 'reload', 'back', 'prefetch', 'beforePopState']`
* events
* `['routeChangeStart', 'beforeHistoryChange', 'routeChangeComplete', 'routeChangeError', 'hashChangeStart', 'hashChangeComplete']`
*/
type StringObject = AtLeastOne<{
[key: string]: string
}>
export type Segment = string | StringObject
const Go = {
/**
* Takes an array of Object, parses them and return an `href` and an `as`
*/
parse(segments: Segment[]) {
const hrefSegments: string[] = []
const asSegments: string[] = []
segments.forEach((segment) => {
switch (kindOf(segment)) {
case "string":
hrefSegments.push(`/${segment}`)
asSegments.push(`/${segment}`)
break
case "object":
Object.entries(segment).forEach(
([key, value]: [string, string | undefined]) => {
if (value === undefined) throw new Error(`Should not happen`)
hrefSegments.push(`/[${key}]`)
asSegments.push(`/${value}`)
}
)
break
default:
invariant(
false,
`Segments mut be a string or object but is ${segment}`
)
}
})
let href = hrefSegments.join("")
let as = asSegments.join("")
if (href === "") href = "/"
if (as === "") as = "/"
return { href, as }
},
/**
* Navigate to the given URL like a `Router.push` but parses the segments
*/
push(segments: Segment[], query?: StringObject) {
const { href, as } = Go.parse(segments)
Router.push({ pathname: href, query }, { pathname: as, query })
},
/**
* Navigate to the given URL like a `Router.replace` but parses the segments
*/
replace(segments: Segment[], query?: StringObject) {
const { href, as } = Go.parse(segments)
Router.replace({ pathname: href, query }, { pathname: as, query })
},
/**
* Refresh page in browser.
*
* You can use `Router.reload` to reload on the server
*/
refresh() {
Router.replace(Router.pathname, Router.asPath)
},
/**
* Reload page on server.
*/
reload() {
Router.reload()
},
/**
* Returns a function that can be used as an event handler.
*/
handler(segments: Segment[]) {
return function (e: React.SyntheticEvent) {
e.preventDefault()
e.stopPropagation()
Go.push(segments)
}
},
Link: React.forwardRef(
(
{
segments,
query,
...props
}: { segments: Segment[]; query: any; [key: string]: any },
ref: React.Ref<any>
) => {
const { href, as } = Go.parse(segments)
return (
<NextLink
{...props}
href={{ pathname: href, query }}
as={{ pathname: as, query }}
ref={ref}
/>
)
}
),
}
export { Go } |
Beta Was this translation helpful? Give feedback.
-
|
@mikestopcontinues cool solution! Could it be that you mixed up the regexes for catch-all and regular dynamic path segments?
The first line would map The second line would map Apart from this, the solution does not support Optional catch all routes yet, and it does not take into account that predefined routes take precedence over dynamic routes. Still a good temporary solution until next.js solves this on their end. |
Beta Was this translation helpful? Give feedback.
-
|
Reading the docs of 9.5.3 I assume this is ready and shipped, is it so? Can we use |
Beta Was this translation helpful? Give feedback.
-
|
@fabb Quite right! Thanks! 😄 I updated the code above to fix that bug as well as add support for optional catch-alls. I think the sort lines should ensure precedence. If not, let me know! |
Beta Was this translation helpful? Give feedback.
-
Not in stable version but per this tweet it's available in canary. |
Beta Was this translation helpful? Give feedback.
-
|
@jnv, @nevnein Using it in |
Beta Was this translation helpful? Give feedback.
-
Cool. This RFC issue can be finally closed. Am I right, @Timer? |
Beta Was this translation helpful? Give feedback.
-
|
Wait a second before closing! We don‘t use |
Beta Was this translation helpful? Give feedback.
-
|
Ok I just found this in the docs:
So when we want to navigate to a path |
Beta Was this translation helpful? Give feedback.
-
Correct!
We've only covered the href resolving case (still needs to be announced publicly). That part is fully backwards compatible. Removing the need of |
Beta Was this translation helpful? Give feedback.
-
|
How are dynamic routes such as /category/[...slug] handled when a rewrite is used? Prefetch seems to not work in this case. For example: Page Route: /pages/category/[...slug].tsx Everything works just fine except for prefetching. I have tried: - No Prefetching - Broken Prefetching (seems to be fetching the current page, not the the "as" linkAm I missing something? |
Beta Was this translation helpful? Give feedback.
-
|
It looks like now it's possible to have trailing slashes in urls when pushing. Nice! |
Beta Was this translation helpful? Give feedback.
-
|
Came here from https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/docs/rules/anchor-is-valid.md. Cannot see a way to use nextjs Link with the eslint rule |
Beta Was this translation helpful? Give feedback.
-
|
This RFC is now obsolete as the individual features have been landed in a slightly different way. Resolving the |
Beta Was this translation helpful? Give feedback.
and others
-
Feature request
Is your feature request related to a problem? Please describe.
Next.js should be able to deduce what route should be rendered for a given URL.
Right now, Next.js requires you specify both the original page and route:
However, Next.js should know that
/news/1234matches/news/[id]as it does with the production web server.Describe the solution you'd like
Next.js should accept a Link prop that automatically handles this behavior, potentially like so:
This proposal uses
toinstead of makinghrefhave different behavior so we could potentially make it opt-into not using the child component, instead, rendering<a>automatically like React Router or Reach Router do.Meaning:
Describe alternatives you've considered
Next.js could fall back to matching a dynamic route if not found:
Additional context
Next.js currently does not ship a routing manifest, and that'd be a requirement for this feature to work.
This routing manifest would make the app size heavier than if the user always instructed Next.js with
hrefandas, but some could argue the DX is worth it.TBD
toso we can make this a framework feature, i.e.trailingSlash: true.Beta Was this translation helpful? Give feedback.
All reactions