produce source maps for middlewares #34409

Schniz · 2022-02-16T08:49:17Z

This PR will generate source maps for middlewares. It's not under any flag (should it be?)

Closes #34523

ijjk · 2022-02-16T21:12:43Z

Failing test suites

Commit: 6bdf34c

yarn testheadless test/integration/gssp-ssr-change-reloading/test/index.test.js

GS(S)P Server-Side Change Reloading > should update page when getStaticProps is changed only
GS(S)P Server-Side Change Reloading > should show indicator when re-fetching data
GS(S)P Server-Side Change Reloading > should update page when getStaticPaths is changed only
GS(S)P Server-Side Change Reloading > should update page when getServerSideProps is changed only

Expand output

● GS(S)P Server-Side Change Reloading › should update page when getStaticProps is changed only

expect(received).toBe(expected) // Object.is equality

Expected: 1
Received: 2

  56 |
  57 |     const props = JSON.parse(await browser.elementByCss('#props').text())
> 58 |     expect(props.count).toBe(1)
     |                         ^
  59 |
  60 |     const page = new File(join(appDir, 'pages/gsp-blog/[post].js'))
  61 |     page.replace('count = 1', 'count = 2')

  at Object.<anonymous> (integration/gssp-ssr-change-reloading/test/index.test.js:58:25)

● GS(S)P Server-Side Change Reloading › should show indicator when re-fetching data

expect(received).toBe(expected) // Object.is equality

Expected: 1
Received: 2

  82 |
  83 |     const props = JSON.parse(await browser.elementByCss('#props').text())
> 84 |     expect(props.count).toBe(1)
     |                         ^
  85 |
  86 |     const page = new File(join(appDir, 'pages/gsp-blog/[post].js'))
  87 |     page.replace('count = 1', 'count = 2')

  at Object.<anonymous> (integration/gssp-ssr-change-reloading/test/index.test.js:84:25)

● GS(S)P Server-Side Change Reloading › should update page when getStaticPaths is changed only

expect(received).toBe(expected) // Object.is equality

Expected: 1
Received: 2

  108 |
  109 |     const props = JSON.parse(await browser.elementByCss('#props').text())
> 110 |     expect(props.count).toBe(1)
      |                         ^
  111 |
  112 |     const page = new File(join(appDir, 'pages/gsp-blog/[post].js'))
  113 |     page.replace('paths = 1', 'paths = 2')

  at Object.<anonymous> (integration/gssp-ssr-change-reloading/test/index.test.js:110:25)

● GS(S)P Server-Side Change Reloading › should update page when getServerSideProps is changed only

TIMED OUT: 1

2

  474 |
  475 |   if (hardError) {
> 476 |     throw new Error('TIMED OUT: ' + regex + '\n\n' + content)
      |           ^
  477 |   }
  478 |   return false
  479 | }

  at Object.check (lib/next-test-utils.js:476:11)
  at Object.<anonymous> (integration/gssp-ssr-change-reloading/test/index.test.js:170:5)

Read more about building and testing Next.js in contributing.md.

timneutkens · 2022-02-17T10:41:57Z

packages/next/build/webpack-config.ts

