Implement "on demand entries" #1111

arunoda · 2017-02-13T21:05:17Z

Related #608

This is a dev only optimization

This is new concept to reduce the dev-rebuild time dramatically if you have a lot of pages in your app.

With this, we don't add pages to webpack when building the dev server. But when we are accessing the page, we'll build the page and add it webpack.

With this, we were able to cut down "15 secs" dev re-build time to "1-2" secs.

In order to use this productively, we have to disable prefetching pages when we are in development.

TODO:

Remove older pages from webpack
- Get the currently active page from the client side.
Add some integration tests

Otherwise it'll discard the use of dynamic-entries.

rauchg · 2017-02-13T21:22:51Z

Why optional?

arunoda · 2017-02-13T21:27:02Z

No special reason. Since this is not the way how webpack works normally, some users may don't like this.

So, we should implement this as opt-in or opt-out.

timneutkens · 2017-02-13T23:37:23Z

This fixes #608 right?

arunoda · 2017-02-14T01:42:17Z

@timneutkens Yes. It's has the core. Let's make both related.

nkzawa · 2017-02-14T11:37:43Z

client/on-demand-entries-client.js

+
+function ping () {
+  const url = `/on-demand-entries-ping?page=${Router.pathname}`
+  fetch(url)


What is this for ?

That's to watch active entries and dispose inactive pages.

Where is it implemented ? on-demand-entry-handler.js#middleware is doing nothing ?

Check now. I was implementing that :)

nkzawa · 2017-02-14T11:46:43Z

server/on-demand-entry-handler.js

+    compilation.addEntry(context, dep, name, (err) => {
+      if (err) return reject(err)
+      resolve()
+    })


I think we should not use private plugins.
Webpack v2 supports dynamic entries by setting a function to the entry option. So what we'd like to do is to change returned entries according to page requests. I think we can do that with an optional entry function on server/build/webpack.js.

This is not a private function. It's documented by webpack.

Anyway, I'll try to use dynamic functions in the refactoring stage.
For now, this seems like the best option for now as we do more stuff related to entry management.

I meant about plugins 'webpack/lib/DynamicEntryPlugin'. I'd like to fix that in this PR if possible or I will do it later.

arunoda · 2017-02-15T20:26:47Z

EDIT: Actually, this issue was there from the beginning. See: #1165

After 8a495aa HMR is not working correctly.

arunoda · 2017-02-25T06:52:45Z

@timneutkens This is in 2.0.
We are taking this early next week.

timneutkens · 2017-02-25T19:15:28Z

@arunoda great, I want it in 😄 ❤️ , @rauchg said after 2.0 in #next on slack yesterday, that's why I added the milestone 👍

leo · 2017-02-25T20:51:54Z

@arunoda ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️

I hope you know that you're literally saving me and the ZEIT team a lot of valuable lifetime.

arunoda · 2017-02-26T02:44:46Z

timneutkens I think we can't wait that long :)

arunoda · 2017-02-26T02:59:56Z

Guys this is ready. Could you run this few tests before we merge in?

arunoda · 2017-02-26T03:03:27Z

server/on-demand-entry-handler.js

