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.

Sign up for GitHub

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

Jump to bottom

Refactor <Document> files #16184

Merged
merged 4 commits into from
Aug 14, 2020
Merged

Refactor <Document> files #16184

merged 4 commits into from
Aug 14, 2020

Conversation

Timer
Copy link
Member

@Timer Timer commented Aug 14, 2020

Instead of reading the BuildManifest and passing it to /_document, it should be able to read it for itself.

Fixes #16182

@ijjk

This comment has been minimized.

Sign in to view
@ijjk

This comment has been minimized.

Sign in to view
@ijjk

This comment has been minimized.

Sign in to view
@Timer Timer marked this pull request as ready for review August 14, 2020 05:14
@Timer
Copy link
Member Author

Timer commented Aug 14, 2020

Re-marking as draft to investigate this:
image

@Timer Timer marked this pull request as draft August 14, 2020 05:16
@ijjk
Copy link
Member

ijjk commented Aug 14, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary Timer/next.js refactor/document-files Change
buildDuration 11.6s 11.9s ⚠️ +230ms
nodeModulesSize 57.3 MB 57.3 MB ⚠️ +1.16 kB
Page Load Tests Overall increase ✓
vercel/next.js canary Timer/next.js refactor/document-files Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.277 2.102 -0.18
/ avg req/sec 1097.75 1189.51 +91.76
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.386 1.4 ⚠️ +0.01
/error-in-render avg req/sec 1803.37 1786.24 ⚠️ -17.13
Client Bundles (main, webpack, commons)
vercel/next.js canary Timer/next.js refactor/document-files Change
677f882d2ed8..c139.js gzip 10.2 kB 10.2 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
main-d9f8cd0..c4c7.js gzip 6.74 kB 6.74 kB ✓
webpack-ccf5..276a.js gzip 751 B 751 B ✓
Overall change 56.9 kB 56.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary Timer/next.js refactor/document-files Change
677f882d2ed8..dule.js gzip 6.12 kB 6.12 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
main-b9a17ec..dule.js gzip 5.82 kB 5.82 kB ✓
webpack-10c7..dule.js gzip 751 B 751 B ✓
Overall change 51.8 kB 51.8 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary Timer/next.js refactor/document-files Change
polyfills-75..1629.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary Timer/next.js refactor/document-files Change
_app-874bd8a..0103.js gzip 1.28 kB 1.28 kB ✓
_error-fa39c..ec40.js gzip 3.45 kB 3.45 kB ✓
hooks-585f07..95a3.js gzip 887 B 887 B ✓
index-c7b63f..fc02.js gzip 227 B 227 B ✓
link-4c2bd9b..eadd.js gzip 1.29 kB 1.29 kB ✓
routerDirect..ebc7.js gzip 284 B 284 B ✓
withRouter-2..db68.js gzip 284 B 284 B ✓
Overall change 7.71 kB 7.71 kB ✓
Client Pages Modern
vercel/next.js canary Timer/next.js refactor/document-files Change
_app-97e743e..dule.js gzip 626 B 626 B ✓
_error-b4004..dule.js gzip 2.3 kB 2.3 kB ✓
hooks-696209..dule.js gzip 387 B 387 B ✓
index-a4dd74..dule.js gzip 226 B 226 B ✓
link-236a801..dule.js gzip 1.26 kB 1.26 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-1..dule.js gzip 282 B 282 B ✓
Overall change 5.37 kB 5.37 kB ✓
Client Build Manifests
vercel/next.js canary Timer/next.js refactor/document-files Change
_buildManifest.js gzip 321 B 321 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 650 B 650 B ✓
Rendered Page Sizes
vercel/next.js canary Timer/next.js refactor/document-files Change
index.html gzip 946 B 946 B ✓
link.html gzip 954 B 954 B ✓
withRouter.html gzip 939 B 939 B ✓
Overall change 2.84 kB 2.84 kB ✓
Serverless Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary Timer/next.js refactor/document-files Change
buildDuration 13.4s 12.9s -431ms
nodeModulesSize 57.3 MB 57.3 MB ⚠️ +1.16 kB
Client Bundles (main, webpack, commons)
vercel/next.js canary Timer/next.js refactor/document-files Change
677f882d2ed8..c139.js gzip 10.2 kB 10.2 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
main-d9f8cd0..c4c7.js gzip 6.74 kB 6.74 kB ✓
webpack-ccf5..276a.js gzip 751 B 751 B ✓
Overall change 56.9 kB 56.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary Timer/next.js refactor/document-files Change
677f882d2ed8..dule.js gzip 6.12 kB 6.12 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
main-b9a17ec..dule.js gzip 5.82 kB 5.82 kB ✓
webpack-10c7..dule.js gzip 751 B 751 B ✓
Overall change 51.8 kB 51.8 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary Timer/next.js refactor/document-files Change
polyfills-75..1629.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary Timer/next.js refactor/document-files Change
_app-874bd8a..0103.js gzip 1.28 kB 1.28 kB ✓
_error-fa39c..ec40.js gzip 3.45 kB 3.45 kB ✓
hooks-585f07..95a3.js gzip 887 B 887 B ✓
index-c7b63f..fc02.js gzip 227 B 227 B ✓
link-4c2bd9b..eadd.js gzip 1.29 kB 1.29 kB ✓
routerDirect..ebc7.js gzip 284 B 284 B ✓
withRouter-2..db68.js gzip 284 B 284 B ✓
Overall change 7.71 kB 7.71 kB ✓
Client Pages Modern
vercel/next.js canary Timer/next.js refactor/document-files Change
_app-97e743e..dule.js gzip 626 B 626 B ✓
_error-b4004..dule.js gzip 2.3 kB 2.3 kB ✓
hooks-696209..dule.js gzip 387 B 387 B ✓
index-a4dd74..dule.js gzip 226 B 226 B ✓
link-236a801..dule.js gzip 1.26 kB 1.26 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-1..dule.js gzip 282 B 282 B ✓
Overall change 5.37 kB 5.37 kB ✓
Client Build Manifests
vercel/next.js canary Timer/next.js refactor/document-files Change
_buildManifest.js gzip 321 B 321 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 650 B 650 B ✓
Serverless bundles Overall increase ⚠️
vercel/next.js canary Timer/next.js refactor/document-files Change
_error.js 1.03 MB 1.03 MB ⚠️ +184 B
404.html 4.18 kB 4.18 kB ✓
hooks.html 3.82 kB 3.82 kB ✓
index.js 1.03 MB 1.03 MB ⚠️ +184 B
link.js 1.07 MB 1.07 MB ⚠️ +184 B
routerDirect.js 1.06 MB 1.06 MB ⚠️ +184 B
withRouter.js 1.06 MB 1.06 MB ⚠️ +184 B
Overall change 5.26 MB 5.26 MB ⚠️ +920 B
Commit: 386de24

@Timer
Copy link
Member Author

Timer commented Aug 14, 2020

It was due to left over console.log!

@Timer Timer marked this pull request as ready for review August 14, 2020 06:20
@Timer
Copy link
Member Author

Timer commented Aug 14, 2020

image

@ijjk
Copy link
Member

ijjk commented Aug 14, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary Timer/next.js refactor/document-files Change
buildDuration 12.7s 12.9s ⚠️ +227ms
nodeModulesSize 57.3 MB 57.3 MB ⚠️ +1.16 kB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary Timer/next.js refactor/document-files Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.224 2.216 -0.01
/ avg req/sec 1123.99 1128.29 +4.3
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.236 1.261 ⚠️ +0.02
/error-in-render avg req/sec 2022.12 1982.99 ⚠️ -39.13
Client Bundles (main, webpack, commons)
vercel/next.js canary Timer/next.js refactor/document-files Change
677f882d2ed8..c139.js gzip 10.2 kB 10.2 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
main-d9f8cd0..c4c7.js gzip 6.74 kB 6.74 kB ✓
webpack-ccf5..276a.js gzip 751 B 751 B ✓
Overall change 56.9 kB 56.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary Timer/next.js refactor/document-files Change
677f882d2ed8..dule.js gzip 6.12 kB 6.12 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
main-b9a17ec..dule.js gzip 5.82 kB 5.82 kB ✓
webpack-10c7..dule.js gzip 751 B 751 B ✓
Overall change 51.8 kB 51.8 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary Timer/next.js refactor/document-files Change
polyfills-75..1629.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary Timer/next.js refactor/document-files Change
_app-874bd8a..0103.js gzip 1.28 kB 1.28 kB ✓
_error-fa39c..ec40.js gzip 3.45 kB 3.45 kB ✓
hooks-585f07..95a3.js gzip 887 B 887 B ✓
index-c7b63f..fc02.js gzip 227 B 227 B ✓
link-4c2bd9b..eadd.js gzip 1.29 kB 1.29 kB ✓
routerDirect..ebc7.js gzip 284 B 284 B ✓
withRouter-2..db68.js gzip 284 B 284 B ✓
Overall change 7.71 kB 7.71 kB ✓
Client Pages Modern
vercel/next.js canary Timer/next.js refactor/document-files Change
_app-97e743e..dule.js gzip 626 B 626 B ✓
_error-b4004..dule.js gzip 2.3 kB 2.3 kB ✓
hooks-696209..dule.js gzip 387 B 387 B ✓
index-a4dd74..dule.js gzip 226 B 226 B ✓
link-236a801..dule.js gzip 1.26 kB 1.26 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-1..dule.js gzip 282 B 282 B ✓
Overall change 5.37 kB 5.37 kB ✓
Client Build Manifests
vercel/next.js canary Timer/next.js refactor/document-files Change
_buildManifest.js gzip 321 B 321 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 650 B 650 B ✓
Rendered Page Sizes
vercel/next.js canary Timer/next.js refactor/document-files Change
index.html gzip 946 B 946 B ✓
link.html gzip 954 B 954 B ✓
withRouter.html gzip 939 B 939 B ✓
Overall change 2.84 kB 2.84 kB ✓
Serverless Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary Timer/next.js refactor/document-files Change
buildDuration 14.3s 14.4s ⚠️ +140ms
nodeModulesSize 57.3 MB 57.3 MB ⚠️ +1.16 kB
Client Bundles (main, webpack, commons)
vercel/next.js canary Timer/next.js refactor/document-files Change
677f882d2ed8..c139.js gzip 10.2 kB 10.2 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
main-d9f8cd0..c4c7.js gzip 6.74 kB 6.74 kB ✓
webpack-ccf5..276a.js gzip 751 B 751 B ✓
Overall change 56.9 kB 56.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary Timer/next.js refactor/document-files Change
677f882d2ed8..dule.js gzip 6.12 kB 6.12 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
main-b9a17ec..dule.js gzip 5.82 kB 5.82 kB ✓
webpack-10c7..dule.js gzip 751 B 751 B ✓
Overall change 51.8 kB 51.8 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary Timer/next.js refactor/document-files Change
polyfills-75..1629.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary Timer/next.js refactor/document-files Change
_app-874bd8a..0103.js gzip 1.28 kB 1.28 kB ✓
_error-fa39c..ec40.js gzip 3.45 kB 3.45 kB ✓
hooks-585f07..95a3.js gzip 887 B 887 B ✓
index-c7b63f..fc02.js gzip 227 B 227 B ✓
link-4c2bd9b..eadd.js gzip 1.29 kB 1.29 kB ✓
routerDirect..ebc7.js gzip 284 B 284 B ✓
withRouter-2..db68.js gzip 284 B 284 B ✓
Overall change 7.71 kB 7.71 kB ✓
Client Pages Modern
vercel/next.js canary Timer/next.js refactor/document-files Change
_app-97e743e..dule.js gzip 626 B 626 B ✓
_error-b4004..dule.js gzip 2.3 kB 2.3 kB ✓
hooks-696209..dule.js gzip 387 B 387 B ✓
index-a4dd74..dule.js gzip 226 B 226 B ✓
link-236a801..dule.js gzip 1.26 kB 1.26 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-1..dule.js gzip 282 B 282 B ✓
Overall change 5.37 kB 5.37 kB ✓
Client Build Manifests
vercel/next.js canary Timer/next.js refactor/document-files Change
_buildManifest.js gzip 321 B 321 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 650 B 650 B ✓
Serverless bundles Overall increase ⚠️
vercel/next.js canary Timer/next.js refactor/document-files Change
_error.js 1.03 MB 1.03 MB ⚠️ +184 B
404.html 4.18 kB 4.18 kB ✓
hooks.html 3.82 kB 3.82 kB ✓
index.js 1.03 MB 1.03 MB ⚠️ +184 B
link.js 1.07 MB 1.07 MB ⚠️ +184 B
routerDirect.js 1.06 MB 1.06 MB ⚠️ +184 B
withRouter.js 1.06 MB 1.06 MB ⚠️ +184 B
Overall change 5.26 MB 5.26 MB ⚠️ +920 B
Commit: bcc71d2

