Skip to content

chore(deps): drop three unmaintained dependencies (unorm, find-root, jsonminify)#7992

Merged
JohnMcLear merged 3 commits into
ether:developfrom
JohnMcLear:chore/drop-stale-deps
Jun 22, 2026
Merged

chore(deps): drop three unmaintained dependencies (unorm, find-root, jsonminify)#7992
JohnMcLear merged 3 commits into
ether:developfrom
JohnMcLear:chore/drop-stale-deps

Conversation

@JohnMcLear

Copy link
Copy Markdown
Member

What

Removes three production dependencies whose upstreams are effectively abandoned, replacing each with a maintained alternative or a native API. No user-facing behaviour change — this just shrinks the dependency surface and fixes a couple of latent settings-parser bugs along the way.

This is the low-risk first slice of a broader dependency-staleness audit of core.

Dep Last npm publish Replacement
unorm 2019-07 (~7 yr) native String.prototype.normalize('NFC')
find-root 2017-06 (~9 yr) ~10-line inline helper in AbsolutePaths.ts
jsonminify 2021-12 (~4.5 yr) jsonc-parser (already used by the admin workspace)

Details

  • unormcontentcollector.ts only used UNorm.nfc(s). String.prototype.normalize('NFC') is available in every Node and browser Etherpad supports, so the polyfill is dead weight. Also dropped from Minify.ts's LIBRARY_WHITELIST (nothing imports it client-side anymore).

  • find-root — used once, in findEtherpadRoot(). Inlined a small helper with identical semantics (closest ancestor directory containing package.json, throws if none found before filesystem root). Verified it resolves to the same repo root.

  • jsonminify — settings parsing was jsonminify(str).replace(',]', ']').replace(',}', '}'). That had two real bugs that jsonc-parser (allowTrailingComma: true) fixes:

    1. String#replace with a string needle only swaps the first match, so a settings file with more than one trailing comma of the same kind stayed invalid.
    2. The blind ,] / ,} replace also corrupted those byte sequences inside string values (e.g. a URL containing ,]).

    Error semantics are preserved: a malformed/empty settings file still logs and process.exit(1)s.

Tests

  • Added a regression test in tests/backend/specs/settings.ts covering multiple trailing commas and ,]/,} sequences inside string values.
  • settings.ts spec: 32 passing.
  • contentcollector.ts + import.ts specs (full server boot + HTML/DOCX/PDF round-trips, exercise the NFC path): 123 passing.
  • tsc --noEmit: clean.

Not in scope

measured-core was in the original audit shortlist but is descoped — it backs the /stats endpoint's toJSON() shape (admin cards + a public endpoint depend on it), so swapping it to prom-client is a behaviour change, not a cleanup. Tracking that, plus security/html-to-docx/languages4translatewiki adoption, separately.

🤖 Generated with Claude Code

Remove dependencies whose upstreams are effectively abandoned, replacing
each with a maintained alternative or native API. No behaviour change for
users; reduces the production dependency surface.

- unorm (last publish 2019): replace `UNorm.nfc(s)` in contentcollector
  with native `String.prototype.normalize('NFC')`, available in every
  supported Node and browser. Also drop it from Minify's LIBRARY_WHITELIST.
- find-root (last publish 2017): inline a ~10-line equivalent in
  AbsolutePaths.findEtherpadRoot(), mirroring find-root's semantics
  (closest ancestor containing package.json, throw if none).
- jsonminify (last publish 2021): swap settings parsing to jsonc-parser
  (already used by the admin workspace, actively maintained). The old
  `jsonminify(str).replace(',]', ']').replace(',}', '}')` had two bugs that
  jsonc-parser's allowTrailingComma fixes: String#replace only swapped the
  FIRST trailing comma of each kind, and the blind replace corrupted ',]' /
  ',}' byte sequences inside string values (e.g. URLs).

Added a regression test in settings.ts covering multiple trailing commas
and ',]'/',}' inside string values.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Drop unmaintained deps (unorm/find-root/jsonminify) with native + jsonc-parser
⚙️ Configuration changes 🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

Description

• Remove three stale runtime dependencies to shrink core’s production dependency surface.
• Replace unorm/find-root/jsonminify with native normalize(), inline root search, and jsonc-parser.
• Add regression tests for JSONC settings parsing edge cases (comments, trailing commas, string
 safety).
Diagram

graph TD
  T["Settings tests"] --> S["Settings loader"] --> J["jsonc-parser"]
  S --> A["AbsolutePaths"] --> F[("Filesystem")]
  M["Minify whitelist"] --> B["Static bundle"] --> C["Content collector"] --> N["String.normalize"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Preprocess with strip-json-comments + custom trailing-comma cleanup
  • ➕ Could avoid adding jsonc-parser if already present in the workspace
  • ➕ Keeps JSON.parse semantics and data types identical
  • ➖ Easy to reintroduce the same class of bugs (partial replacements, string corruption)
  • ➖ Harder to produce good error locations/messages than jsonc-parser
2. Use json5 for settings parsing
  • ➕ Well-known, tolerant parser for comments/trailing commas
  • ➖ Adds a new dependency with slightly different grammar/semantics than JSON/JSONC
  • ➖ May accept inputs the project doesn’t want to support long-term
3. Use a maintained root-finding package (e.g., pkg-dir / find-up)
  • ➕ Battle-tested edge cases and symlink handling
  • ➖ Reintroduces a dependency for a small, stable behavior that’s easy to inline
  • ➖ Undercuts the goal of reducing dependency surface

Recommendation: Current approach is best: native String.prototype.normalize removes an unnecessary polyfill, the inlined findRoot keeps behavior while eliminating an abandoned dependency, and jsonc-parser (already used elsewhere) fixes real correctness issues without fragile string munging. The added regression tests specifically lock in the intended parsing semantics.

Files changed (7) +98 / -46

Enhancement (2) +23 / -3
AbsolutePaths.tsInline package.json root discovery to replace find-root +20/-1

Inline package.json root discovery to replace find-root

• Adds a small synchronous directory-walk helper that finds the nearest ancestor containing package.json, throwing at filesystem root. findEtherpadRoot() now calls the inline helper instead of requiring the unmaintained find-root package.

src/node/utils/AbsolutePaths.ts

contentcollector.tsUse native NFC normalization instead of unorm polyfill +3/-2

Use native NFC normalization instead of unorm polyfill

• Removes the unorm import and replaces UNorm.nfc() with String.prototype.normalize('NFC'). Keeps the same normalization intent while relying on built-in runtime support.

src/static/js/contentcollector.ts

Bug fix (1) +13 / -4
Settings.tsSwitch settings parsing from jsonminify+JSON.parse to jsonc-parser +13/-4

Switch settings parsing from jsonminify+JSON.parse to jsonc-parser

• Replaces jsonminify-based preprocessing and naive trailing-comma string replacement with jsonc-parser configured to allow trailing commas. Preserves failure behavior (logs error and exits) while avoiding corruption of string contents and correctly handling multiple trailing commas.

src/node/utils/Settings.ts

Tests (1) +58 / -0
settings.tsAdd regression coverage for JSONC parsing migration +58/-0

Add regression coverage for JSONC parsing migration

• Adds tests that validate comment stripping, multiple trailing commas at different nesting levels, and preservation of literal ",]"/",}" sequences inside string values. Prevents regressions back to the previous naive replace-based approach.

src/tests/backend/specs/settings.ts

Other (3) +4 / -39
pnpm-lock.yamlRemove find-root/jsonminify/unorm and add jsonc-parser to lockfile +3/-34

Remove find-root/jsonminify/unorm and add jsonc-parser to lockfile

• Drops the three stale packages (and @types/jsonminify) from the lockfile and records jsonc-parser as a new dependency for core. Snapshot entries are updated accordingly to reflect the new dependency graph.

pnpm-lock.yaml

Minify.tsRemove unorm from client library whitelist +0/-1

Remove unorm from client library whitelist

• Deletes unorm from LIBRARY_WHITELIST so it is no longer treated as an allowed/minified client-side library. Reflects that the codebase no longer imports unorm in the browser bundle.

src/node/utils/Minify.ts

package.jsonReplace jsonminify/unorm/find-root with jsonc-parser dependency +1/-4

Replace jsonminify/unorm/find-root with jsonc-parser dependency

• Removes find-root, jsonminify, unorm, and @types/jsonminify from core manifests and adds jsonc-parser. This reduces production dependency surface while keeping supported behavior via maintained/native alternatives.

src/package.json

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 21, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Stale unorm bundle entry ✓ Resolved 🐞 Bug ≡ Correctness
Description
The PR removes unorm from the static-file whitelist, but the client bundle manifest still lists
$unorm/lib/unorm.js under ace2_inner.js, so the server will try to serve a static asset that no
longer exists and clients can hit 404s when loading that bundle.
Code

src/node/utils/Minify.ts[46]

-  'unorm',
Evidence
tar.json still includes $unorm/lib/unorm.js, and the server reads tar.json to build the client
bundle file list, but unorm is no longer whitelisted for static serving and is no longer a
dependency. That combination makes the referenced asset path unresolvable.

src/node/utils/tar.json[61-84]
src/node/hooks/express/static.ts[13-31]
src/node/utils/Minify.ts[39-46]
src/package.json[36-93]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`unorm` was removed from dependencies and from `LIBRARY_WHITELIST`, but `src/node/utils/tar.json` still references `$unorm/lib/unorm.js` for the `ace2_inner.js` bundle. This leaves a bundle manifest entry pointing to a non-existent static asset, causing 404s/broken bundle loads.
## Issue Context
- `getTar()` reads `tar.json` at runtime to build the list of files that make up client bundles.
- `Minify.ts` no longer allows serving `unorm` from `node_modules`, and `unorm` is no longer installed.
## Fix Focus Areas
- src/node/utils/tar.json[61-84]
- (optional verification) src/node/hooks/express/static.ts[13-31]
- (optional verification) src/node/utils/Minify.ts[39-46]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/node/utils/Minify.ts
JohnMcLear and others added 2 commits June 21, 2026 13:21
The unorm removal left `$unorm/lib/unorm.js` listed in the ace2_inner.js
client bundle manifest. With unorm uninstalled, getTar() would point at a
node_modules asset that no longer exists, producing 404s when loading that
bundle. Nothing imports unorm anymore (contentcollector now uses native
String.prototype.normalize), so the entry is dead and removed.

Caught by Qodo review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tests/container/loadSettings.js is a standalone helper (separate from
node/utils/Settings.ts) that parsed settings.json.docker with jsonminify
directly. Removing jsonminify from dependencies broke the container test
suite with MODULE_NOT_FOUND. Switch it to jsonc-parser to match Settings.ts.

Verified loadSettings() parses settings.json.docker and applies the
container ip/port overrides.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@JohnMcLear JohnMcLear merged commit 7168f14 into ether:develop Jun 22, 2026
24 checks passed
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.

1 participant