Skip to content

Removng prop writable checking in prod#31929

Merged
kodiakhq[bot] merged 4 commits intovercel:canaryfrom
huozhi:revert-writable-checking
Nov 30, 2021
Merged

Removng prop writable checking in prod#31929
kodiakhq[bot] merged 4 commits intovercel:canaryfrom
huozhi:revert-writable-checking

Conversation

@huozhi
Copy link
Copy Markdown
Contributor

@huozhi huozhi commented Nov 29, 2021 •
edited
Loading

x-ref: #28498, #31784

I can repro the issue #24783 with next-boost 0.10.1 and it was fixed in 0.10.2 (ref: next-boost/next-boost@eba6d10). The actual case is missing setting node env to "production".

React freeze element props and element itself in dev mode (ref: https://github.com/facebook/react/blob/a724a3b578dce77d427bef313102a4d0e978d9b4/packages/react/src/ReactElement.js#L194-L196)

Then next.js will reassign props with react development bundle while next-boost is enabled. Those operations are only happened in non-dev mode so it's good to remove now.

This PR + #31898 == revert #31784

cc @styfle @awareness481

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Nov 29, 2021
@huozhi huozhi mentioned this pull request Nov 29, 2021
Merged
@ijjk

This comment has been minimized.

Sign in to view
@huozhi huozhi changed the title revert prop writable checking in prod Revert prop writable checking in prod Nov 29, 2021
@huozhi huozhi force-pushed the revert-writable-checking branch from 1e2971c to 165718e Compare November 29, 2021 16:45
@ijjk

This comment has been minimized.

Sign in to view
@huozhi
Copy link
Copy Markdown
Contributor Author

huozhi commented Nov 29, 2021

🤔 not sure why this is happening..

Run git describe
fatal: No tags can describe 'c45be2370b43a92576a4328cc614d58a6dd08cfd'.
Try --always, or create some tags.
Error: Process completed with exit code 128.

@awareness481
Copy link
Copy Markdown
Contributor

awareness481 commented Nov 29, 2021

I checked just in case, no error 🎉 That's awesome work @huozhi! I wasn't able to find out what exactly was going on when I had initially made the PR

@huozhi huozhi changed the title Revert prop writable checking in prod Removng prop writable checking in prod Nov 29, 2021
@ijjk

This comment has been minimized.

Sign in to view
@ijjk

This comment has been minimized.

Sign in to view
@ijjk
Copy link
Copy Markdown
Member

ijjk commented Nov 30, 2021

Stats from current PR

Default Build (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary huozhi/next.js revert-writable-checking Change
buildDuration 18.3s 18.4s ⚠️ +122ms
buildDurationCached 3.4s 3.4s -37ms
nodeModulesSize 346 MB 346 MB -335 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary huozhi/next.js revert-writable-checking Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.896 2.938 ⚠️ +0.04
/ avg req/sec 863.2 850.84 ⚠️ -12.36
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.354 1.396 ⚠️ +0.04
/error-in-render avg req/sec 1846.21 1791.45 ⚠️ -54.76
Client Bundles (main, webpack, commons)
vercel/next.js canary huozhi/next.js revert-writable-checking Change
450.HASH.js gzip 179 B 179 B ✓
framework-HASH.js gzip 42.2 kB 42.2 kB ✓
main-HASH.js gzip 28.4 kB 28.4 kB ✓
webpack-HASH.js gzip 1.45 kB 1.45 kB ✓
Overall change 72.2 kB 72.2 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary huozhi/next.js revert-writable-checking Change
polyfills-HASH.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary huozhi/next.js revert-writable-checking 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.38 kB 2.38 kB ✓
head-HASH.js gzip 350 B 350 B ✓
hooks-HASH.js gzip 635 B 635 B ✓
image-HASH.js gzip 4.45 kB 4.45 kB ✓
index-HASH.js gzip 263 B 263 B ✓
link-HASH.js gzip 1.87 kB 1.87 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 13.3 kB 13.3 kB ✓
Client Build Manifests
vercel/next.js canary huozhi/next.js revert-writable-checking Change
_buildManifest.js gzip 460 B 460 B ✓
Overall change 460 B 460 B ✓
Rendered Page Sizes
vercel/next.js canary huozhi/next.js revert-writable-checking Change
index.html gzip 532 B 532 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 (Increase detected ⚠️)
General Overall decrease ✓
vercel/next.js canary huozhi/next.js revert-writable-checking Change
buildDuration 19.9s 19.8s -4ms
buildDurationCached 3.4s 3.3s -38ms
nodeModulesSize 346 MB 346 MB -335 B
Page Load Tests Overall increase ✓
vercel/next.js canary huozhi/next.js revert-writable-checking Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.903 3.005 ⚠️ +0.1
/ avg req/sec 861.07 831.96 ⚠️ -29.11
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.373 1.324 -0.05
/error-in-render avg req/sec 1820.73 1887.81 +67.08
Client Bundles (main, webpack, commons)
vercel/next.js canary huozhi/next.js revert-writable-checking Change
450.HASH.js gzip 179 B 179 B ✓
framework-HASH.js gzip 42.3 kB 42.3 kB ✓
main-HASH.js gzip 28.6 kB 28.6 kB ✓
webpack-HASH.js gzip 1.44 kB 1.44 kB ✓
Overall change 72.5 kB 72.5 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary huozhi/next.js revert-writable-checking Change
polyfills-HASH.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary huozhi/next.js revert-writable-checking 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.37 kB 2.37 kB ✓
head-HASH.js gzip 342 B 342 B ✓
hooks-HASH.js gzip 622 B 622 B ✓
image-HASH.js gzip 4.47 kB 4.47 kB ✓
index-HASH.js gzip 256 B 256 B ✓
link-HASH.js gzip 1.91 kB 1.91 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 13.2 kB 13.2 kB ✓
Client Build Manifests
vercel/next.js canary huozhi/next.js revert-writable-checking Change
_buildManifest.js gzip 459 B 459 B ✓
Overall change 459 B 459 B ✓
Rendered Page Sizes
vercel/next.js canary huozhi/next.js revert-writable-checking Change
index.html gzip 532 B 532 B ✓
link.html gzip 545 B 545 B ✓
withRouter.html gzip 525 B 525 B ✓
Overall change 1.6 kB 1.6 kB ✓
Commit: e0d210f

@kodiakhq kodiakhq Bot merged commit c789301 into vercel:canary Nov 30, 2021
@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
@huozhi huozhi deleted the revert-writable-checking branch February 3, 2022 12:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ijjk ijjk ijjk approved these changes

@shuding shuding Awaiting requested review from shuding

@timneutkens timneutkens Awaiting requested review from timneutkens

Assignees

No one assigned

Labels

created-by: Next.js team PRs by the Next.js team. type: next

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@huozhi @ijjk @awareness481