Skip to content

Add production Express server (dist/ + dev/prod parity)#13

Open
claude[bot] wants to merge 7 commits into
mainfrom
claude/issue-12-run-26930285359
Open

Add production Express server (dist/ + dev/prod parity)#13
claude[bot] wants to merge 7 commits into
mainfrom
claude/issue-12-run-26930285359

Conversation

@claude

@claude claude Bot commented Jun 4, 2026

Copy link
Copy Markdown

Summary

Closes #12. Part of #7.

Usage

# dev (once #8 + #9 are wired into vite.config.js)
npm run dev

# prod
SESSION_SECRET=<secret> npm run build && npm start

SESSION_SECRET must be set in production — client-sessions uses it to sign + encrypt the cookie. Without it the middleware will throw on startup.

Notes on dependencies

src/api/index.js and src/api/404.js are created here because server.js imports them. They are also the prerequisite files for issues #8 (dev API), #10 (404 handler), and #11 (auth). Those issues can build on / refine these files rather than create them from scratch.

Test plan

  • npm run build completes without errors
  • SESSION_SECRET=test node server.js starts without errors
  • curl http://localhost:3000/ returns the built index.html
  • curl http://localhost:3000/nonexistent returns a 404 HTML response
  • curl -X POST http://localhost:3000/api/login -H 'content-type: application/json' -d '{"name":"alice"}' returns {"ok":true} with a Set-Cookie header

🤖 Generated with Claude Code

Closes #12. Part of #7.

- server.js: reuses src/api/index.js (sessions + API) + serves dist/ + custom 404
- src/api/index.js: shared Express app with client-sessions, /api/login, /api/logout, requireUser
- src/api/404.js: terminal 404 handler for both dev (post-hook) and prod
- package.json: adds express + client-sessions deps; changes start script to node server.js

SESSION_SECRET must be set in production. Run with: npm run build && npm start

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@chriscalo chriscalo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@claude, please address all PR comments