@@ -1277,6 +1277,16 @@ export default async function getBaseWebpackConfig(
      ].filter(Boolean),
    },
    plugins: [
+      // Produce source maps for middlewares
+      new webpack.SourceMapDevToolPlugin({


Just to be super clear about this @sokra this does not enable sourcemap generation for all files and then discard only the ones in include right?

This should probably only be applied for production right?

It only generates SourceMaps for the included files, but it enables SourceMap generation in loaders for all modules, so that could affect performance...

timneutkens

Blocking this pr on perf concerns that need to be addressed.

packages/next/build/webpack/plugins/middleware-source-maps-plugin.ts

… chunks > I would juest omit that and leave source maps on for them Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>

packages/next/build/webpack-config.ts

timneutkens · 2022-02-24T12:48:20Z

packages/next/build/webpack/plugins/middleware-source-maps-plugin.ts

+    const PLUGIN_NAME = 'NextJsMiddlewareSourceMapsPlugin'
+    compiler.hooks.compilation.tap(PLUGIN_NAME, (compilation) => {
+      compilation.hooks.buildModule.tap(PLUGIN_NAME, (module) => {
+        module.useSourceMap = module.layer === 'middleware'


This seems to override other conditionals if you have sourcemaps enabled in a different way 🤔

Probably needs

if(!module.useSourceMap) to make sure you don't disable other plugins enabling sourcemaps for certain modules.

timneutkens · 2022-02-24T12:50:12Z

packages/next/build/webpack-config.ts

@@ -1272,6 +1273,11 @@ export default async function getBaseWebpackConfig(
      ].filter(Boolean),
    },
    plugins: [
+      ...(!isServer &&


This still applies in development where eval sourcemaps are already enabled. Is that intended?

timneutkens

This needs to be changed, then it looks good: https://github.com/vercel/next.js/pull/34409/files#r813847032

ijjk · 2022-02-24T15:16:08Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
buildDuration	18.9s	18.2s	-698ms
buildDurationCached	7.2s	7.1s	-116ms
nodeModulesSize	367 MB	367 MB	⚠️ +3.54 kB

Page Load Tests Overall increase ✓

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.895	3.675	-0.22
/ avg req/sec	641.93	680.22	+38.29
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2	1.942	-0.06
/error-in-render avg req/sec	1249.82	1287.58	+37.76

Client Bundles (main, webpack)

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42 kB	42 kB	✓
main-HASH.js gzip	27.8 kB	27.8 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	71.5 kB	71.5 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
_app-HASH.js gzip	1.36 kB	1.36 kB	✓
_error-HASH.js gzip	194 B	194 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	326 B	326 B	✓
dynamic-HASH.js gzip	2.57 kB	2.57 kB	✓
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	919 B	919 B	✓
image-HASH.js gzip	5.05 kB	5.05 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	2.26 kB	2.26 kB	✓
routerDirect..HASH.js gzip	321 B	321 B	✓
script-HASH.js gzip	383 B	383 B	✓
withRouter-HASH.js gzip	318 B	318 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	14.7 kB	14.7 kB	✓

Client Build Manifests

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
_buildManifest.js gzip	460 B	460 B	✓
Overall change	460 B	460 B	✓

Rendered Page Sizes

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
index.html gzip	531 B	531 B	✓
link.html gzip	544 B	544 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Default Build with SWC (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
buildDuration	22.5s	22s	-533ms
buildDurationCached	6.9s	7.3s	⚠️ +451ms
nodeModulesSize	367 MB	367 MB	⚠️ +3.54 kB

Page Load Tests Overall increase ✓

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.939	3.836	-0.1
/ avg req/sec	634.7	651.66	+16.96
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.974	1.967	-0.01
/error-in-render avg req/sec	1266.52	1270.67	+4.15

Client Bundles (main, webpack)

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.1 kB	42.1 kB	✓
main-HASH.js gzip	27.8 kB	27.8 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	71.6 kB	71.6 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
_app-HASH.js gzip	1.35 kB	1.35 kB	✓
_error-HASH.js gzip	180 B	180 B	✓
amp-HASH.js gzip	305 B	305 B	✓
css-HASH.js gzip	321 B	321 B	✓
dynamic-HASH.js gzip	2.56 kB	2.56 kB	✓
head-HASH.js gzip	342 B	342 B	✓
hooks-HASH.js gzip	911 B	911 B	✓
image-HASH.js gzip	5.08 kB	5.08 kB	✓
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	2.28 kB	2.28 kB	✓
routerDirect..HASH.js gzip	314 B	314 B	✓
script-HASH.js gzip	375 B	375 B	✓
withRouter-HASH.js gzip	309 B	309 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	14.7 kB	14.7 kB	✓

Client Build Manifests

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes

	vercel/next.js canary	Schniz/next.js create-sourcemaps-for-middlewares	Change
index.html gzip	534 B	534 B	✓
link.html gzip	547 B	547 B	✓
withRouter.html gzip	527 B	527 B	✓
Overall change	1.61 kB	1.61 kB	✓

Commit: a3910a8

Schniz requested review from timneutkens, ijjk, shuding, padmaia and huozhi as code owners February 16, 2022 08:49

ijjk added the type: next label Feb 16, 2022

This comment has been minimized.

Sign in to view

Schniz force-pushed the create-sourcemaps-for-middlewares branch from 6b760ce to 4758d92 Compare February 16, 2022 18:18

This comment has been minimized.

Sign in to view

Schniz force-pushed the create-sourcemaps-for-middlewares branch from 6bdf34c to c2e3c96 Compare February 17, 2022 09:47

This comment has been minimized.

Sign in to view

timneutkens reviewed Feb 17, 2022

View reviewed changes

timneutkens requested changes Feb 17, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

sokra reviewed Feb 17, 2022

View reviewed changes

packages/next/build/webpack/plugins/middleware-source-maps-plugin.ts Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Schniz requested a review from timneutkens February 18, 2022 16:27

Schniz force-pushed the create-sourcemaps-for-middlewares branch from 27b7c80 to f4e5f1a Compare February 24, 2022 10:16

Schniz and others added 4 commits February 24, 2022 12:16

produce source maps for middlewares

11d1a3b

Try to enable source map work only for middleware layer

76ecdcb

runtimeModules are never in middleware layer, but maybe in middleware…

ea1dccd

… chunks > I would juest omit that and leave source maps on for them Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>

Make it under an experimental flag

dafe3ea

Schniz force-pushed the create-sourcemaps-for-middlewares branch from f4e5f1a to dafe3ea Compare February 24, 2022 10:16

This comment has been minimized.

Sign in to view

sokra requested changes Feb 24, 2022

View reviewed changes

packages/next/build/webpack-config.ts Outdated Show resolved Hide resolved

Add a test for the experimental flag

ca37797

Lift conditionals to the webpack config

54b8c05

This comment has been minimized.

Sign in to view

sokra previously approved these changes Feb 24, 2022

View reviewed changes

timneutkens approved these changes Feb 24, 2022

View reviewed changes

timneutkens reviewed Feb 24, 2022

View reviewed changes

Merge branch 'canary' into create-sourcemaps-for-middlewares

a1762ec

timneutkens requested changes Feb 24, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

Don't run on dev

a3910a8

Schniz dismissed sokra’s stale review via a3910a8 February 24, 2022 14:56

Schniz requested a review from timneutkens February 24, 2022 15:19

timneutkens approved these changes Feb 24, 2022

View reviewed changes

kodiakhq bot merged commit 1a0d149 into vercel:canary Feb 24, 2022

github-actions bot locked as resolved and limited conversation to collaborators Mar 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

produce source maps for middlewares #34409

produce source maps for middlewares #34409

Schniz commented Feb 16, 2022 •

edited by javivelasco

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Feb 16, 2022

This comment has been minimized.

This comment has been minimized.

timneutkens Feb 17, 2022

timneutkens Feb 17, 2022

sokra Feb 17, 2022 •

edited

timneutkens left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

timneutkens Feb 24, 2022

timneutkens Feb 24, 2022

timneutkens Feb 24, 2022

timneutkens left a comment

This comment has been minimized.

ijjk commented Feb 24, 2022

produce source maps for middlewares #34409

produce source maps for middlewares #34409

Conversation

Schniz commented Feb 16, 2022 • edited by javivelasco

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Feb 16, 2022

Failing test suites

This comment has been minimized.

This comment has been minimized.

timneutkens Feb 17, 2022

Choose a reason for hiding this comment

timneutkens Feb 17, 2022

Choose a reason for hiding this comment

sokra Feb 17, 2022 • edited

Choose a reason for hiding this comment

timneutkens left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

timneutkens Feb 24, 2022

Choose a reason for hiding this comment

timneutkens Feb 24, 2022

Choose a reason for hiding this comment

timneutkens Feb 24, 2022

Choose a reason for hiding this comment

timneutkens left a comment

Choose a reason for hiding this comment

This comment has been minimized.

ijjk commented Feb 24, 2022

Stats from current PR

Schniz commented Feb 16, 2022 •

edited by javivelasco

sokra Feb 17, 2022 •

edited