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

experimental: unstable_after #65038

Open
wants to merge 10 commits into
base: canary
Choose a base branch
from
Open

experimental: unstable_after #65038

wants to merge 10 commits into from

Conversation

lubieowoce
Copy link
Member

@lubieowoce lubieowoce commented Apr 25, 2024

Implements unstable_after, which lets the user schedule work to be executed after the response is finished.

Implementation notes

  • unstable_after() is a dynamic function (bypassable only with export dynamic = "force-static")

  • Usable in: server components (including generateMetadata), actions, route handlers, middleware

  • It is meant to run its callbacks even if a response didn't complete successfully (thrown error) or called notFound()/redirect()

  • Currently gated behind a experimental.after feature flag, because it touches many runtime bits (including a React monkeypatch...)

  • The state for unstable_after() in a given request lives in requestAsyncStorage (added via RequestAsyncStorageWrapper)

  • the implementation is based around two functions that we inject via renderOpts:

    • waitUntil(promise) - keep a function invocation alive until a promise settles. it is provided as a platform primitive in serverless contexts, and a noop in next start
      • for serverless (nodejs), Next.js will attempt to get waitUntil from globalThis[Symbol.for('@next/request-context')].get().waitUntil. This should be considered unstable for now. See packages/next/src/server/after/wait-until-builtin.ts for details.
    • onClose(callback) [NEW] - run something when a response is done. basically res.on('close', callback), but also implemented for Web APIs
      • unfortunately, for Web, this requires some potentially expensive tricks - see packages/next/src/server/web/web-on-close.ts

Closes NEXT-3224

@ijjk
Copy link
Member

ijjk commented Apr 25, 2024

Tests Passed

@ijjk
Copy link
Member

ijjk commented Apr 25, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js next-after Change
buildDuration 22.9s 20.6s N/A
buildDurationCached 11.5s 10s N/A
nodeModulesSize 346 MB 346 MB ⚠️ +569 kB
nextStartRea..uration (ms) 511ms 525ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js next-after Change
2141-HASH.js gzip 33.5 kB 33.5 kB N/A
2592-HASH.js gzip 5.06 kB 5.06 kB N/A
48cf7de5-HASH.js gzip 51 kB 51 kB N/A
6539.HASH.js gzip 169 B 169 B
framework-HASH.js gzip 56 kB 56 kB N/A
main-app-HASH.js gzip 219 B 222 B N/A
main-HASH.js gzip 32.3 kB 32.3 kB N/A
webpack-HASH.js gzip 1.7 kB 1.7 kB N/A
Overall change 169 B 169 B
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js next-after Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary vercel/next.js next-after Change
_app-HASH.js gzip 193 B 193 B
_error-HASH.js gzip 191 B 192 B N/A
amp-HASH.js gzip 511 B 510 B N/A
css-HASH.js gzip 341 B 341 B
dynamic-HASH.js gzip 2.52 kB 2.53 kB N/A
edge-ssr-HASH.js gzip 265 B 266 B N/A
head-HASH.js gzip 363 B 365 B N/A
hooks-HASH.js gzip 392 B 392 B
image-HASH.js gzip 4.27 kB 4.27 kB N/A
index-HASH.js gzip 268 B 266 B N/A
link-HASH.js gzip 2.69 kB 2.69 kB N/A
routerDirect..HASH.js gzip 328 B 326 B N/A
script-HASH.js gzip 396 B 395 B N/A
withRouter-HASH.js gzip 325 B 320 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 1.03 kB 1.03 kB
Client Build Manifests
vercel/next.js canary vercel/next.js next-after Change
_buildManifest.js gzip 481 B 483 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js next-after Change
index.html gzip 521 B 521 B
link.html gzip 535 B 534 B N/A
withRouter.html gzip 516 B 519 B N/A
Overall change 521 B 521 B
Edge SSR bundle Size Overall increase ⚠️
vercel/next.js canary vercel/next.js next-after Change
edge-ssr.js gzip 121 kB 122 kB ⚠️ +1.22 kB
page.js gzip 179 kB 181 kB ⚠️ +2.01 kB
Overall change 300 kB 303 kB ⚠️ +3.23 kB
Middleware size Overall increase ⚠️
vercel/next.js canary vercel/next.js next-after Change
middleware-b..fest.js gzip 658 B 656 B N/A
middleware-r..fest.js gzip 156 B 155 B N/A
middleware.js gzip 25.8 kB 27 kB ⚠️ +1.23 kB
edge-runtime..pack.js gzip 839 B 1.02 kB ⚠️ +182 B
Overall change 26.7 kB 28.1 kB ⚠️ +1.41 kB
Next Runtimes Overall increase ⚠️
vercel/next.js canary vercel/next.js next-after Change
app-page-exp...dev.js gzip 177 kB 177 kB ⚠️ +689 B
app-page-exp..prod.js gzip 108 kB 108 kB ⚠️ +699 B
app-page-tur..prod.js gzip 117 kB 118 kB ⚠️ +692 B
app-page-tur..prod.js gzip 94.6 kB 95.3 kB ⚠️ +691 B
app-page.run...dev.js gzip 160 kB 161 kB ⚠️ +726 B
app-page.run..prod.js gzip 93.2 kB 93.9 kB ⚠️ +702 B
app-route-ex...dev.js gzip 21.1 kB 21.8 kB ⚠️ +704 B
app-route-ex..prod.js gzip 15 kB 15.7 kB ⚠️ +709 B
app-route-tu..prod.js gzip 15 kB 15.7 kB ⚠️ +709 B
app-route-tu..prod.js gzip 14.8 kB 15.5 kB ⚠️ +703 B
app-route.ru...dev.js gzip 20.9 kB 21.6 kB ⚠️ +702 B
app-route.ru..prod.js gzip 14.8 kB 15.5 kB ⚠️ +702 B
pages-api-tu..prod.js gzip 9.55 kB 9.55 kB
pages-api.ru...dev.js gzip 9.82 kB 9.82 kB
pages-api.ru..prod.js gzip 9.55 kB 9.55 kB
pages-turbo...prod.js gzip 21.5 kB 21.5 kB
pages.runtim...dev.js gzip 22 kB 22 kB
pages.runtim..prod.js gzip 21.5 kB 21.5 kB
server.runti..prod.js gzip 51.8 kB 52 kB ⚠️ +160 B
Overall change 996 kB 1.01 MB ⚠️ +8.59 kB
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js next-after Change
0.pack gzip 1.65 MB 1.66 MB ⚠️ +17.5 kB
index.pack gzip 127 kB 128 kB ⚠️ +1.2 kB
Overall change 1.77 MB 1.79 MB ⚠️ +18.7 kB
Diff details
Diff for page.js