Comment thread package.json
Comment thread src/api/404.js Outdated
export default function notFound(req, res) {
res.statusCode = 404;
res.setHeader("content-type", "text/html");
res.end(`<!DOCTYPE html>

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please put this in a separate 404.html file

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done — moved the HTML into src/api/404.html and updated 404.js to read it from disk using readFileSync(new URL('404.html', import.meta.url), 'utf8') (ESM-compatible path resolution).

Comment thread src/api/index.js Outdated
});

export function requireUser(req, res, next) {
if (!req.session?.user) return res.status(401).json({ error: "unauthorized" });

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't smash this into one line

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed — expanded into a braced block.

- Move inline HTML from 404.js into src/api/404.html
- Read 404.html from disk via import.meta.url (ESM-compatible)
- Expand one-line guard in requireUser into braced block

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@chriscalo chriscalo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@claude, some more changes, please

Comment thread src/api/404.html
Comment thread src/api/404.js
Comment thread src/api/404.js Outdated
export default function notFound(req, res) {
res.statusCode = 404;
res.setHeader("content-type", "text/html");
res.end(html);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Isn't there a send file function?

Or a way to use templated views to send it?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done — using res.sendFile(). Dropped the readFileSync pre-read; sendFile handles disk I/O, content-type, and ETag automatically.

Comment thread src/api/index.js Outdated
res.json({ ok: true });
});

export function requireUser(req, res, next) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

requireUser is a weird name. There's always a user. Maybe we mean requireLogin or similar?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Renamed to requireLogin.

- 404.html: proper 2-space indentation, self-closing <meta />
- 404.js: use res.status(404).sendFile() instead of readFileSync + res.end
- index.js: rename requireUser → requireLogin (there's always a user)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed all comments — pushed in one commit. Summary:

  • 404.html: 2-space indentation, elements on separate lines, <meta /> self-closing
  • 404.js: replaced readFileSync + raw Node response with res.status(404).sendFile()
  • index.js: requireUserrequireLogin

@chriscalo chriscalo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@claude, more changes

Comment thread src/api/index.js Outdated
app.use(sessions({
cookieName: "session",
secret: process.env.SESSION_SECRET,
duration: 24 * 60 * 60 * 1000,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Use named constants

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done — extracted ONE_DAY_MS and FIVE_MINUTES_MS constants above the app setup.

Comment thread src/api/index.js
Comment thread src/api/index.js Outdated
- Extract ONE_DAY_MS and FIVE_MINUTES_MS constants for session durations
- Add GET /login route serving login.html (name field, no password)
- Add path helpers to index.js (needed for sendFile)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@chriscalo chriscalo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@claude, a few more when you're done with the last batch

Comment thread server.js Outdated

app.use(express.static("dist"));
app.use(notFound);
app.listen(process.env.PORT || 3000);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Comment thread src/api/404.js Outdated
const __dirname = dirname(fileURLToPath(import.meta.url));

export default function notFound(req, res) {
res.status(404).sendFile(join(__dirname, "404.html"));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Construct the path separately from sending the file

- server.js: replace app.listen() with createServer + promise-based
  startup (node:http pattern), logs URL on ready
- src/api/404.js: extract filePath variable before calling sendFile

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jun 6, 2026

Copy link
Copy Markdown
Author

Addressed both comments in the latest push:

  • server.js: replaced app.listen() with the promisified startup pattern — createServer(app) from node:http, wrapped in a Promise with .once('listening') / .once('error') handlers, logs the URL on ready
  • src/api/404.js: extracted const filePath = join(__dirname, '404.html') on its own line before passing to sendFile

@chriscalo chriscalo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@claude, some more

Comment thread src/api/login.html Outdated
<button type="submit">Log In</button>
</form>
<script>
document.getElementById("login-form").addEventListener("submit", async (e) => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Comment thread src/api/login.html Outdated
const name = e.target.elements.name.value;
const res = await fetch("/api/login", {
method: "POST",
headers: { "content-type": "application/json" },

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why JSON instead of form data?

… selectors, form data

- server.js: wrap startup in listen() helper that returns { url, port, listener }
- src/api/404.js: hoist filePath to module level (computed once, not per request)
- src/api/login.html: use document.querySelector("form"), send URLSearchParams form data
- src/api/index.js: use express.urlencoded() to match form data submission

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jun 6, 2026

Copy link
Copy Markdown
Author

Addressed all remaining comments in one commit:

  • server.js: wrapped startup in a listen(app, port) helper that returns { url, port, listener } per the server-startup pattern
  • src/api/404.js: hoisted filePath to module level — constructed once at load time, not on every request
  • src/api/login.html:
    • document.querySelector("form") instead of getElementById; removed the now-unused id attribute
    • sends new URLSearchParams(new FormData(e.target)) (URL-encoded form data) instead of JSON
  • src/api/index.js: switched login route to express.urlencoded({ extended: false }) to match

@chriscalo chriscalo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@claude, more changes

Comment thread server.js

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is not following the pattern:

https://chriscalo.github.io/dev-skills/skills/nodejs/express/server-startup

Please fix. Use the listen() function verbatim.

Comment thread src/api/404.js Outdated
const __dirname = dirname(fileURLToPath(import.meta.url));

export default function notFound(req, res) {
const filePath = join(__dirname, "404.html");

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Replace generic var name with something like notFoundHTML

And build the path once outside this function and reference in each execution

Comment thread src/api/index.js
res.json({ ok: true });
});

app.post("/api/logout", (req, res) => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

There needs to be a UI for logging out

- server.js: rewrite listen() verbatim from the server-startup pattern —
  nested startServer(), try/catch, EADDRINUSE retry with clearPort/delay;
  add DEVELOPMENT constant
- src/api/404.js: rename filePath → notFoundPath
- src/api/index.js: add GET /logout route serving logout.html
- src/api/logout.html: new logout page with form that POSTs to /api/logout
  and redirects to /login on success

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jun 6, 2026

Copy link
Copy Markdown
Author

Addressed all three comments in the latest push:

  • server.js: rewrote listen() verbatim from the server-startup pattern — nested startServer() inner function, try/catch block, EADDRINUSE retry with clearPort + delay; added DEVELOPMENT constant and helper functions
  • src/api/404.js: renamed filePathnotFoundPath
  • src/api/index.js: added GET /logout route that serves logout.html; created src/api/logout.html — a page with a Log Out button that POSTs to /api/logout and redirects to /login on success

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a production Express server (dist/ + dev/prod parity)

1 participant