fix: backward-compat for ESM etherpad#423
Conversation
- Drop trailing slash on ep_etherpad-lite/node/eejs/ require (index.js) - Migrate log4js to ep_plugin_helpers/logger (commentManager.js) Backward-compatible with current CJS etherpad release; also compatible with the upcoming ESM etherpad branch which has stricter exports map resolution.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Review Summary by QodoMake plugin backward-compatible with ESM etherpad
WalkthroughsDescription• Remove trailing slash from eejs require path for ESM compatibility • Migrate log4js to ep_plugin_helpers/logger helper • Update ep_plugin_helpers dependency to ^0.6.7 • Bump version to 11.1.15 Diagramflowchart LR
A["Current CJS<br/>Etherpad"] -->|"Backward<br/>Compatible"| B["Plugin v11.1.15"]
C["Upcoming ESM<br/>Etherpad"] -->|"Now<br/>Compatible"| B
B -->|"Remove trailing slash<br/>from eejs path"| D["index.js"]
B -->|"Use ep_plugin_helpers<br/>logger"| E["commentManager.js"]
B -->|"Update dependencies"| F["package.json"]
File Changes1. index.js
|
Code Review by Qodo
1. No regression test added
|
| const AttributePool = require('ep_etherpad-lite/static/js/AttributePool').default || require('ep_etherpad-lite/static/js/AttributePool'); | ||
| const Changeset = require('ep_etherpad-lite/static/js/Changeset').default || require('ep_etherpad-lite/static/js/Changeset'); | ||
| const eejs = require('ep_etherpad-lite/node/eejs/'); | ||
| const eejs = require('ep_etherpad-lite/node/eejs'); |
There was a problem hiding this comment.
1. No regression test added 📘 Rule violation ☼ Reliability
This PR changes runtime behavior (logger import and eejs require path) but does not add or update any regression test to ensure the bugfix remains covered. Without a test that fails pre-fix and passes post-fix, future refactors could silently reintroduce the compatibility issue.
Agent Prompt
## Issue description
This PR introduces bugfix/compatibility changes but does not include a regression test added/updated in the same commit.
## Issue Context
The changes affect module resolution at runtime (switching `eejs` require path and migrating logging to `ep_plugin_helpers/logger`). A regression test should fail on the pre-fix code and pass with this PR applied.
## Fix Focus Areas
- index.js[7-7]
- commentManager.js[5-9]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| "dependencies": { | ||
| "cheerio": "^1.2.0", | ||
| "ep_plugin_helpers": "^0.6.0", | ||
| "ep_plugin_helpers": "^0.6.7", |
There was a problem hiding this comment.
2. Outdated pnpm lockfile 🐞 Bug ☼ Reliability
package.json now depends on ep_plugin_helpers@^0.6.7 but pnpm-lock.yaml in this PR branch still resolves ep_plugin_helpers to 0.6.2 (and the old specifier ^0.6.0). This can make pnpm installs fail under frozen lockfile settings and can leave the code running with an older ep_plugin_helpers while commentManager.js now requires ep_plugin_helpers/logger.
Agent Prompt
## Issue description
`package.json` was updated to require `ep_plugin_helpers: ^0.6.7`, and the code now imports `ep_plugin_helpers/logger`, but `pnpm-lock.yaml` is still pinned to `ep_plugin_helpers@0.6.2` (and shows the old specifier `^0.6.0`). This makes installs non-reproducible and commonly breaks CI when using `pnpm install --frozen-lockfile`.
## Issue Context
The updated code path is in `commentManager.js` which now does `require('ep_plugin_helpers/logger')`.
## Fix Focus Areas
- package.json[24-28]
- pnpm-lock.yaml[9-22]
- pnpm-lock.yaml[508-511]
- commentManager.js[5-9]
## Suggested fix
1. Run `pnpm install` (or `pnpm install --lockfile-only`) so the lockfile matches the updated dependency range.
2. Commit the updated `pnpm-lock.yaml` so `ep_plugin_helpers` resolves to a version compatible with the new `ep_plugin_helpers/logger` import.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This PR makes the plugin backward-compatible with the upcoming ESM etherpad branch (ether/etherpad#7605).
Changes:
require("ep_etherpad-lite/node/eejs/")→require("ep_etherpad-lite/node/eejs")(inindex.js)require("ep_etherpad-lite/node_modules/log4js")toep_plugin_helpers/logger(incommentManager.js)Both changes are backward-compatible with the current CJS etherpad release.