Diff too large to display

Diff for edge-runtime-webpack.js
@@ -117,6 +117,54 @@
     /******/
   })();
   /******/
+  /******/ /* webpack/runtime/create fake namespace object */
+  /******/ (() => {
+    /******/ var getProto = Object.getPrototypeOf
+      ? (obj) => Object.getPrototypeOf(obj)
+      : (obj) => obj.__proto__;
+    /******/ var leafPrototypes;
+    /******/ // create a fake namespace object
+    /******/ // mode & 1: value is a module id, require it
+    /******/ // mode & 2: merge all properties of value into the ns
+    /******/ // mode & 4: return value when already ns object
+    /******/ // mode & 16: return value when it's Promise-like
+    /******/ // mode & 8|1: behave like require
+    /******/ __webpack_require__.t = function (value, mode) {
+      /******/ if (mode & 1) value = this(value);
+      /******/ if (mode & 8) return value;
+      /******/ if (typeof value === "object" && value) {
+        /******/ if (mode & 4 && value.__esModule) return value;
+        /******/ if (mode & 16 && typeof value.then === "function")
+          return value;
+        /******/
+      }
+      /******/ var ns = Object.create(null);
+      /******/ __webpack_require__.r(ns);
+      /******/ var def = {};
+      /******/ leafPrototypes = leafPrototypes || [
+        null,
+        getProto({}),
+        getProto([]),
+        getProto(getProto),
+      ];
+      /******/ for (
+        var current = mode & 2 && value;
+        typeof current == "object" && !~leafPrototypes.indexOf(current);
+        current = getProto(current)
+      ) {
+        /******/ Object.getOwnPropertyNames(current).forEach(
+          (key) => (def[key] = () => value[key])
+        );
+        /******/
+      }
+      /******/ def["default"] = () => value;
+      /******/ __webpack_require__.d(ns, def);
+      /******/ return ns;
+      /******/
+    };
+    /******/
+  })();
+  /******/
   /******/ /* webpack/runtime/define property getters */
   /******/ (() => {
     /******/ // define getter functions for harmony exports
Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for image-HASH.js
@@ -1,7 +1,7 @@
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
   [8358],
   {
-    /***/ 5498: /***/ (
+    /***/ 8536: /***/ (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/image",
         function () {
-          return __webpack_require__(8082);
+          return __webpack_require__(3672);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 771: /***/ (module, exports, __webpack_require__) => {
+    /***/ 400: /***/ (module, exports, __webpack_require__) => {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -40,15 +40,15 @@
         __webpack_require__(5614)
       );
       const _head = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(1549)
+        __webpack_require__(2537)
       );
-      const _getimgprops = __webpack_require__(6472);
-      const _imageconfig = __webpack_require__(6915);
-      const _imageconfigcontextsharedruntime = __webpack_require__(3993);
-      const _warnonce = __webpack_require__(3260);
-      const _routercontextsharedruntime = __webpack_require__(4781);
+      const _getimgprops = __webpack_require__(4031);
+      const _imageconfig = __webpack_require__(8267);
+      const _imageconfigcontextsharedruntime = __webpack_require__(6469);
+      const _warnonce = __webpack_require__(5240);
+      const _routercontextsharedruntime = __webpack_require__(5635);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(933)
+        __webpack_require__(271)
       );
       // This is replaced by webpack define plugin
       const configEnv = {
@@ -376,7 +376,7 @@
       /***/
     },
 
-    /***/ 6472: /***/ (
+    /***/ 4031: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -392,9 +392,9 @@
           return getImgProps;
         },
       });
-      const _warnonce = __webpack_require__(3260);
-      const _imageblursvg = __webpack_require__(7851);
-      const _imageconfig = __webpack_require__(6915);
+      const _warnonce = __webpack_require__(5240);
+      const _imageblursvg = __webpack_require__(2682);
+      const _imageconfig = __webpack_require__(8267);
       const VALID_LOADING_VALUES =
         /* unused pure expression or super */ null && [
           "lazy",
@@ -769,7 +769,7 @@
       /***/
     },
 
-    /***/ 7851: /***/ (__unused_webpack_module, exports) => {
+    /***/ 2682: /***/ (__unused_webpack_module, exports) => {
       "use strict";
       /**
        * A shared function, used on both client and server, to generate a SVG blur placeholder.
@@ -824,7 +824,7 @@
       /***/
     },
 
-    /***/ 4770: /***/ (
+    /***/ 3669: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -851,10 +851,10 @@
         },
       });
       const _interop_require_default = __webpack_require__(1478);
-      const _getimgprops = __webpack_require__(6472);
-      const _imagecomponent = __webpack_require__(771);
+      const _getimgprops = __webpack_require__(4031);
+      const _imagecomponent = __webpack_require__(400);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(933)
+        __webpack_require__(271)
       );
       function getImageProps(imgProps) {
         const { props } = (0, _getimgprops.getImgProps)(imgProps, {
@@ -886,7 +886,7 @@
       /***/
     },
 
-    /***/ 933: /***/ (__unused_webpack_module, exports) => {
+    /***/ 271: /***/ (__unused_webpack_module, exports) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -921,7 +921,7 @@
       /***/
     },
 
-    /***/ 8082: /***/ (
+    /***/ 3672: /***/ (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -938,8 +938,8 @@
 
       // EXTERNAL MODULE: ./node_modules/.pnpm/react@19.0.0-beta-04b058868c-20240508/node_modules/react/jsx-runtime.js
       var jsx_runtime = __webpack_require__(1847);
-      // EXTERNAL MODULE: ./node_modules/.pnpm/file+..+main-repo+packages+next+next-packed.tgz_react-dom@19.0.0-beta-04b058868c-20240508_rea_x6nrtfoohwvncerarvzrfulp2u/node_modules/next/image.js
-      var next_image = __webpack_require__(5945);
+      // EXTERNAL MODULE: ./node_modules/.pnpm/file+..+diff-repo+packages+next+next-packed.tgz_react-dom@19.0.0-beta-04b058868c-20240508_rea_dbavudu66yewnjanau4b4qzpou/node_modules/next/image.js
+      var next_image = __webpack_require__(7037);
       var image_default = /*#__PURE__*/ __webpack_require__.n(next_image); // CONCATENATED MODULE: ./pages/nextjs.png
       /* harmony default export */ const nextjs = {
         src: "/_next/static/media/nextjs.cae0b805.png",
@@ -969,12 +969,12 @@
       /***/
     },
 
-    /***/ 5945: /***/ (
+    /***/ 7037: /***/ (
       module,
       __unused_webpack_exports,
       __webpack_require__
     ) => {
-      module.exports = __webpack_require__(4770);
+      module.exports = __webpack_require__(3669);
 
       /***/
     },
@@ -984,7 +984,7 @@
     /******/ var __webpack_exec__ = (moduleId) =>
       __webpack_require__((__webpack_require__.s = moduleId));
     /******/ __webpack_require__.O(0, [2888, 9774, 179], () =>
-      __webpack_exec__(5498)
+      __webpack_exec__(8536)
     );
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;
Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js
failed to diff
Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js
failed to diff
Diff for app-page.runtime.prod.js

Diff too large to display

Diff for app-route-ex..ntime.dev.js

Diff too large to display

Diff for app-route-ex..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route.runtime.dev.js

Diff too large to display

Diff for app-route.ru..time.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: 7b77613

@lubieowoce lubieowoce changed the title next/after [WIP] [WIP] next/after Apr 25, 2024
@lubieowoce lubieowoce force-pushed the next-after branch 6 times, most recently from cf97e74 to d1d7664 Compare April 26, 2024 17:09
@ijjk ijjk added the Turbopack Related to Turbopack with Next.js. label May 7, 2024
@lubieowoce lubieowoce self-assigned this May 7, 2024
@lubieowoce lubieowoce force-pushed the next-after branch 3 times, most recently from 8e2a9ad to 0d7f029 Compare May 8, 2024 22:54
Copy link
Member Author

@lubieowoce lubieowoce May 8, 2024

Choose a reason for hiding this comment

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

i'm a little bit on the fence about how this test is structured, because it really tests the implementation details quite heavily. but OTOH, i find the interplay of [onClose, waitUntil and everything else] confusing enough that it feels useful to spell out exactly how this is meant to behave.

(also, some of the details of when exactly something's getting called are pretty tricky to test in an E2E way -- observing stuff that happens after the response is finished is kinda hard)

@@ -1391,6 +1391,9 @@ export async function buildAppStaticPaths({
incrementalCache,
supportsDynamicHTML: true,
isRevalidate: false,
experimental: {
after: false,
Copy link
Member Author

@lubieowoce lubieowoce May 8, 2024

Choose a reason for hiding this comment

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

in theory this might yield a confusing error message ("enable experimental.after in next.config.js") even if it's enabled, because there's currently no good way to encode "enabled, but not allowed in this context".

in practice, it shouldn't happen, because after() should trigger a static bailout before it gets a chance to print that other message, but i need to double-check that.

afterContext: {
enabled: true,
after: () => {
throw new Error('Cannot call after() from within after()')
Copy link
Member Author

Choose a reason for hiding this comment

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

after-within-after is a feature we probably want, but i think it'd be better to leave that for a follow-up PR -- this is big enough as it is.

Copy link
Member Author

Choose a reason for hiding this comment

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

This a bit nasty, but it lets cache()d functions work as expected within after callbacks -- not having that would probably end up being a pretty big footgun. Hopefully we can upstream this soon, as it's pretty small

(this also ties in nicely with after being gated behind an experimental flag -- when we upstream it, createCacheScope would initially only be present in experimental builds of React, so we'd make the flag opt into that.)

}
this.waitUntil(task)
} else if (typeof task === 'function') {
// TODO(after): will this trace correctly?
Copy link
Member

Choose a reason for hiding this comment

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

The tracing looks fine, let's remove these TODO(after) comment questions?

Copy link
Member Author

Choose a reason for hiding this comment

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

the thing i'm unsure about is that this callback could run after the root request span is finished, and i'm not sure what'd happen with it in that case -- can you attach more child spans to a span that's already finished? and if not, what do we do with them?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like this tracing doesn't actually work because of some subtleties of when spans stop recording. dropping this for now, will add proper tracing as a follow up.

@lubieowoce lubieowoce force-pushed the next-after branch 5 times, most recently from cd92b2a to 0109746 Compare May 16, 2024 08:31
@lubieowoce lubieowoce changed the title next/after experimental: unstable_after May 16, 2024
@lubieowoce lubieowoce force-pushed the next-after branch 2 times, most recently from 5cc8b6c to d3da97b Compare May 17, 2024 14:17
Copy link
Member

@huozhi huozhi left a comment

Choose a reason for hiding this comment

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

Looks good on the parts I reviewed:

  • We agree to skip the tracing in this PR since it's failing now, and get a follow up for it
  • There're a few errors added feel bit concerned but most need to be asserted to make sure the feature is working, like react cache patching part unfortunately need to have that for now.
  • types change all good

@lubieowoce lubieowoce force-pushed the next-after branch 2 times, most recently from c3a3aff to 11c2f24 Compare May 17, 2024 16:43
Copy link
Member

@timneutkens timneutkens left a comment

Choose a reason for hiding this comment

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

The test have to run against edge as discussed while going through the diff together. Besides that this looks good to me. We'll want to use a different value the request context as Wyatt/Zack highlighted as well.

@lubieowoce lubieowoce enabled auto-merge (squash) May 18, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
created-by: Next.js team PRs by the Next.js team tests Turbopack Related to Turbopack with Next.js. type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants