fix: resolve profile-installed plugins from cordis-plugin-loader - #83
Open
yaojin3616 wants to merge 3 commits into
Open
fix: resolve profile-installed plugins from cordis-plugin-loader#83yaojin3616 wants to merge 3 commits into
yaojin3616 wants to merge 3 commits into
Conversation
Plugins installed via the plugin market (dsh-market) go into the profile's node_modules (e.g. DSH_HOME/profiles/web/node_modules), but the cordis-plugin-loader resolves packages from the app bundle's own node_modules directory. After packaging, these two paths are different, so both hot-mount and post-restart plugin loading fail with 'Cannot find package' errors. This change injects the profile's node_modules directories into the harness Node.js process via two mechanisms: 1. NODE_PATH + Module.globalPaths for CommonJS resolution 2. A custom ESM resolve hook (registered via --import) that falls back to profile node_modules when the default resolution fails The ESM hook uses a synthetic parent URL inside each profile's node_modules directory so that Node's standard resolver (including package.json exports, conditions, subpaths) handles the actual resolution correctly. Closes #73
Profile (project-level) packages now take priority during both ESM and CJS resolution, with the app bundle acting as fallback — matching Node's local-first resolution semantics. - ESM resolver tries synthetic profile parents before default resolution - CJS patches Module._resolveFilename to try profile paths first - Add priority and fallback tests, including a CJS child-process test
Profile-first resolution broke more than it fixed. Node's nextResolve merges the context it is handed into the shared context object, so once the ESM hook had tried a synthetic profile parent, the fallback resolved from that anchor instead of the real importer — every bare import the app bundle owns failed with ERR_MODULE_NOT_FOUND as soon as any profile had a node_modules directory. Profile-first also let a plugin's copy of a shared package replace the app bundle's own nested dependency, and discarded the paths callers pass to require.resolve. Default resolution now runs first and profiles only fill in what the app bundle cannot provide, which is all the original bug needs: the plugin loader ships in the bundle and cannot see DSH_HOME/profiles/<name>/node_modules. Plugins keep their pinned dependency versions either way, because the market installs them with pnpm and their deps live beside their real path in .pnpm. - Pass the preload to --import as a file URL. A bare Windows path parses as a `c:` URL scheme, so Node exited before the harness entry ran and the app could not start at all on Windows. - Skip the preload when the shim is missing instead of turning a missing best-effort resource into an unexplained startup crash. - Look up profile directories on every failed resolution rather than snapshotting them at startup, so the first plugin installed into a fresh profile mounts without a restart. - Inject only the profile the harness runs, named by DSH_DESKTOP_PROFILES, instead of every directory under DSH_HOME/profiles in readdir order. - Report the importer's original error when no profile provides the package, and surface exports/package-config errors from a profile copy instead of hiding them behind the next candidate. - Drop the NODE_PATH mutation: it has no effect on the running process and leaked into every child the harness spawns. Rewrites the ESM tests as real child-process integration tests. The previous ones mocked nextResolve, which is why they passed against a resolver that could not resolve the app bundle at all; 7 of the new tests fail against the previous implementation.
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.
Fixes #73
Root cause
Plugins installed via the plugin market go into the profile's
node_modules(e.g.DSH_HOME/profiles/web/node_modules), but thecordis-plugin-loaderresolves packages from the app bundle's ownnode_modulesdirectory. After Electron packaging, these two paths are different, so both hot-mount and post-restart plugin loading fail withCannot find packageerrors.The key error from user logs:
Fix
Injects the profile's
node_modulesdirectories into the harness Node.js process via two mechanisms:NODE_PATHand appends toModule.globalPaths--import) that falls back to profilenode_moduleswhen default resolution failsThe ESM hook uses a synthetic parent URL inside each profile's
node_modulesdirectory so that Node's standard resolver — includingpackage.jsonexports, conditions, and subpaths — handles the actual resolution correctly.Files changed
build/profile-module-paths.mjs— Setup script: collects profilenode_modulesdirs fromDSH_HOME, configures CJS paths, registers ESM hookbuild/profile-esm-resolver.mjs— ESM resolve hook with synthetic parent URL fallbacksrc/main/runtime/harness-runtime.ts— Passes--importflag andDSH_DESKTOP_PROFILE_MODULE_PATHSenv to harness processsrc/main/index.ts— Wires profile module paths resource into HarnessRuntimepackage.json— Adds new build files toextraResourcestest/runtime.test.ts— Updated tests for new argumentstest/profile-module-paths.test.ts— Tests for ESM resolver behaviorResolution priority
Profile (project-level)
node_modulestake priority over the app bundle, matching Node's local-first resolution semantics:node_modulesfirst (via synthetic parent URL), then falls back to default resolutionModule._resolveFilenameis wrapped to try profile paths first viaoptions.paths, then falls back to default resolutionThis means a plugin can pin its own dependency version in the profile without being shadowed by a different version bundled with the app.