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

templates(express): use watcher.on("add").on("change") #6922

Closed
wants to merge 5 commits into from
Closed

templates(express): use watcher.on("add").on("change") #6922

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a792978
Use watcher.on("add").on("change") for express template
xHomu Jul 21, 2023
503a3f3
Merge branch 'remix-run:main' into patch-2
xHomu Jul 22, 2023
6cbb0e9
Update server.js
xHomu Jul 24, 2023
84371b5
Update server.js
xHomu Aug 1, 2023
3dc9615
Merge remote-tracking branch 'upstream/main' into patch-2
xHomu Aug 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 59 additions & 10 deletions templates/express/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ const BUILD_PATH = "./build/index.js";
*/
let build = await import(BUILD_PATH);

//Swap in this if using Remix in CJS mode
// let build = require(BUILD_PATH);

const app = express();

app.use(compression());
Expand Down Expand Up @@ -48,31 +51,77 @@ const port = process.env.PORT || 3000;
app.listen(port, async () => {
console.log(`Express server listening on port ${port}`);

// send "ready" message to dev server
if (process.env.NODE_ENV === "development") {
broadcastDevReady(build);
}
});


// Create a request handler that watches for changes to the server build during development.
function createDevRequestHandler() {
const watcher = chokidar.watch(BUILD_PATH, { ignoreInitial: true });
async function handleServerUpdate() {
// 1. re-import the server build
build = await reimportServer();

watcher.on("all", async () => {
// 1. purge require cache && load updated server build
const stat = fs.statSync(BUILD_PATH);
build = import(BUILD_PATH + "?t=" + stat.mtimeMs);
// 2. tell dev server that this app server is now ready
broadcastDevReady(await build);
});
// Add debugger to assist in v2 dev debugging
if (build?.assets === undefined) {
console.log(build.assets);
debugger;
}

// 2. tell dev server that this app server is now up-to-date and ready
broadcastDevReady(build);
}

chokidar
Copy link
Contributor

Choose a reason for hiding this comment

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

chokidar isn't a production dependency, so it would be better to use (await import("chokidar")), remove the chokidar import at the top of the file, make createDevRequestHandler() async, and await on it at the app.all() handler.

Copy link
Contributor Author

@xHomu xHomu Aug 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Updated the patch to use await import, not sure if it's the best way to write this?

Copy link
Contributor

Choose a reason for hiding this comment

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

In my code, I imported directly in the createDevRequestHandler() function, but this way is probably the cleanest if swapping in CJS is important.

.watch(BUILD_PATH, {
// Chokidar settings to avoid certain race condition issues #6831
ignoreInitial: true,
awaitWriteFinish: { stabilityThreshold: 200 },
})
.on("add", handleServerUpdate)
.on("change", handleServerUpdate);

// wrap request handler to make sure its recreated with the latest build for every request
return async (req, res, next) => {
try {
//
return createRequestHandler({
build: await build,
build,
mode: "development",
})(req, res, next);
} catch (error) {
next(error);
}
};
}

// ESM import cache busting
// Swap this out for the CJS require cache below if you switch to serverModuleFormat: "cjs" in remix.config
/**
* @type {() => Promise<ServerBuild>}
*/
async function reimportServer() {
const stat = fs.statSync(BUILD_PATH);

// use a timestamp query parameter to bust the import cache
return import(BUILD_PATH + "?t=" + stat.mtimeMs);
}


// // CJS require cache busting
// /**
// * @type {() => Promise<ServerBuild>}
// */
// async function reimportServer() {
// // 1. manually remove the server build from the require cache
// Object.keys(require.cache).forEach((key) => {
// if (key.startsWith(BUILD_PATH)) {
// delete require.cache[key];
// }
// });

// // 2. re-import the server build
// return require(BUILD_PATH);
// }