+  dev,
+  maxInactiveAge = 1000 * 25
+}) {
+  const server = await getServer()


Here we are starting a websocket server to accept pings.
The best idea is to hook into our existing app.
But with our API, we don't have access to the server object. So, we can't do that without changing the API.

Since this is dev only, it's fine to start a new server. Which is lightweight of course.

Why?

Earlier we did by sending a HTTP request from the client for every 5 secs. But those requests could be blocked at any time by the chrome for sending too much of requests.

And that could block HMR updates as well.
So, that's why we moved to WS.

But those requests could be blocked at any time by the chrome for sending too much of requests.

Elaborate please?

Chrome(and other browsers) has 6 concurrent connection limit for hosts. We won't send 6 connections but sometimes chrome set some of these requests into the stalled state.
May be it things we are sending too many requests.

Then it'll block the request which webpack asks for HMR changes time to time.
I couldn't find a better way other than using a Websocket connection.

I think Chrome sets it at 8 nowadays. Even then, we would only take up one socket ? Also, I believe webpack uses WS?

Even then, we would only take up one socket ?

With HTTP2 yes it is. But it's not there always.

I believe webpack uses WS?

No it's not. It uses server sent events(HMR middleware) + 2 HTTP requests.

This reverts commit f706a49.

sedubois · 2017-02-26T12:55:49Z

lib/router/router.js

@@ -224,6 +224,10 @@ export default class Router extends EventEmitter {
  }

  async prefetch (url) {
+    // We don't add support for prefetch in the development mode.
+    // If we do that, our on-demand-entries optimization won't performs better
+    if (process.env.NODE_ENV === 'development') return


Some other places test with process.env.NODE_ENV !== 'production' instead.

Developers might have forgotten to define process.env.NODE_ENV but still expect to have a development behaviour in that case.

I think it would be nice if all the code tested whether it's in a production mode or not, in a coherent way.

I wonder if we could use the dev provided when setting up the app (server side). Not sure if we pass it through to the client side though, will allow for consistent behaviour not based on NODE_ENV.

@sedubois we set NODE_ENV always. See: https://github.com/zeit/next.js/pull/1111/files#diff-0f2f34c098f5954f99483c9bd61e439dR94

We didn't use 'production' check here because, we need to allow prefetching in JEST.
So, this is safe.

sedubois · 2017-02-26T12:58:17Z

It's mentioned that this should be opt-in/-out configurable, but I don't see any user documentation added in this PR.

arunoda · 2017-02-26T13:29:43Z

@sedubois currently we make this as default and there'll be no opt-out.
We'll go with the beta like that.
Then if we need to make it opt-out, let's do it.

@rauchg proposed that and the idea is to remove options as possible as we can. (Without harming the dev/prod experience).

jschumme · 2017-02-27T21:30:34Z

I'm wondering if that change causes a permanent recompiling of my application in development. It actually stops after 16 recompilations.

 Ready on http://localhost:3000
Client pings, but there's no entry for page: /
> Building page: /
 DONE  Compiled successfully in 5545ms22:20:00

 WAIT  Compiling...22:20:00

 DONE  Compiled successfully in 259ms22:20:01

 WAIT  Compiling...22:20:01

 DONE  Compiled successfully in 258ms22:20:01

 WAIT  Compiling...22:20:01

 DONE  Compiled successfully in 190ms22:20:02

 WAIT  Compiling...22:20:02

 DONE  Compiled successfully in 270ms22:20:03

rauchg · 2017-02-27T21:31:15Z

@DarKAdmiral a fix is coming. Beta 34 is on your way.

Rajat421 · 2017-03-06T12:16:30Z

client/on-demand-entries-client.js

+
+function ping () {
+  const url = `/on-demand-entries-ping?page=${Router.pathname}`
+  fetch(url)


(node:18740) DeprecationWarning: loaderUtils.parseQuery() received a non-string value which can be problematic, see webpack/loader-utils#56
parseQuery() will be replaced with getOptions() in the next major version of loader-utils.

i am recieving this warning can you tell me how to resolve this, this warning came when i've updated my 'next' version

Can you create an issue for this?

mojo5000 · 2017-04-29T14:21:05Z

So there is no 'opt-out' flag avail at this time?

arunoda · 2017-04-29T16:51:01Z

@mojo5000 nope. Currently there's no such flag.

arunoda added 8 commits February 13, 2017 09:32

Add a plan for dynamic entry middleware.

b295c24

Use dynamic pages middleware to load pages in dev.

3d975ef

Add the first version of middleware but not tested.

93b0967

Integrated.

e237580

Disable prefetching in development.

04754a8

Otherwise it'll discard the use of dynamic-entries.

Build custom document and error always.

b2383e1

Refactor code base.

93fc9d6

Change branding as on-demand entries.

dd5fb13

arunoda added 2 commits February 14, 2017 08:56

Fix tests.

39219d0

Add a client side pinger for on-demand-entries.

fc0a54f

nkzawa reviewed Feb 14, 2017

View reviewed changes

arunoda added 4 commits February 14, 2017 18:44

Dispose inactive entries.

617c33f

Add proper logs.

cf0da3f

Update grammer changes.

f40d425

Resolve conflicts.

0ab1c28

arunoda changed the title ~~[WIP] Implement "on demand entries"~~ Implement "on demand entries" Feb 14, 2017

arunoda added 3 commits February 15, 2017 10:18

Add integration tests for ondemand entries.

620e838

Improve ondemand entry disposing logic.

bea21d5

Try to improve testing.

d32c56a

arunoda force-pushed the ondemand-entries branch from 9370d6b to d32c56a Compare February 15, 2017 06:26

arunoda added 3 commits February 15, 2017 12:05

Make sure entries are not getting disposed in basic integration tests.

8a495aa

Resolve conflicts.

30b6351

Resolve conflicts.

fbca518

arunoda changed the title ~~Implement "on demand entries"~~ [WIP] Implement "on demand entries" Feb 15, 2017

arunoda added 2 commits February 25, 2017 11:12

Improve the pinger code.

ddf6a0c

Touch the first page to speed up the future rebuild times.

d11c5b3

arunoda modified the milestones: 2.0, 2.1 Feb 25, 2017

Add Websockets based pinger.

f706a49

Resolve conflicts on yarn.lock.

a6df1cc

arunoda commented Feb 26, 2017

View reviewed changes

arunoda added 2 commits February 26, 2017 18:15

Revert "Add Websockets based pinger."

3166858

This reverts commit f706a49.

Resolve conflicts.

291c827

sedubois reviewed Feb 26, 2017

View reviewed changes

arunoda added 2 commits February 26, 2017 18:51

Do not send requests per every route change.

07605a1

Make sure we are completing the middleware request always.

4278a18

Make sure test pages are prebuilt.

f87f796

rauchg merged commit d3b1ead into vercel:master Feb 26, 2017

arunoda deleted the ondemand-entries branch February 26, 2017 19:56

timneutkens mentioned this pull request Feb 28, 2017

Lazy compilation during development #608

Closed

Rajat421 reviewed Mar 6, 2017

View reviewed changes

fridays mentioned this pull request Jul 11, 2017

Enhancements fridays/next-routes#71

Closed

lock bot locked as resolved and limited conversation to collaborators May 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement "on demand entries" #1111

Implement "on demand entries" #1111

arunoda commented Feb 13, 2017 •

edited

rauchg commented Feb 13, 2017

arunoda commented Feb 13, 2017

timneutkens commented Feb 13, 2017

arunoda commented Feb 14, 2017

nkzawa Feb 14, 2017

arunoda Feb 14, 2017

nkzawa Feb 14, 2017

arunoda Feb 14, 2017

nkzawa Feb 14, 2017 •

edited

arunoda Feb 14, 2017 •

edited

nkzawa Feb 14, 2017

arunoda commented Feb 15, 2017 •

edited

arunoda commented Feb 25, 2017

timneutkens commented Feb 25, 2017 •

edited

leo commented Feb 25, 2017 •

edited

arunoda commented Feb 26, 2017 •

edited

arunoda commented Feb 26, 2017 •

edited

arunoda Feb 26, 2017

rauchg Feb 26, 2017

arunoda Feb 26, 2017

rauchg Feb 26, 2017

arunoda Feb 26, 2017 •

edited

sedubois Feb 26, 2017

timneutkens Feb 26, 2017

arunoda Feb 26, 2017

sedubois commented Feb 26, 2017 •

edited

arunoda commented Feb 26, 2017

jschumme commented Feb 27, 2017

rauchg commented Feb 27, 2017

Rajat421 Mar 6, 2017 •

edited

timneutkens Mar 6, 2017

mojo5000 commented Apr 29, 2017

arunoda commented Apr 29, 2017

Implement "on demand entries" #1111

Implement "on demand entries" #1111

Conversation

arunoda commented Feb 13, 2017 • edited

rauchg commented Feb 13, 2017

arunoda commented Feb 13, 2017

timneutkens commented Feb 13, 2017

arunoda commented Feb 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkzawa Feb 14, 2017 • edited

Choose a reason for hiding this comment

arunoda Feb 14, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arunoda commented Feb 15, 2017 • edited

arunoda commented Feb 25, 2017

timneutkens commented Feb 25, 2017 • edited

leo commented Feb 25, 2017 • edited

arunoda commented Feb 26, 2017 • edited

arunoda commented Feb 26, 2017 • edited

Choose a reason for hiding this comment

Why?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arunoda Feb 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sedubois commented Feb 26, 2017 • edited

arunoda commented Feb 26, 2017

jschumme commented Feb 27, 2017

rauchg commented Feb 27, 2017

Rajat421 Mar 6, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mojo5000 commented Apr 29, 2017

arunoda commented Apr 29, 2017

arunoda commented Feb 13, 2017 •

edited

nkzawa Feb 14, 2017 •

edited

arunoda Feb 14, 2017 •

edited

arunoda commented Feb 15, 2017 •

edited

timneutkens commented Feb 25, 2017 •

edited

leo commented Feb 25, 2017 •

edited

arunoda commented Feb 26, 2017 •

edited

arunoda commented Feb 26, 2017 •

edited

arunoda Feb 26, 2017 •

edited

sedubois commented Feb 26, 2017 •

edited

Rajat421 Mar 6, 2017 •

edited