fix(plugins): derive plugin id from parent dir for generic basenames (#596)#613
Merged
Conversation
…596) The near-universal '<name>/plugin.js' convention made every such file derive the id 'plugin' (basename minus extension), so the loader's duplicate-id guard refused to boot the moment you loaded two of them. It also blocked the '--plugin' CLI form entirely, which has no way to set an explicit id: jss start --plugin ./relay/plugin.js@/relay --plugin ./dashboard/plugin.js@/dashboard # Error: plugins: duplicate id 'plugin' Fix: when the basename is generic (plugin.js / index.js), derive the id from the immediate PARENT DIRECTORY ('relay/plugin.js' -> 'relay') instead. The parent dir distinguishes these files without pinning the machine-specific path prefix — the reason the basename (not the full path) was used in the first place, so pluginDir stays stable across deployments. Non-generic basenames, bare package specifiers, and explicit ids are unchanged. Tests: pluginId cases (generic -> parent, non-generic unchanged, no-parent fallback, explicit id wins) + an integration boot of two <name>/plugin.js files with no ids (the exact CLI case) that now succeeds with both routes answering. Full suite 1055/1055. docs/configuration.md updated.
There was a problem hiding this comment.
Pull request overview
This PR fixes a plugin-loader collision where conventionally named modules like <name>/plugin.js or <name>/index.js all previously derived the same plugin id (e.g., plugin), preventing multiple such plugins from being loaded (including via the --plugin CLI form). The loader now derives ids from the immediate parent directory when the module basename is generic, keeping ids stable without tying them to machine-specific path prefixes.
Changes:
- Add
fileStem()and a generic-basename fallback (plugin.js/index.js→ parent directory) for file-path modules when deriving plugin ids. - Extend plugin-loader tests to cover generic-basename id derivation and an integration case loading two
<name>/plugin.jsplugins without explicit ids. - Update configuration documentation to describe the new default id derivation behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/plugins.test.js | Adds unit + integration coverage for generic-basename id derivation and multi-plugin boot without explicit ids. |
| src/plugins.js | Implements parent-directory fallback for generic basenames via fileStem() and updates pluginId() to use it. |
| docs/configuration.md | Documents the updated default plugin id derivation rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
163
to
167
| ? spec.id | ||
| : (module.startsWith('.') || path.isAbsolute(module) | ||
| ? path.basename(module) | ||
| ? fileStem(module) | ||
| : module | ||
| ).replace(/\.[cm]?js$/, ''); |
…name Addresses the Copilot review note on #613: pluginId() stripped the extension AFTER fileStem(), so a parent directory whose own name ends in .js/.mjs/.cjs (e.g. 'foo.js/plugin.js') would lose it and derive 'foo' instead of 'foo-js' — collapsing 'foo.js/' and 'foo/' to the same id. Restructure so the extension is stripped where it applies (the file basename via fileStem, and a bare specifier's tail) and the parent directory name is used as-is. No change on any normal input; added tests for the .js-named-directory edge case.
melvincarvalho
added a commit
that referenced
this pull request
Jul 11, 2026
Plugin-loader id fix (#596, #613): a plugin loaded from a file path with a generic basename — the near-universal `<name>/plugin.js` (and `index.js`) — now derives its id from the immediate parent directory (`relay/plugin.js` -> `relay`) instead of the basename, which reduced every such file to the id `plugin` and collided. The parent directory distinguishes them without pinning the machine-specific path prefix, so pluginDir stays stable across deployments. This also unblocks the `--plugin` CLI flag, which has no field to set an explicit id: `jss start --plugin ./relay/plugin.js@/relay --plugin ./dashboard/plugin.js@/dashboard` now boots (ids `relay` and `dashboard`) rather than failing with `duplicate id 'plugin'`. Bare package specifiers, non-generic basenames, and explicit ids are unchanged.
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
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.
Closes #596.
The bug
The near-universal
<name>/plugin.jsconvention makes every such file derive the idplugin(basename minus extension), so the loader's (correct) duplicate-id guard refuses to boot the moment you load two of them. As noted on the issue, it also blocks the--pluginCLI form entirely — that flag has no field to set an explicitid:jss start --plugin ./relay/plugin.js@/relay --plugin ./dashboard/plugin.js@/dashboard # Error: plugins: duplicate id 'plugin' — set entry.id ...All 33 plugins in the out-of-tree plugins repo hit this; it's why they're loaded via explicit-
idconfig rather thanjss --plugin.The fix
When the basename is generic (
plugin.js/index.js), derive the id from the immediate parent directory (relay/plugin.js→relay) instead of the basename.The parent directory is the right fallback specifically because it distinguishes
<name>/plugin.jsfiles from each other without pinning the machine-specific path prefix — which is the exact reason the basename (not the full path) was used originally: a deployment-dependent prefix must not name the plugin's data dir, orpluginDirwould move with the deployment.relayis stable;/home/alice/apps/relayis not.Unchanged: non-generic basenames (
foo.js→foo), bare package specifiers (@scope/pkg/plugin.js→scope-pkg-plugin), and explicitids. The duplicate-id guard still fires for genuine collisions.Tests
pluginIdunit cases: generic → parent dir,index.js→ parent, uppercase/.mjshandling, non-generic unchanged, no-usable-parent fallback (./plugin.js→plugin), explicit id still wins.<name>/plugin.jsfiles with no ids (the exact CLI case) — it now boots and both routes answer. Existing duplicate-id rejection test still passes.docs/configuration.mdupdated.Context
Found while trying to run the out-of-tree
dashboard/plugin from the core CLI afterapi.plugins/api.serverInfoshipped (#605, #612) — the seams work, but this id collision was the last thing between "clone and--plugin" and a one-liner. With this, bare--plugin ./relay/plugin.jsderivesrelayand conventional layouts load without ceremony.