Repository navigation
fix(module_loader)!: load only the plugin holding a required module - #53
Merged
Merged
Conversation
With one pending lazy plugin whose lua/ has a module's top-level name,
a require of any module under that name loaded the plugin, whether it
held the module or not. An optional probe at startup turned lazy plugins
eager: pcall(require, 'mini.icons'), as snacks and which-key do, loaded
and configured { 'echasnovski/mini.pairs', event = 'InsertEnter' }, and
pcall(require, 'nvim-treesitter.configs') loaded nvim-treesitter's main
branch, which has no configs.lua. 2.x did the same, for any number of
claimants.
The fallback came in with the shared-namespace fix to keep 2.x's
behaviour for a sole claimant; no plugin was found that needs it. It
didn't serve a module a plugin/ script puts in package.preload either:
the loader hands back only package.loaded or the plugin's file, so that
first require failed anyway. A module brought in by an update is still
found, since the lookup cache resets on PackChanged.
As in lazy.nvim, whose loader loads a plugin only when Cache.find finds
the module's file, a require now loads the first pending plugin holding
it (.lua, /init.lua, .so or .dll) and otherwise fails with the usual
"module not found", the same rule as for several claimants. A plugin
that makes a module at load time needs a trigger of its own.
Refs zpack_nvim-50v, zpack_nvim-8n1
require('lspconfig/util'), still written in nvim-lspconfig's own
configs, never loaded a pending plugin holding lua/lspconfig/util.lua:
the top-level name came out as 'lspconfig/util', which no plugin
claims. 2.x did the same. As lazy.nvim's Cache.find and vim.loader.find
do, turn '/' into '.' and drop leading dots before the lookup.
The top-level name may now be empty, so require('') and require('/')
fail with "module not found" rather than an error inside the loader.
…g trigger's other triggers
prune and resolve_all each set entry.has_opts by hand after their own merge, and fold's re-merge left it to resolve_all's later loop. Any new re-merge that forgot the line would leave a stale has_opts, and setup() would run or skip on specs that are gone. Every merge now sets it.
…ders
When two pending plugins hold a required module's file, load the one
whose normalised name matches the top-level module, else the first
listed. lazy.nvim's get_unloaded_rtp puts that plugin first, so
require('mini.ai') with mini.nvim and a standalone mini.ai both pending
loads mini.nvim in either order. zpack took the first in spec order.
Once a first holder is found, only plugins named like the module are
scanned, so a hit still lists no other plugin's lua/ directory.
…quire threw When a lazy plugin's own opts or config requires its module and the module throws, require leaves its loading sentinel in package.loaded. The loader handed that back, so the first require and every later one returned true. Fall through to the plugin's file instead, as lazy.nvim does, so the require raises the error and later ones give "loop or previous error". A module that loaded is still returned as is, whatever its type, so a non-table module isn't run twice.
…wording On CI, leafo/gh-actions-lua installs Lua into ./.lua, so Lua's own ./?.lua searcher opens that directory for '' and '/' and reports "error loading module" rather than "not found". Assert what 5a2e0a6 fixed instead: the error doesn't come from inside zpack.
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.
Title: fix(module_loader)!: load only the plugin holding a required module
Found in a post-merge review of #52. For 3.0 (#48).
What changed: a
requireloads a lazy plugin only when the plugin holds the module's file underlua/(.lua,/init.lua, or a compiled.so/.dll). If no pending plugin holds it, nothing loads and therequirefails with Lua's usual "module 'x' not found". Before this, when only one pending plugin had the module's top-level name underlua/, anyrequireunder that name loaded it, whether the module was there or not. The change deletes that fallback (net -7 lines inmodule_loader.lua).Why: lazy plu 8000 gins turning eager is zpack's main startup risk, and this fallback did it on ordinary optional-module checks:
{ 'echasnovski/mini.pairs', event = 'InsertEnter', opts = {} }pluspcall(require, 'mini.icons')at startup, as snacks and which-key do: mini.pairs loaded at startup and itssetup()ran.{ 'nvim-treesitter/nvim-treesitter', event = 'BufReadPost' }on the main branch, which has noconfigs.lua, pluspcall(require, 'nvim-treesitter.configs'): nvim-treesitter loaded at startup.lazy.nvim's
loader.luaM.loaderloads a plugin only whenCache.findfinds the module's file (cache.lua:lsmod(path)[topmod], then a stat of/lua/<mod>.luaor/init.lua).docs/tips.mdalready said "As in lazy.nvim", which wasn't true for this case. The same happens at 85ba638 and at 2.0.4 (ec09994), where any pending plugin with the top-level name loaded, so this is old behaviour, not a #52 regression.Why the fallback was there: it came in with #52's shared-namespace fix (e3ace75: "a sole claimant still loads for a module it doesn't hold") to keep 2.x's behaviour when only one plugin claims the name. No real plugin was named as needing it, and the ledger (zpack_nvim-8n1 (f)) already applied the lazy.nvim rule to several claimants. The cases it could serve:
plugin/script puts inpackage.preload: it didn't actually help. The loader returns onlypackage.loaded[mod]or the plugin's own file, so the firstrequirefailed anyway. perf: stop tracing zpack's startup code #52 already says such a plugin needs a trigger of its own.PackChangedresets the lookup cache and the file is stat'ed fresh (pinned by "a top-level module an in-session update brings to a pending plugin").Behaviour change: a lazy plugin whose
lua/has only a module's top-level name no longer loads when that module is required, and therequirefails. Its own triggers and its own modules still load it. A plugin that creates a module at load time needs a trigger.docs/tips.md"Upgrading to 3.0" and:h zpack-Spec.modulesay so.Also from the review loop:
require('lspconfig/util'), still written in nvim-lspconfig's own configs, now loads the pending plugin holdinglua/lspconfig/util.lua. 2.x and 3.0 so far never did. As lazy.nvim'sCache.findandvim.loader.finddo,/becomes.and leading dots go before the lookup.require('')fails with "module '' not found" again instead of an error inside the loader (from perf: stop tracing zpack's startup code #52).get_unloaded_rtp:require('mini.ai')with both mini.nvim and a standalone mini.ai pending loads mini.nvim. 3.0 so far took the first in spec order, and 2.x took mini.ai, named like the whole module.optsorconfigrequires it, a laterrequireraises that error, then "loop or previous error", as in lazy.nvim. It returnedtrue, so another plugin'spcall(require, …)probe took the broken plugin for present and indexed a boolean. Same in 2.x.has_optsis derived inmerge_entry, not by hand after two of its three callers.optionalplugin'sdependenciesandspecsno longer count" now namesoptionalparents. LazyVim's lang extras have that shape (an optional nvim-lint carrying mason'sensure_installed) but lose nothing: 2.x never added that entry, and 3.0 adds it when nvim-lint is listed withoutoptional. The throwing-trigger entry now says the plugin's other triggers or arequirestill load it, not only arequire.Tests: "still loads a sole claimant for a submodule it doesn't hold" pinned the old rule. It and "loads what's left once another trigger has loaded the other claimant" are replaced by real-process tests of the new rule:
mini.iconsand nvim-treesitter main +nvim-treesitter.configsshapes, both required from an eager plugin'sconfigat startup: the plugin stayspending,pcallreturns false with "module '…' not found:", and mini.pairs still loads and runssetup()onInsertEnter.init.luaand a submodule.Putting the fallback back fails three of them. The other shared-namespace tests are unchanged.
Checks:
zpack-lazyvim-compare, 936649e vs this branch, SEEDS=40: structural=0, opts=0 on all-extras, and no subset lines differ. My config, run throughfilemode with{ import = 'plugins' }: 55 plugins, 33 lazy, 14 edges, identical. The lazy.nvim oracle shows only the harness's<expr>placeholders. This harness doesn't runrequires, so no difference was expected.real2.py, 5 probes each): every plugin's status, 'runtimepath',package.loaded, globals, autocmds, commands and messages are identical, base vs branch. That's expected: the namespaces this config shares,lualineandmini, belong to plugins that are already loaded at startup, so the fallback never fired here.BEGIN_COMMIT_OVERRIDE
fix(module_loader)!: a
requireloads a lazy plugin only when the plugin holds the module's file, as in lazy.nvim; one with only the top-level name stays lazyfix(module_loader): a slash-form
require('lspconfig/util')loads the pending plugin holding the modulefix(module_loader): when several pending plugins hold a module, the one named like it loads, as in lazy.nvim
fix(module_loader): a later
requireof a module whose load threw raises that error instead of returningtrueEND_COMMIT_OVERRIDE