Improved docs, package.json and .npmignore for package preparation

[Mehulantony]

Added npm installation instructions, clarified environment variable security, and documented packaging verification steps (npm pack --dry-run, npm publish --dry-run) to ensure safe and clean npm publishing.

[thehabes]

I lack knowledge in this area so I don't want to approve or request changes. I relied heavily on the static reviewer. Look over the comments and see if they have any merit and warrant changes.

Static Review Comments

Branch: mehul/npm-package-docs
Review Date: 2026-03-30
Reviewer: Pair Static Review - Claude & @thehabes

Claude and Bryan make mistakes. Verify all issues and suggestions. Avoid unnecessary scope creep.

Category	Issues Found
🔴 Critical	1
🟠 Major	1
🟡 Minor	1
🔵 Suggestions	0

Critical Issues 🔴

Issue 1: Missing critical runtime files in files array — package will crash on import

File: package.json:32-43
Category: Breaking Change / Runtime Error

Problem:
The files array lists the directories and files to include in the npm tarball, but omits three root-level modules that are required by the included code:

Missing File	Imported By
utils.js	controllers/utils.js, controllers/crud.js, controllers/history.js, controllers/bulk.js, controllers/putUpdate.js, controllers/patchUpdate.js, controllers/patchSet.js, controllers/patchUnset.js, controllers/overwrite.js, controllers/release.js, controllers/search.js, controllers/gog.js, controllers/delete.js
rest.js	app.js, routes/patchUpdate.js, routes/patchUnset.js, routes/patchSet.js
db-controller.js	Nearly every route handler (routes/create.js, routes/query.js, routes/delete.js, routes/id.js, routes/history.js, routes/since.js, routes/search.js, routes/release.js, routes/overwrite.js, routes/putUpdate.js, routes/patchUpdate.js, routes/patchSet.js, routes/patchUnset.js, routes/bulkCreate.js, routes/bulkUpdate.js, routes/client.js, GOG routes)

Confirmed via npm pack --dry-run — none of these three files appear in the tarball. Any consumer running import { app } from 'rerum_server_nodejs' will get a MODULE_NOT_FOUND error.

Current Code:

  "files": [
    "app.js",
    "index.js",
    "bin/",
    "auth/",
    "config/",
    "controllers/",
    "database/",
    "public/",
    "routes/",
    "README.md"
  ],

Suggested Fix:

  "files": [
    "app.js",
    "index.js",
    "utils.js",
    "rest.js",
    "db-controller.js",
    "bin/",
    "auth/",
    "config/",
    "controllers/",
    "database/",
    "public/",
    "routes/",
    "README.md"
  ],

How to Verify:

npm pack --dry-run 2>&1 | grep -E "(utils\.js|rest\.js|db-controller\.js)" | grep -v controllers/

You should see all three root-level files listed. Then install locally and verify import works:

npm pack && mkdir /tmp/test-rerum && cd /tmp/test-rerum && npm init -y && npm install /path/to/rerum_server_nodejs-0.0.0.tgz && node -e "import('rerum_server_nodejs')"

---

Major Issues 🟠

Issue 1: Test and mock files leak into the published package

File: package.json:32-43
Category: Code Hygiene / Package Size

Problem:
22 test and mock files are included in the tarball (~82 KB of unnecessary content). The .npmignore already has patterns for __tests__/ and __mocks__/, but per npm docs: a root-level .npmignore does not override the files field. Since files includes whole directories like routes/, auth/, and database/, test subdirectories within them are packaged regardless of .npmignore.

Confirmed via npm pack --dry-run:

auth/__mocks__/index.txt
auth/__tests__/token.test.txt
database/__mocks__/index.txt
routes/__tests__/bulkCreate.test.js
routes/__tests__/create.test.js
... (22 files total)

Consumers of the package don't need test files. This inflates package size and may confuse users browsing node_modules.

Suggested Fix — Since .npmignore cannot override files, use negation globs directly in the files array:

  "files": [
    "app.js",
    "index.js",
    "utils.js",
    "rest.js",
    "db-controller.js",
    "bin/",
    "auth/",
    "config/",
    "controllers/",
    "database/",
    "public/",
    "routes/",
    "README.md",
    "!**/__tests__/",
    "!**/__mocks__/"
  ],

If negation globs in files don't work for your npm version, the alternative is to list specific subdirectories instead of whole parent directories (e.g. routes/*.js instead of routes/).

How to Verify:

npm pack --dry-run 2>&1 | grep -E "(__tests__|__mocks__|\.test\.)"

Should return no results.

Minor Issues 🟡

Issue 1: files array has inconsistent indentation

File: package.json:32
Category: Code Hygiene / JSON Formatting

Problem:
The files key has extra leading whitespace compared to sibling keys. While valid JSON, it breaks the formatting consistency of the file, making it look like files is nested under main when it's actually a peer.

