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

with-data-prefetch example doesn't work with Next v8 #6376

Closed
adamsoffer opened this issue Feb 20, 2019 · 12 comments
Closed

with-data-prefetch example doesn't work with Next v8 #6376

adamsoffer opened this issue Feb 20, 2019 · 12 comments
Labels
good first issue Easy to fix issues, good for newcomers

Comments

@adamsoffer
Copy link
Contributor

Examples bug report

Example name

with-data-prefetch

Describe the bug

The example fails to compile with this error:

This dependency was not found:

* next/dist/lib/utils in ./components/link.js

To install it, you can run: npm install --save next/dist/lib/utils

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Install the example npx create-next-app --example with-data-prefetch with-data-prefetch-app
  2. Install the deps npm install
  3. Run the example npm run dev
  4. See error

Expected behavior

The example should run and data prefetching should work as described.

Screenshots

System information

  • OS: macOS
  • Version of Next.js: latest

Additional context

This bug also affects the data-prefetch-link library and the next-apollo library.

@Timer Timer added help wanted good first issue Easy to fix issues, good for newcomers labels Feb 26, 2019
@Timer
Copy link
Member

Timer commented Feb 26, 2019

Please send a PR fixing. 😄

@justinwhall
Copy link
Contributor

justinwhall commented Feb 27, 2019

In addition to the missing module issue reported above, looks like maybe some architecture changed here.

This line now returns undefined and it used to return a component.

prefetch used to return this.pageLoader.loadpage by way of fetchRoute.

Now, it checks if hasPreload and if it does, it loads the script and returns.

@timneutkens
Copy link
Member

Yeah I was telling @Timer this example can't be fixed without core changes. We might allow an option passed to router.prefetch however it's a dangerous to use data prefetching already, as it doesn't happen in the background.

@adamsoffer
Copy link
Contributor Author

@timneutkens gotcha. Are you implying that data prefetching is no longer possible with nextjs?

@timneutkens
Copy link
Member

timneutkens commented Feb 27, 2019

I'm saying that this solution/example is using internal APIs hence why it breaks on majors. We should probably expose an external API for loading the component.

@justinwhall
Copy link
Contributor

justinwhall commented Feb 27, 2019

We should probably expose an external API for loading the component.

^^ That would be pretty awesome!

@adamsoffer
Copy link
Contributor Author

adamsoffer commented Apr 11, 2019

Hey @timneutkens 👋 - what are the internal APIs that this example currently relies on that we can expose to make prefetching data possible again?

@kachkaev
Copy link
Contributor

kachkaev commented May 2, 2019

Given that the example no longer works in Next 8, will you be happy to accept a PR that deletes it from canary? I spent a couple of hours setting up something similar in my app before realising that await router.prefetch(parsedHref) is now undefined instead of being a component.

@wongmjane
Copy link
Contributor

wongmjane commented Jul 1, 2019

There's a method called fetchComponent in Router class:

https://github.com/zeit/next.js/blob/564eac47462cf65cd80c140dff72160845e4e4de/packages/next-server/lib/router/router.ts#L528-L549

For some reason fetchComponent isn't mentioned in RouterProps TypeScript type definition:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/3fee2364dfda7c2beb65dc6e1980e41b2ed30751/types/next-server/router.d.ts#L32-L73

If you use TypeScript, it is possible to still call/reference the method by casting RouterProps to something that contains the fetchComponent method.

My following approach isn't perfect but at least it fetched the component, hence able to call the getInitialProps:

import Router from 'next/router'
import { UrlLike, RouterProps } from 'next-server/router'
import { format, resolve, parse } from 'url'

interface MyRouterProps extends RouterProps {
  fetchComponent: (route: string) => Promise<NextComponentType>
}

type UrlAlike = string | UrlLike

export default async (href: UrlAlike, asPath?: UrlAlike) => {
  if (typeof window === 'undefined') return

  const hrefString = href === 'string' ? href : format(href)
  const route = resolve(location.pathname, hrefString)

  if (Router.router === null) return

  // the following fetches the component for the valid route
  const component = await (router as MyRouterProps).fetchComponent(route)
  // ...
}

edit: My approach avoids using Router.prefetch at all because its originating method seems to be written in a way it never returns anything:

https://github.com/zeit/next.js/blob/564eac47462cf65cd80c140dff72160845e4e4de/packages/next/client/page-loader.js#L166-L215

@timneutkens
Copy link
Member

Replied on twitter but also posting here:

https://twitter.com/timneutkens/status/1145573026869760000

It’s a private method, hence why it’s not documented and not available in the types. I think I already replied there that we might want to expose a public method for it, however after that there was no community RFC to try and solve it.

Router.router is private too.

As said in my tweet I'd love for someone to draft up a RFC for this.

Requirements from my side:

  • Doesn't increase Router bundle size (maybe a separate function you import)
  • Only does prefetching work in the background / when idle (as it's quite heavy)

@Timer
Copy link
Member

Timer commented Apr 15, 2020

Closing as this example was deleted. Next.js automatically prefetches data when it's safe now (getStaticProps).

@Timer Timer closed this as completed Apr 15, 2020
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Easy to fix issues, good for newcomers
Projects
None yet
Development

No branches or pull requests

7 participants