Don't require source files to be writeable in dev mode #30758
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
evan-bradley
requested review from
huozhi,
ijjk,
padmaia,
shuding,
styfle and
timneutkens
as code owners
timneutkens
previously requested changes
Nov 2, 2021
evan-bradley
force-pushed
the
fix-hot-reloader-permissions-check
branch
2 times, most recently
from
0b70497
to
96ea294
Compare
Currently, hot reloading is broken when running dev mode in systems like Bazel, where the source files are marked as read-only. The dev server will not render pages, and the browser's request will hang when trying to load a page. This is caused by a check in the hot reloader to see if a file exists, which is currently done by seeing if the file is writeable. The goal of the check is merely to see if a file exists, so checking for read permissions should be sufficient.
evan-bradley
force-pushed
the
fix-hot-reloader-permissions-check
branch
from
96ea294
to
9f0beec
Compare
ijjk
approved these changes
Nov 2, 2021
Stats from current PRDefault Build (Increase detected
|
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| buildDuration | 22.1s | 22.1s | |
| buildDurationCached | 4.6s | 4.6s | |
| nodeModulesSize | 293 MB | 293 MB | -5 B |
Page Load Tests Overall increase ✓
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| / failed reqs | 0 | 0 | ✓ |
| / total time (seconds) | 3.47 | 3.644 | |
| / avg req/sec | 720.47 | 686.11 | |
| /error-in-render failed reqs | 0 | 0 | ✓ |
| /error-in-render total time (seconds) | 1.752 | 1.638 | -0.11 |
| /error-in-render avg req/sec | 1427.26 | 1526.41 | +99.15 |
Client Bundles (main, webpack, commons)
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| 450.HASH.js gzip | 179 B | 179 B | ✓ |
| framework-HASH.js gzip | 42.2 kB | 42.2 kB | ✓ |
| main-HASH.js gzip | 28 kB | 28 kB | ✓ |
| webpack-HASH.js gzip | 1.45 kB | 1.45 kB | ✓ |
| Overall change | 71.9 kB | 71.9 kB | ✓ |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
| Overall change | 31 kB | 31 kB | ✓ |
Client Pages
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| _app-HASH.js gzip | 1.23 kB | 1.23 kB | ✓ |
| _error-HASH.js gzip | 194 B | 194 B | ✓ |
| amp-HASH.js gzip | 312 B | 312 B | ✓ |
| css-HASH.js gzip | 327 B | 327 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.44 kB | 4.44 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 | ✓ |
| 334f979574ae..6f4.css gzip | 106 B | 106 B | ✓ |
| Overall change | 13.1 kB | 13.1 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| _buildManifest.js gzip | 459 B | 459 B | ✓ |
| Overall change | 459 B | 459 B | ✓ |
Rendered Page Sizes
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| index.html gzip | 521 B | 521 B | ✓ |
| link.html gzip | 533 B | 533 B | ✓ |
| withRouter.html gzip | 515 B | 515 B | ✓ |
| Overall change | 1.57 kB | 1.57 kB | ✓ |
Default Build with SWC (Increase detected ⚠️ )
General Overall decrease ✓
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| buildDuration | 23.4s | 22.8s | -528ms |
| buildDurationCached | 4.7s | 4.6s | -86ms |
| nodeModulesSize | 293 MB | 293 MB | -5 B |
Page Load Tests Overall increase ✓
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| / failed reqs | 0 | 0 | ✓ |
| / total time (seconds) | 3.423 | 3.396 | -0.03 |
| / avg req/sec | 730.41 | 736.2 | +5.79 |
| /error-in-render failed reqs | 0 | 0 | ✓ |
| /error-in-render total time (seconds) | 1.726 | 1.618 | -0.11 |
| /error-in-render avg req/sec | 1448.86 | 1545.43 | +96.57 |
Client Bundles (main, webpack, commons)
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| 450.HASH.js gzip | 179 B | 179 B | ✓ |
| framework-HASH.js gzip | 42.3 kB | 42.3 kB | ✓ |
| main-HASH.js gzip | 28.2 kB | 28.2 kB | ✓ |
| webpack-HASH.js gzip | 1.43 kB | 1.43 kB | ✓ |
| Overall change | 72.1 kB | 72.1 kB | ✓ |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
| Overall change | 31 kB | 31 kB | ✓ |
Client Pages
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| _app-HASH.js gzip | 1.22 kB | 1.22 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.38 kB | 2.38 kB | ✓ |
| head-HASH.js gzip | 342 B | 342 B | ✓ |
| hooks-HASH.js gzip | 622 B | 622 B | ✓ |
| image-HASH.js gzip | 4.46 kB | 4.46 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 | ✓ |
| 334f979574ae..6f4.css gzip | 106 B | 106 B | ✓ |
| Overall change | 13.1 kB | 13.1 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| _buildManifest.js gzip | 460 B | 460 B | ✓ |
| Overall change | 460 B | 460 B | ✓ |
Rendered Page Sizes
| vercel/next.js canary | evan-bradley/next.js fix-hot-reloader-permissions-check | Change | |
|---|---|---|---|
| index.html gzip | 521 B | 521 B | ✓ |
| link.html gzip | 534 B | 534 B | ✓ |
| withRouter.html gzip | 516 B | 516 B | ✓ |
| Overall change | 1.57 kB | 1.57 kB | ✓ |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, hot reloading is broken when running dev mode in systems
like Bazel, where the source files are marked as read-only. The goal of
the check is merely to see if a file exists, so checking for read
permissions should be sufficient.
Let me know if any additional information is needed. I didn't see any
good spots to add tests, but I'm willing to add them with some
guidance.
Bug
fixes #numbercontributing.md