Skip to content

fix: backward-compat for ESM etherpad#87

Merged
JohnMcLear merged 1 commit into
mainfrom
fix/esm-compat
May 26, 2026
Merged

fix: backward-compat for ESM etherpad#87
JohnMcLear merged 1 commit into
mainfrom
fix/esm-compat

Conversation

@SamTV12345

Copy link
Copy Markdown
Member

This PR makes the plugin backward-compatible with the upcoming ESM etherpad branch (ether/etherpad#7605).

Change: Remove trailing slash from require("ep_etherpad-lite/node/eejs/")require("ep_etherpad-lite/node/eejs")

The trailing-slash form breaks under Node's strict ESM exports map resolution. This change is backward-compatible with the current CJS etherpad release.

- Drop trailing slash on ep_etherpad-lite/node/eejs/ require

Backward-compatible with current CJS etherpad release; also
compatible with the upcoming ESM etherpad branch which has stricter
exports map resolution.
@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

Review Summary by Qodo

Remove trailing slash for ESM etherpad compatibility

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Remove trailing slash from eejs require path
• Ensures ESM etherpad compatibility with strict exports
• Maintains backward compatibility with current CJS release
• Bump version to 0.0.58
Diagram
flowchart LR
  A["require with trailing slash"] -- "remove slash" --> B["require without trailing slash"]
  B -- "compatible with" --> C["ESM etherpad"]
  B -- "compatible with" --> D["CJS etherpad"]

Loading

File Changes

1. eejs.js 🐞 Bug fix +1/-1

Remove trailing slash from eejs require

• Remove trailing slash from ep_etherpad-lite/node/eejs/ require statement
• Change from require('ep_etherpad-lite/node/eejs/') to require('ep_etherpad-lite/node/eejs')
• Fixes compatibility with strict ESM exports map resolution

eejs.js


2. package.json ⚙️ Configuration changes +1/-1

Bump version to 0.0.58

• Increment version from 0.0.57 to 0.0.58
• Reflects the bug fix release

package.json


Grey Divider

Qodo Logo

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

qodo-free-for-open-source-projects Bot commented May 25, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1)

Grey Divider


Action required

1. No regression test for eejs require 📘 Rule violation ☼ Reliability
Description
This PR changes the require() path in eejs.js as a bug fix, but it does not add or update any
automated test to prevent regression. Without a regression test, the ESM compatibility fix is not
verifiable and may break again unnoticed.
Code

eejs.js[3]

Evidence
PR Compliance ID 1 requires a regression test alongside any bug fix. The diff shows a bug-fix change
to the require() path in eejs.js, but there are no accompanying test additions/updates in this
PR to cover the fixed behavior.

AGENTS.md: Bug Fixes Must Include a Regression Test in the Same Commit
eejs.js[1-8]

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

## Issue description
A bug fix was made (changing `require('ep_etherpad-lite/node/eejs/')` to `require('ep_etherpad-lite/node/eejs')`) but no regression test was added/updated in the same PR.

## Issue Context
The project already has CI workflows that execute plugin-provided backend tests from `static/tests/backend/specs/**` (via Etherpad core's Mocha runner). Add a small backend regression test that would fail with the old trailing-slash form and pass with the new form (for example, by asserting the plugin's `eejs.js` source does not contain the trailing-slash require path, or by requiring the module in a way that triggers resolution).

## Fix Focus Areas
- eejs.js[1-8]
- static/tests/backend/specs/eejs-esm-compat.spec.ts[1-200]

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


Grey Divider

Qodo Logo

Comment thread eejs.js
'use strict';

const eejs = require('ep_etherpad-lite/node/eejs/');
const eejs = require('ep_etherpad-lite/node/eejs');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. No regression test for eejs require 📘 Rule violation ☼ Reliability

This PR changes the require() path in eejs.js as a bug fix, but it does not add or update any
automated test to prevent regression. Without a regression test, the ESM compatibility fix is not
verifiable and may break again unnoticed.
Agent Prompt
## Issue description
A bug fix was made (changing `require('ep_etherpad-lite/node/eejs/')` to `require('ep_etherpad-lite/node/eejs')`) but no regression test was added/updated in the same PR.

## Issue Context
The project already has CI workflows that execute plugin-provided backend tests from `static/tests/backend/specs/**` (via Etherpad core's Mocha runner). Add a small backend regression test that would fail with the old trailing-slash form and pass with the new form (for example, by asserting the plugin's `eejs.js` source does not contain the trailing-slash require path, or by requiring the module in a way that triggers resolution).

## Fix Focus Areas
- eejs.js[1-8]
- static/tests/backend/specs/eejs-esm-compat.spec.ts[1-200]

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

@JohnMcLear JohnMcLear merged commit 9607f77 into main May 26, 2026
2 of 3 checks passed
@JohnMcLear JohnMcLear deleted the fix/esm-compat branch May 26, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants