fix: backward-compat for ESM etherpad#99
Merged
Merged
Conversation
- 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 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 QodoRemove trailing slash from eejs require path
WalkthroughsDescription• Remove trailing slash from eejs require path • Ensures compatibility with ESM etherpad exports • Maintains backward compatibility with current CJS • Bump version to 0.1.62 Diagramflowchart LR
A["require with trailing slash"] -- "remove slash" --> B["require without slash"]
B -- "compatible with" --> C["ESM etherpad"]
B -- "compatible with" --> D["CJS etherpad"]
File Changes1. index.js
|
Code Review by Qodo
1. ESM eejs require untested
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| const {template} = require('ep_plugin_helpers'); | ||
|
|
||
| const eejs = require('ep_etherpad-lite/node/eejs/'); | ||
| const eejs = require('ep_etherpad-lite/node/eejs'); |
There was a problem hiding this comment.
1. Esm eejs require untested 📘 Rule violation ☼ Reliability
This PR fixes a runtime module resolution bug by changing the require() path, but it does not add or update any automated regression test to cover the failing scenario. Without a test, the same ESM compatibility break could reoccur unnoticed in future changes.
Agent Prompt
## Issue description
The PR fixes an ESM-related runtime break (changing the `require('ep_etherpad-lite/node/eejs/')` path), but no regression test was added/updated to ensure the plugin loads and works under the relevant environment.
## Issue Context
Per compliance, bug fixes must include at least one regression test that would fail before the fix and pass after. For this change, a good regression test is a plugin-load/route smoke test that fails if the plugin cannot be required/initialized due to module resolution rules.
## Fix Focus Areas
- static/tests/backend/specs/ (add a new spec file)
- static/tests/frontend-new/specs/smoke.spec.ts[8-13] (optional: extend to cover `/list/...` behavior)
- index.js[5-5]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.