feat(extension): native extension loader, ESM support and callback attribution - #5732
Open
chemzqm wants to merge 11 commits into
Open
feat(extension): native extension loader, ESM support and callback attribution#5732chemzqm wants to merge 11 commits into
chemzqm wants to merge 11 commits into
Conversation
Replace the VM sandbox extension loader with native Node.js require():
- ExtensionPathIndex maps module filenames to owning extensions with
longest-root match and symlink-safe real/logical path domains
- ExtensionApiFactory provides one stable top-level API object per
extension while keeping shared core services underneath
- CocModuleInterceptor maps require("coc.nvim") by importer ownership
through a process-wide Module._load wrapper, failing unknown callers
- ExtensionModuleLoader executes entries with native require() and
normalizes exports with the legacy semantics
- ExtensionModuleCache clears only extension-owned require.cache entries
with root safety guards
Manager now loads extensions natively by default; COC_EXTENSION_LOADER
=legacy remains as a temporary comparison switch until validation
finishes. Adds characterization and unit tests plus reload ownership
integration coverage.
Delete the vm.runInContext()/Module._compile based extension loader now that the native CommonJS loader is validated: - delete src/util/factory.ts and its sandbox tests - drop the COC_EXTENSION_LOADER comparison switch, native loading is the only production path - remove sandbox-specific coverage from modules-util tests and add load-retry and symlink-entry cases to the native loader tests Full suite passes with one production loader: native Node.js CommonJS semantics, no Module.prototype._compile patching, no vm-based extension execution.
Add a changelog entry for the native CommonJS loader and its behavior changes (shared realm, per-extension API identity, scoped reload cache).
Load ESM extension entries (".mjs" or package "type": "module")
through native import() and resolve import ... from "coc.nvim" by
importer ownership using process-wide node:module hooks:
- module hooks map "coc.nvim" to a per-owner virtual module; all ESM
files of one extension share one module instance and API object while
different extensions receive different objects, unknown importers fail
with the same diagnostic as the CommonJS interceptor
- normalizeExtensionExports accepts ESM namespaces: named activate,
default function, or default object with activate/deactivate
- extension module type is derived from the entry extension and the
package "type" field at registration time
- ESM reload re-activates the extension without re-executing module
code (restart required for code changes), with a warning logged
Adds hook unit tests, loader ESM cases, and a manager integration test
covering activation and cached reload behavior.
Close the remaining extension-loader migration plan items: - log extension id, entry, module type and load result from the native loader, plus activation success from the manager (§9.4) - register extension ownership before installing the interceptor (§8.1) - add reload failure-path tests: deactivate order before cache cleanup, cleanup continues when deactivate throws, failed reload activation leaves the extension inactive (§13.5) - add JSON require coverage (§13.1), async activate rejection (§5.2) and cross-realm error identity (§12.3) - validate coc-eslint (ESLint-style) and a real extension installed through a symlink, both activate and reload (§14)
The per-extension API object now wraps registration surfaces (commands.registerCommand/register, events.on, languages.register*) and tags the registered callback with the extension id. Diagnostics can therefore name the plugin without parsing stack traces: - command errors shown by Vim are prefixed with [extension: <id>] - event handler errors and slow-handler warnings include the extension id - provider errors logged through Manager.handleResults and completion timeouts/errors include the extension id - provider __extensionName is derived from the registration owner instead of the captured stack Registration methods keep their existing signatures; the extension id is attached to the callback object by the wrapper, and manager registration surfaces consume it from there.
Address review findings on the attribution work: - keep g:coc_timeout_sources as plain source names; the extension-attributed form only appears in the timeout log - widen attribution to events.once, the Command-object form of commands.register, workspace.onDid*/onWill* listeners, workspace keymap registration and registerBufferSync - look up path ownership per entry (real path then logical path) so deepest root still wins when symlink and logical roots interleave - fail with a clear owner diagnostic when an ESM importer has a non-file URL - correct the Module._load interceptor comment to describe per-manager installation and chaining Adds unit coverage for the newly wrapped surfaces.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #5732 +/- ##
========================================
Coverage 97.96% 97.97%
========================================
Files 297 302 +5
Lines 50174 50555 +381
Branches 8668 8768 +100
========================================
+ Hits 49155 49530 +375
- Misses 900 906 +6
Partials 119 119 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
Author
|
@codex review |
Replace the Module._load interceptor and ESM loader hooks with a single
process-wide node:module resolve hook that routes both require("coc.nvim")
and import ... from "coc.nvim" by importer ownership.
- Resolve module type from the nearest package.json for nested entries
- Prefer ESM named activate over a default export and preserve deactivate
- Retain ESM API identity across reload; only CJS extensions get a fresh API
- Keep prefixExtensionError working on frozen Error instances via cause
Add coverage for parentless requires, unknown importers, non-file importers and ESM named export filtering in moduleHook.ts.
- remove orphaned src/__tests__/helper.ts and unused vm export - merge duplicate waitImmediate, adjustRange, loadJson, escapeRegExp, wasm init and MCP tool schema fragments - drop export on helpers only used within their own file
- add unit tests for extension id tagging and error prefixing edge cases - cover apiFactory proxy wrap, method binding and onWill registration - cover moduleLoader ExtensionLoadError rethrow and ESM no-op activate - cover completion timeout attribution and synchronous provider throw - extract shared noop activate and workspace registration set so the previously unrecorded lines become reportable
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.
Summary
Migrate coc.nvim extension loading from the VM sandbox to the VS Code Extension Host model, then add native ESM support and per-extension callback error attribution.
Changes
Native CommonJS loader
require().vm.runInContext()execution,Module.prototype._compilepatching,Module.wrap()and the custom sandbox.require("coc.nvim")resolves by importing-module ownership:ExtensionPathIndexmaps module filenames to owning extensions (longest-root match, symlink-safe real/logical domains).ExtensionApiFactoryprovides one stable top-level API object per extension while keeping shared core services underneath.CocModuleInterceptorwrapsModule._loadonce per manager and fails unknown importers with a diagnostic error.ExtensionModuleLoadernormalizes exports with the legacy semantics and preserves original errors ascause.ExtensionModuleCacheclears only extension-ownedrequire.cacheentries, with root safety guards.Native ESM support
.mjsentries (or"type": "module"packages) load throughimport().import ... from "coc.nvim"resolves per importer throughnode:modulehooks.Callback error attribution
[extension: <id>]; event/provider errors and completion timeouts include the extension id in logs.__extensionNameis derived from the registration owner instead of parsing stack traces.Behavior changes
globalThis,process, constructors). The loader is intentionally not a security sandbox.require("coc.nvim")returns a per-extension top-level object; shared services such asworkspaceremain shared underneath (now wrapped per extension for attribution).Validation
tsc --noEmit, oxlint and the production build all pass.Notes
.codex/migration.mdfor the full migration plan and acceptance checklist.