Current Code:

  "main": "index.js",
    "files": [
    "app.js",

Suggested Fix:

  "main": "index.js",
  "files": [
    "app.js",

How to Verify:
Visual inspection. Run a JSON formatter to confirm the file parses correctly and is consistently indented.

---

Suggestions 🔵

None.

---

If there are significant code changes in response to this review please test those changes. Run the application manually and test or perform internal application tests when applicable.

[thehabes]

@allenbakki the previous review still stands, as there have been no changes committed since the previous review.

[thehabes]

Again I am not very knowledgeable on the npm packing so I am relying heavily on the static reviewer here, I am not able to verify what it is saying against my own expertise. Take the review with a grain of salt and look at these things yourself to decide if they warrant any changes. Merge the pull request when you believe it is tidy and functional.

I think it makes a good point about the README, the two installation types/paths should be decoupled and made clear.

Static Review Comments

Branch: mehul/npm-package-docs
Review Date: 2026-04-26
Reviewer: Pair Static Review - Claude & @thehabes

Claude and Bryan make mistakes. Verify all issues and suggestions. Avoid unnecessary scope creep.

Category	Issues Found
🔴 Critical	0
🟠 Major	1
🟡 Minor	4
🔵 Suggestions	1

---

Critical Issues 🔴

None.

---

Major Issues 🟠

---

Issue 1: .npmignore is overridden by the new files whitelist in package.json

File: package.json:37-53 and .npmignore (entire file)
Category: Packaging / Maintenance trap

Problem:
When a package.json has a files array, npm uses it as a whitelist that takes precedence over .npmignore (with only a few always-included exceptions: package.json, README, LICENSE, the main file, etc.). The npm pack --dry-run run on this branch confirmed this: nothing in .npmignore is doing actual filtering — the whitelist already excludes everything not listed. The .npmignore rewrite is therefore mostly cosmetic, and worse, it sets a trap where a future maintainer adds an entry to .npmignore (e.g. blocking a newly added secret file) and is surprised when it still ships because it's inside a whitelisted directory.

Two concrete consequences worth noting:

• .npmignore lists CONTRIBUTING.md and DEVELOPMENT.md for exclusion, but files already excludes them by omission. Harmless, but misleading.
• The !**/__tests__/ and !**/__mocks__/ negations inside files are what's actually keeping routes/__tests__/, auth/__tests__/, and database/__mocks__/ out of the tarball — not the .npmignore rules.

Suggested Fix:
Either:

1. Delete .npmignore and rely on files as the single source of truth, with a one-line comment in package.json near the files block explaining the strategy, or
2. Keep .npmignore as a defense-in-depth/documentation file but add a header comment:

# NOTE: package.json `files` is the authoritative whitelist for npm publishing.
# Edits to this file alone will NOT change what gets published — adjust `files`
# in package.json (or its `!` negations) as well.

How to Verify:
Add a sentinel file (e.g. routes/SECRET.txt) with routes/SECRET.txt listed in .npmignore only, then run npm pack --dry-run. The file will appear in the tarball, demonstrating that .npmignore is not authoritative. Remove the sentinel after the demo.

---

Minor Issues 🟡

Issue 2: README heading hierarchy break — ### Installation is now stranded under ## Install via npm

File: README.md:69-78
Category: Documentation / Readability

Problem:
Inserting ## Install via npm (H2) immediately before the existing ### Installation (H3) reparents the H3 under the wrong H2. The content of ### Installation is the git-clone-from-source flow (Mongo setup, git clone, .env, etc.) — the semantic opposite of "Install via npm." A reader navigating from the new H2 will read instructions that contradict the section heading. TOC generators and screen readers will also reflect the broken hierarchy.

Current Code:

## 🌟👍 Contributors 👍🌟
…

## Install via npm

If published as a package, install using:
…

### Installation

#### Get a Mongo Database
…

Suggested Fix:
Either move the new section after the source-install section, or re-parent both as siblings under a unifying H2:

## Installation

### Install via npm
If published as a package, install using:

```shell
npm install rerum_server_nodejs
```
…

### Install from source
#### Get a Mongo Database
…

How to Verify:
Render the README on GitHub and check that the auto-generated outline groups the two install methods together rather than nesting source-install under "Install via npm."

---

Issue 3: Duplicate .github/ entry in .npmignore

File: .npmignore

.github/ appears twice — once under "# Development and CI/CD" and again under "# Project documentation and guides". Pick one section; remove the duplicate.

---

Issue 4: .git/ in .npmignore is redundant

File: .npmignore

npm always excludes .git/ from the published tarball regardless of .npmignore. The entry is harmless but adds noise.

---

Issue 5: .npmignore missing trailing newline

File: .npmignore (last line)

Diff shows \ No newline at end of file. Add a trailing newline.

---

Suggestions 🔵

Suggestion 1: Verify config/ does not embed dev-only defaults you don't want shipped

config/index.js is included in files and contains defaults like 'http://localhost:3001/v1/' for RERUM_PREFIX and friends. They're already overridden by .env at runtime, but a consumer of the package will see localhost defaults if they forget to set env vars. Consider either (a) throwing on missing required env vars for the public-facing prefixes, or (b) documenting these defaults explicitly in the npm install section.

---

If there are significant code changes in response to this review please test those changes. Run the application manually and test or perform internal application tests when applicable.