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

Simplify getMiddlewareInfo calls #33542

Merged
merged 2 commits into from
Jan 21, 2022
Merged

Simplify getMiddlewareInfo calls #33542

merged 2 commits into from
Jan 21, 2022

Conversation

huozhi
Copy link
Member

@huozhi huozhi commented Jan 21, 2022

Subtask of #31506

  • move serverBuildDir and getter function of publicDir to next-server as only used place
  • simplify getMiddlewareInfo, remove distDir and other params could be accessed from web server itself

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team type: next labels Jan 21, 2022
@ijjk

This comment has been minimized.

@ijjk
Copy link
Member

ijjk commented Jan 21, 2022

Failing test suites

Commit: 09c4949

test/integration/absolute-assetprefix/test/index.test.js

  • absolute assetPrefix with path prefix > should not fetch static data from a CDN
  • absolute assetPrefix with path prefix > should fetch from cache correctly
  • absolute assetPrefix with path prefix > should work with getStaticPaths prerendered
  • absolute assetPrefix with path prefix > should work with getStaticPaths fallback
  • absolute assetPrefix with path prefix > should work with getServerSideProps
Expand output

● absolute assetPrefix with path prefix › should not fetch static data from a CDN

page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:33801/
=========================== logs ===========================
navigating to "http://localhost:33801/", waiting until "load"
============================================================

  103 |         if (tracePlaywright) {
  104 |           if (!frame.payload.includes('pong')) {
> 105 |             page
      |                 ^
  106 |               .evaluate(`console.log('received ws message ${frame.payload}')`)
  107 |               .catch(() => {})
  108 |           }

  at Playwright.loadPage (lib/browsers/playwright.ts:105:20)
  at Object.webdriver [as default] (lib/next-webdriver.ts:59:5)
  at Object.<anonymous> (integration/absolute-assetprefix/test/index.test.js:84:25)

● absolute assetPrefix with path prefix › should fetch from cache correctly

page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:33801/
=========================== logs ===========================
navigating to "http://localhost:33801/", waiting until "load"
============================================================

  103 |         if (tracePlaywright) {
  104 |           if (!frame.payload.includes('pong')) {
> 105 |             page
      |                 ^
  106 |               .evaluate(`console.log('received ws message ${frame.payload}')`)
  107 |               .catch(() => {})
  108 |           }

  at Playwright.loadPage (lib/browsers/playwright.ts:105:20)
  at Object.webdriver [as default] (lib/next-webdriver.ts:59:5)
  at Object.<anonymous> (integration/absolute-assetprefix/test/index.test.js:91:25)

● absolute assetPrefix with path prefix › should work with getStaticPaths prerendered

page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:33801/
=========================== logs ===========================
navigating to "http://localhost:33801/", waiting until "load"
============================================================

  103 |         if (tracePlaywright) {
  104 |           if (!frame.payload.includes('pong')) {
> 105 |             page
      |                 ^
  106 |               .evaluate(`console.log('received ws message ${frame.payload}')`)
  107 |               .catch(() => {})
  108 |           }

  at Playwright.loadPage (lib/browsers/playwright.ts:105:20)
  at Object.webdriver [as default] (lib/next-webdriver.ts:59:5)
  at Object.<anonymous> (integration/absolute-assetprefix/test/index.test.js:104:25)

● absolute assetPrefix with path prefix › should work with getStaticPaths fallback

page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:33801/
=========================== logs ===========================
navigating to "http://localhost:33801/", waiting until "load"
============================================================

  103 |         if (tracePlaywright) {
  104 |           if (!frame.payload.includes('pong')) {
> 105 |             page
      |                 ^
  106 |               .evaluate(`console.log('received ws message ${frame.payload}')`)
  107 |               .catch(() => {})
  108 |           }

  at Playwright.loadPage (lib/browsers/playwright.ts:105:20)
  at Object.webdriver [as default] (lib/next-webdriver.ts:59:5)
  at Object.<anonymous> (integration/absolute-assetprefix/test/index.test.js:111:25)

● absolute assetPrefix with path prefix › should work with getServerSideProps

page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:33801/
=========================== logs ===========================
navigating to "http://localhost:33801/", waiting until "load"
============================================================

  103 |         if (tracePlaywright) {
  104 |           if (!frame.payload.includes('pong')) {
> 105 |             page
      |                 ^
  106 |               .evaluate(`console.log('received ws message ${frame.payload}')`)
  107 |               .catch(() => {})
  108 |           }

  at Playwright.loadPage (lib/browsers/playwright.ts:105:20)
  at Object.webdriver [as default] (lib/next-webdriver.ts:59:5)
  at Object.<anonymous> (integration/absolute-assetprefix/test/index.test.js:118:25)

test/integration/amphtml-custom-validator/test/index.test.js

  • AMP Custom Validator > should build and start successfully
Expand output

● AMP Custom Validator › should build and start successfully

FetchError: request to http://localhost:39077/ failed, reason: read ECONNRESET

  at ClientRequest.<anonymous> (../node_modules/node-fetch/lib/index.js:1491:11)

test/integration/amphtml-fragment-style/test/index.test.js

  • AMP Fragment Styles > adds styles from fragment in AMP mode correctly
Expand output

● AMP Fragment Styles › adds styles from fragment in AMP mode correctly

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined

  16 |
  17 | import fs from 'fs'
> 18 | import { join, relative, resolve, sep } from 'path'
     |                                                    ^
  19 | import { IncomingMessage, ServerResponse } from 'http'
  20 |
  21 | import {

  at NextNodeServer.getPagesManifest (../packages/next/server/next-server.ts:18:52)
  at new Server (../packages/next/server/base-server.ts:348:31)
  at new NextNodeServer (../packages/next/server/next-server.ts:96:5)
  at NextServer.createServer (../packages/next/server/next.ts:124:12)
  at ../packages/next/server/next.ts:140:23

● Test suite failed to run

TypeError: Cannot read property '__app' of undefined

  409 |   return new Promise((resolve, reject) => {
  410 |     const newArgs = [
> 411 |       ...args,
      |               ^
  412 |       function (err, res) {
  413 |         if (err) return reject(err)
  414 |         resolve(res)

  at Object.stopApp (lib/next-test-utils.js:411:16)
  at integration/amphtml-fragment-style/test/index.test.js:26:38

test/integration/app-document-style-fragment/test/index.test.js

  • Custom Document Fragment Styles > correctly adds styles from fragment styles key
Expand output

● Custom Document Fragment Styles › correctly adds styles from fragment styles key

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined

  16 |
  17 | import fs from 'fs'
> 18 | import { join, relative, resolve, sep } from 'path'
     |                                                    ^
  19 | import { IncomingMessage, ServerResponse } from 'http'
  20 |
  21 | import {

  at NextNodeServer.getPagesManifest (../packages/next/server/next-server.ts:18:52)
  at new Server (../packages/next/server/base-server.ts:348:31)
  at new NextNodeServer (../packages/next/server/next-server.ts:96:5)
  at NextServer.createServer (../packages/next/server/next.ts:124:12)
  at ../packages/next/server/next.ts:140:23

● Test suite failed to run

TypeError: Cannot read property '__app' of undefined

  409 |   return new Promise((resolve, reject) => {
  410 |     const newArgs = [
> 411 |       ...args,
      |               ^
  412 |       function (err, res) {
  413 |         if (err) return reject(err)
  414 |         resolve(res)

  at Object.stopApp (lib/next-test-utils.js:411:16)
  at integration/app-document-style-fragment/test/index.test.js:25:38

test/production/fallback-export-error/index.test.ts

  • fallback export error > should have built
Expand output

● fallback export error › should have built

FetchError: request to http://localhost:44771/ failed, reason: socket hang up

  at ClientRequest.<anonymous> (../node_modules/node-fetch/lib/index.js:1491:11)

test/integration/basepath-root-catch-all/test/index.test.js

  • production mode > should use correct data URL for root catch-all
  • serverless mode > should use correct data URL for root catch-all
Expand output

● production mode › should use correct data URL for root catch-all

page.goto: net::ERR_CONNECTION_RESET at http://localhost:32929/docs/hello
=========================== logs ===========================
navigating to "http://localhost:32929/docs/hello", waiting until "load"
============================================================

  103 |         if (tracePlaywright) {
  104 |           if (!frame.payload.includes('pong')) {
> 105 |             page
      |                 ^
  106 |               .evaluate(`console.log('received ws message ${frame.payload}')`)
  107 |               .catch(() => {})
  108 |           }

  at Playwright.loadPage (lib/browsers/playwright.ts:105:20)
  at Object.webdriver [as default] (lib/next-webdriver.ts:59:5)
  at Object.<anonymous> (integration/basepath-root-catch-all/test/index.test.js:19:25)

● serverless mode › should use correct data URL for root catch-all

page.goto: net::ERR_CONNECTION_RESET at http://localhost:46181/docs/hello
=========================== logs ===========================
navigating to "http://localhost:46181/docs/hello", waiting until "load"
============================================================

  103 |         if (tracePlaywright) {
  104 |           if (!frame.payload.includes('pong')) {
> 105 |             page
      |                 ^
  106 |               .evaluate(`console.log('received ws message ${frame.payload}')`)
  107 |               .catch(() => {})
  108 |           }

  at Playwright.loadPage (lib/browsers/playwright.ts:105:20)
  at Object.webdriver [as default] (lib/next-webdriver.ts:59:5)
  at Object.<anonymous> (integration/basepath-root-catch-all/test/index.test.js:19:25)

test/integration/404-page-app/test/index.test.js

  • 404 Page Support with _app > production mode > should not output static 404 if _app has getInitialProps
  • 404 Page Support with _app > production mode > should still use 404 page
Expand output

● 404 Page Support with _app › production mode › should not output static 404 if _app has getInitialProps

page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:38113/404
=========================== logs ===========================
navigating to "http://localhost:38113/404", waiting until "load"
============================================================

  103 |         if (tracePlaywright) {
  104 |           if (!frame.payload.includes('pong')) {
> 105 |             page
      |                 ^
  106 |               .evaluate(`console.log('received ws message ${frame.payload}')`)
  107 |               .catch(() => {})
  108 |           }

  at Playwright.loadPage (lib/browsers/playwright.ts:105:20)
  at Object.webdriver [as default] (lib/next-webdriver.ts:59:5)
  at Object.<anonymous> (integration/404-page-app/test/index.test.js:32:29)

● 404 Page Support with _app › production mode › should still use 404 page

FetchError: request to http://localhost:38113/abc failed, reason: connect ECONNREFUSED 127.0.0.1:38113

  at ClientRequest.<anonymous> (../node_modules/node-fetch/lib/index.js:1491:11)

@ijjk
Copy link
Member

ijjk commented Jan 21, 2022

Stats from current PR

Default Build (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary huozhi/next.js bs/simplify Change
buildDuration 12.6s 12.4s -122ms
buildDurationCached 3.2s 3.2s ⚠️ +1ms
nodeModulesSize 355 MB 355 MB -396 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary huozhi/next.js bs/simplify Change
/ failed reqs 0 0
/ total time (seconds) 2.959 2.964 0
/ avg req/sec 844.77 843.34 ⚠️ -1.43
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.381 1.387 ⚠️ +0.01
/error-in-render avg req/sec 1810.85 1801.86 ⚠️ -8.99
Client Bundles (main, webpack, commons)
vercel/next.js canary huozhi/next.js bs/simplify Change
450.HASH.js gzip 179 B 179 B
framework-HASH.js gzip 42.2 kB 42.2 kB
main-HASH.js gzip 27.2 kB 27.2 kB
webpack-HASH.js gzip 1.44 kB 1.44 kB
Overall change 71 kB 71 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary huozhi/next.js bs/simplify Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary huozhi/next.js bs/simplify Change
_app-HASH.js gzip 1.37 kB 1.37 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.37 kB 2.37 kB
head-HASH.js gzip 350 B 350 B
hooks-HASH.js gzip 919 B 919 B
image-HASH.js gzip 4.87 kB 4.87 kB
index-HASH.js gzip 263 B 263 B
link-HASH.js gzip 2.13 kB 2.13 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.2 kB 14.2 kB
Client Build Manifests
vercel/next.js canary huozhi/next.js bs/simplify Change
_buildManifest.js gzip 459 B 459 B
Overall change 459 B 459 B
Rendered Page Sizes
vercel/next.js canary huozhi/next.js bs/simplify Change
index.html gzip 531 B 531 B
link.html gzip 546 B 546 B
withRouter.html gzip 527 B 527 B
Overall change 1.6 kB 1.6 kB

Default Build with SWC (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary huozhi/next.js bs/simplify Change
buildDuration 15.7s 16s ⚠️ +295ms
buildDurationCached 3.2s 3.2s ⚠️ +26ms
nodeModulesSize 355 MB 355 MB -396 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary huozhi/next.js bs/simplify Change
/ failed reqs 0 0
/ total time (seconds) 3.011 2.998 -0.01
/ avg req/sec 830.26 833.86 +3.6
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.341 1.361 ⚠️ +0.02
/error-in-render avg req/sec 1864.06 1836.45 ⚠️ -27.61
Client Bundles (main, webpack, commons)
vercel/next.js canary huozhi/next.js bs/simplify Change
450.HASH.js gzip 179 B 179 B
framework-HASH.js gzip 42.3 kB 42.3 kB
main-HASH.js gzip 27.3 kB 27.3 kB
webpack-HASH.js gzip 1.44 kB 1.44 kB
Overall change 71.3 kB 71.3 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary huozhi/next.js bs/simplify Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary huozhi/next.js bs/simplify 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.36 kB 2.36 kB
head-HASH.js gzip 342 B 342 B
hooks-HASH.js gzip 911 B 911 B
image-HASH.js gzip 4.89 kB 4.89 kB
index-HASH.js gzip 256 B 256 B
link-HASH.js gzip 2.19 kB 2.19 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.2 kB 14.2 kB
Client Build Manifests
vercel/next.js canary huozhi/next.js bs/simplify Change
_buildManifest.js gzip 459 B 459 B
Overall change 459 B 459 B
Rendered Page Sizes
vercel/next.js canary huozhi/next.js bs/simplify Change
index.html gzip 530 B 530 B
link.html gzip 544 B 544 B
withRouter.html gzip 525 B 525 B
Overall change 1.6 kB 1.6 kB
Commit: 401c12e

@huozhi huozhi marked this pull request as ready for review January 21, 2022 16:15
@huozhi
Copy link
Member Author

huozhi commented Jan 21, 2022

Seems JJ bot outdated comment is not hidden? @ijjk

@kodiakhq kodiakhq bot merged commit 557a972 into vercel:canary Jan 21, 2022
@huozhi huozhi deleted the bs/simplify branch January 21, 2022 16:28
natew pushed a commit to natew/next.js that referenced this pull request Feb 16, 2022
Subtask of vercel#31506

* move `serverBuildDir` and getter function of `publicDir` to next-server as only used place
* simplify `getMiddlewareInfo`, remove `distDir` and other params could be accessed from web server itself
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants