Skip to content

Commit

Permalink
Don't require source files to be writeable in dev mode
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Evan Bradley committed Nov 2, 2021
1 parent d5d1bc0 commit 0b70497
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 2 deletions.
4 changes: 2 additions & 2 deletions packages/next/server/dev/hot-reloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import onDemandEntryHandler, {
} from './on-demand-entry-handler'
import { denormalizePagePath, normalizePathSep } from '../normalize-page-path'
import getRouteFromEntrypoint from '../get-route-from-entrypoint'
import { isWriteable } from '../../build/is-writeable'
import { fileExists } from '../../lib/file-exists'
import { ClientPagesLoaderOptions } from '../../build/webpack/loaders/next-client-pages-loader'
import { ssrEntries } from '../../build/webpack/plugins/middleware-plugin'
import { stringify } from 'querystring'
Expand Down Expand Up @@ -452,7 +452,7 @@ export default class HotReloader {
}

const { bundlePath, absolutePagePath, dispose } = entries[pageKey]
const pageExists = !dispose && (await isWriteable(absolutePagePath))
const pageExists = !dispose && (await fileExists(absolutePagePath))
if (!pageExists) {
// page was removed or disposed
delete entries[pageKey]
Expand Down
6 changes: 6 additions & 0 deletions test/integration/read-only-source-hmr/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
onDemandEntries: {
// Make sure entries are not getting disposed.
maxInactiveAge: 1000 * 60 * 60,
},
}
3 changes: 3 additions & 0 deletions test/integration/read-only-source-hmr/pages/hello.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const Hello = () => <p>Hello World</p>

export default Hello
117 changes: 117 additions & 0 deletions test/integration/read-only-source-hmr/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/* eslint-env jest */

import fs from 'fs-extra'
import {
check,
findPort,
getBrowserBodyText,
killApp,
launchApp,
} from 'next-test-utils'
import webdriver from 'next-webdriver'
import { join } from 'path'

const READ_ONLY_PERMISSIONS = 0o444
const READ_WRITE_PERMISSIONS = 0o644

const appDir = join(__dirname, '..')
const pagePath = join(appDir, 'pages/hello.js')

let appPort
let app

async function writeReadOnlyFile(path, content) {
await fs.chmod(pagePath, READ_WRITE_PERMISSIONS)
await fs.writeFile(path, content, 'utf8')
await fs.chmod(pagePath, READ_ONLY_PERMISSIONS)
}

describe('Read-only source HMR', () => {
beforeAll(async () => {
await fs.chmod(pagePath, READ_ONLY_PERMISSIONS)

appPort = await findPort()
app = await launchApp(appDir, appPort, {
env: {
__NEXT_TEST_WITH_DEVTOOL: 1,
// Events can be finicky in CI. This switches to a more reliable
// polling method.
CHOKIDAR_USEPOLLING: 'true',
CHOKIDAR_INTERVAL: 500,
},
})
})

afterAll(async () => {
await fs.chmod(pagePath, READ_WRITE_PERMISSIONS)
await killApp(app)
})

it('should detect changes to a page', async () => {
let browser

try {
browser = await webdriver(appPort, '/hello')
await check(() => getBrowserBodyText(browser), /Hello World/)

const originalContent = await fs.readFile(pagePath, 'utf8')
const editedContent = originalContent.replace('Hello World', 'COOL page')

await writeReadOnlyFile(pagePath, editedContent)
await check(() => getBrowserBodyText(browser), /COOL page/)

await writeReadOnlyFile(pagePath, originalContent)
await check(() => getBrowserBodyText(browser), /Hello World/)
} finally {
if (browser) {
await browser.close()
}
}
})

it('should handle page deletion and subsequent recreation', async () => {
let browser

try {
browser = await webdriver(appPort, '/hello')
await check(() => getBrowserBodyText(browser), /Hello World/)

const originalContent = await fs.readFile(pagePath, 'utf8')

await fs.remove(pagePath)

await fs.writeFile(pagePath, originalContent, {
mode: READ_ONLY_PERMISSIONS,
})
await check(() => getBrowserBodyText(browser), /Hello World/)
} finally {
if (browser) {
await browser.close()
}
}
})

it('should detect a new page', async () => {
let browser
const newPagePath = join(appDir, 'pages/new.js')

await writeReadOnlyFile(
newPagePath,
`
const New = () => <p>New page</p>
export default New
`
)

try {
browser = await webdriver(appPort, '/new')
await check(() => getBrowserBodyText(browser), /New page/)
} finally {
if (browser) {
await browser.close()
}
await fs.remove(newPagePath)
}
})
})
20 changes: 20 additions & 0 deletions test/integration/read-only-source-hmr/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"compilerOptions": {
"esModuleInterop": true,
"module": "esnext",
"jsx": "preserve",
"target": "es5",
"lib": ["dom", "dom.iterable", "esnext"],
"allowJs": true,
"skipLibCheck": true,
"strict": true,
"forceConsistentCasingInFileNames": true,
"noEmit": true,
"incremental": true,
"moduleResolution": "node",
"resolveJsonModule": true,
"isolatedModules": true
},
"exclude": ["node_modules"],
"include": ["next-env.d.ts", "pages"]
}

0 comments on commit 0b70497

Please sign in to comment.