@kodiakhq kodiakhq bot merged commit 9fcf39a into vercel:canary Aug 14, 2020
@Timer Timer deleted the refactor/document-files branch August 14, 2020 15:13
m-lautenbach pushed a commit to m-lautenbach/next.js that referenced this pull request Aug 20, 2020
@Timer
Refactor <Document> files (vercel#16184)
32d88ab 
Instead of reading the `BuildManifest` and passing it to `/_document`, it should be able to read it for itself.

---

Fixes vercel#16182
@svenheden svenheden mentioned this pull request Sep 4, 2020
Closed
@ka2jun8 ka2jun8 mentioned this pull request Sep 8, 2020
Closed
@msluther msluther mentioned this pull request Oct 8, 2020
Closed
janicklas-ralph
janicklas-ralph reviewed Dec 4, 2020
View reviewed changes
packages/next/pages/_document.tsx
const { assetPrefix, files, devOnlyCacheBusterQueryString } = this.context
const cssFiles =
files && files.length ? files.filter((f) => f.endsWith('.css')) : []
getCssLinks(files: DocumentFiles): JSX.Element[] | null {
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes it harder for users to extend getCssLinks function. When they want to insert custom css links and control the order of the resources.

Same for the getPreloadMainLinks too

Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be touched by users, it's framework internals

@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@janicklas-ralph janicklas-ralph janicklas-ralph left review comments

@timneutkens timneutkens timneutkens approved these changes

@ijjk ijjk Awaiting requested review from ijjk ijjk is a code owner

@lfades lfades Awaiting requested review from lfades

Assignees
No one assigned
Labels
type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[internal] Refactor /_document to rely on BuildManifest
4 participants
@Timer @ijjk @janicklas-ralph @timneutkens