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

Prefetch pages with Service Workers #375

Merged
merged 12 commits into from
Dec 15, 2016
Merged

Prefetch pages with Service Workers #375

merged 12 commits into from
Dec 15, 2016

Conversation

arunoda
Copy link
Contributor

@arunoda arunoda commented Dec 10, 2016

This will add support for prefetching pages using service workers.

This PR is based on the master, but porting it to support #310 is pretty simple.

Declarative API

You can simply ask to prefetch pages by using the <Link> provided by next/prefetch. See:

import Link from 'next/prefetch'

// This is the header component
export default () => (
  <div>
    <Link href='/'>Home</Link>
    <Link href='/about'>Home</Link>
    <Link href='/contact'>Home</Link>
  </div>
) 

Now we'll prefetch all the pages. So, when you click on any of the link it won't need to do a network hit.

If you need you could stop prefetching like this:

<Link href='/' prefetch={false}>Home</Link>

Imperative API

You can get started with <Link> prefetching pretty quickly and it's a pretty decent way to prefetch. But you may want to prefetch based on your own logic.

Then you can do it like this:

import { prefetch } from 'next/prefetch'

prefetch('/')
prefetch('/features')

Browser Support

Service Workers are available only in Chrome, Firefox and Opera. For other browsers prefetching won't work. But <Link> from next/prefetch will be work like <Link> from next/link

@coveralls
Copy link

coveralls commented Dec 10, 2016

Coverage Status

Coverage decreased (-3.2%) to 33.979% when pulling 57f57b2 on arunoda:prefetch into 3de4788 on zeit:master.

This is pretty important since initiating will reset the cache.
If we don't do this, it's possible to have old cached resources
after the user decided to remove all of the prefetching logic.
In this case, even the page didn't prefetch it'll use the
previously cached pages. That because of there might be a already running
service worker.
@coveralls
Copy link

coveralls commented Dec 10, 2016

Coverage Status

Coverage decreased (-2.9%) to 34.331% when pulling 09325d9 on arunoda:prefetch into 3de4788 on zeit:master.

@arunoda arunoda closed this Dec 10, 2016
@arunoda arunoda reopened this Dec 10, 2016
@coveralls
Copy link

coveralls commented Dec 10, 2016

Coverage Status

Coverage decreased (-2.9%) to 34.331% when pulling 09325d9 on arunoda:prefetch into 3de4788 on zeit:master.

parser.href = href

return parser.pathname
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use url module here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -0,0 +1,108 @@
/* global self */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this file to client dir ?
pages directory is intended to be for default pages like /_error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks this file is not a target of gulp and webpack. Any reasons ?

Copy link
Contributor Author

@arunoda arunoda Dec 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried at first, but it adds more code (because I use async await). But writing it with pure ES5 make it pretty simple (I just want this to work).
But I'd like to try something different now.

This is the all the features we need to add to the service worker code. Writing ES5 makes it smaller. Do we need to use ES6 and transpile it?

Anyway, I'll move it to the client directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkzawa I tried this now and It'll add ~28K with webpack but without it it's just 4K. Do we still need to proceed with webpack or just use the code written with ES5.

Copy link
Contributor Author

@arunoda arunoda Dec 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkzawa Moved the prefetcher code to client. I also managed to reduced the bundle size by using babel-preset-env and not using async await.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally prefer consistency than small optimizations which we can improve later if required.

@coveralls
Copy link

coveralls commented Dec 10, 2016

Coverage Status

Coverage decreased (-2.7%) to 34.513% when pulling aebe067 on arunoda:prefetch into 3de4788 on zeit:master.

Now we also do a webpack build for the prefetcher code.
@coveralls
Copy link

coveralls commented Dec 10, 2016

Coverage Status

Coverage decreased (-2.7%) to 34.513% when pulling 3db546d on arunoda:prefetch into 3de4788 on zeit:master.

@luisrudge
Copy link

This is going to be nuts! ❤️

@coveralls
Copy link

coveralls commented Dec 12, 2016

Coverage Status

Coverage decreased (-2.7%) to 34.513% when pulling 08c8249 on arunoda:prefetch into 3de4788 on zeit:master.

@coveralls
Copy link

coveralls commented Dec 12, 2016

Coverage Status

Coverage decreased (-2.7%) to 34.513% when pulling cd82429 on arunoda:prefetch into 3de4788 on zeit:master.

@@ -182,6 +182,58 @@ Each top-level component receives a `url` property with the following API:
- `pushTo(url)` - performs a `pushState` call that renders the new `url`. This is equivalent to following a `<Link>`
- `replaceTo(url)` - performs a `replaceState` call that renders the new `url`

### Prefetching Pages
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rauchg here's the docs for README.

@ccorcos
Copy link

ccorcos commented Dec 12, 2016

Doesn't Next already prefetch routes?

@luisrudge
Copy link

@ccorcos no

@ccorcos
Copy link

ccorcos commented Dec 12, 2016

Go to https://zeit.co and look at the network tab when you click on "blog":

screen shot 2016-12-11 at 7 46 29 pm

Also: https://zeit.co/blog/next#anticipation-is-the-key-to-performance

For www.zeit.co we've implemented a technique on top of Next.js that brings us the best of both worlds: every single tag pre-fetches the component's JSON representation on the background, via a ServiceWorker.

@luisrudge
Copy link

they implemented it. it's not on next.js yet. they were running an internal component (probably was extracted as this PR). Related: #24

@ccorcos
Copy link

ccorcos commented Dec 12, 2016

ah, I see

@arunoda arunoda mentioned this pull request Dec 12, 2016
@arunoda
Copy link
Contributor Author

arunoda commented Dec 12, 2016

@ccorcos @luisrudge

Guys, this is a ground up implementation of prefetching for Next.js. It's not based on the zeit.co's implementation but definitely I took some core ideas from that.

With this PR, Next.js doesn't prefetch all the pages. But only pages you have asked to prefetch.
Have a look at the docs for more info.

@ccorcos
Copy link

ccorcos commented Dec 12, 2016

Looking awesome @arunoda

['env', {
'targets': {
// All browsers which supports service workers
'browsers': ['chrome 49', 'firefox 49', 'opera 41']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍 👍

pathname = '/index'
}

const url = `${pathname}.json`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw this url will become the same with non-json url and need to set the Accept header with #310

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I will update this once we merge #310.


_register () {
this.serviceWorkerState = 'REGISTERING'
navigator.serviceWorker.register('/_next-prefetcher.js')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a way to disable Service Worker completely ? I believe we need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get this?
You mean implement prefetching without service workers of something else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean you'd like to not register the script of ServiceWorker at all if you don't need prefetching.
It looks _register() is always called if browser supports ServiceWorker for now ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that way. See this commit message.
09325d9

@queso
Copy link

queso commented Dec 12, 2016

This looks awesome, what else needs done to get this merged?

We also clean the cache always, even we initialize
the service worker or not.
@coveralls
Copy link

coveralls commented Dec 13, 2016

Coverage Status

Coverage decreased (-3.0%) to 34.211% when pulling f862847 on arunoda:prefetch into 3de4788 on zeit:master.

@rauchg rauchg merged commit 36abdc7 into vercel:master Dec 15, 2016
@arunoda arunoda deleted the prefetch branch December 15, 2016 22:12
@zackify
Copy link

zackify commented Dec 16, 2016

When will there be a release with this new prefetch stuff?

@philcockfield
Copy link

@arunoda - this has been merged into master, it doesn't look like it's been released on NPM yet though. Is that right?

@arunoda
Copy link
Contributor Author

arunoda commented Dec 20, 2016

@philcockfield That's right.
@rauchg is looking for few more things before doing 2.0 release.

I assume it's coming pretty soon.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants