Skip to content

fix(dashboard): Filter dashboard extensions to plugins active in runtime config#4732

Open
grolmus wants to merge 1 commit into
vendurehq:masterfrom
grolmus:fix/oss-522-dashboard-extension-filter-active-plugins
Open

fix(dashboard): Filter dashboard extensions to plugins active in runtime config#4732
grolmus wants to merge 1 commit into
vendurehq:masterfrom
grolmus:fix/oss-522-dashboard-extension-filter-active-plugins

Conversation

@grolmus
Copy link
Copy Markdown
Collaborator

@grolmus grolmus commented May 13, 2026

Summary

The dashboard's Vite plugin discovery walks the import graph from vendure-config.ts and additionally expands package.json dependencies of every imported package. Any plugin reachable through these channels has its dashboard extension bundled and registered in the dashboard runtime — even when the plugin is not present in the actual plugins array passed to bootstrap().

This breaks any pattern where plugins are conditionally added to the runtime plugins array (env-driven, feature-flagged, wrapper/meta-package orchestrators, multi-tenant setups with a shared bundle). The dashboard ends up loading nav items, routes and page blocks for plugins whose server-side resolvers, GraphQL types and services are absent at runtime — causing broken nav items and extension code that crashes on load.

Solution

Add a shared filterActivePluginInfo(pluginInfo, vendureConfig) helper in packages/dashboard/vite/utils/get-active-plugin-info.ts that intersects the statically discovered PluginInfo[] against VendureConfig.plugins[]. Apply it before every consumer that emits dashboard artefacts:

Consumer What it emits Now filtered?
vite-plugin-dashboard-metadata.ts runDashboardExtensions() virtual module
vite-plugin-translations.ts (load + generateBundle) merged plugin .po catalogs
vite-plugin-tailwind-source.ts @source directives for Tailwind v4 scanning
vite-plugin-lingui-babel.ts allowed node_modules packages for Lingui macro transform

The helper handles both forms a VendureConfig.plugins[] entry can take:

  • a class with @VendurePlugin (including the return value of SomePlugin.init(opts), which by convention returns the class itself)
  • a NestJS DynamicModule of shape { module: SomePluginClass, ... }

Matching is done by class name because the runtime config and static discovery load plugin modules through different import paths and therefore see distinct class objects. The discovery step already keys on name, so this filter introduces no new ambiguity — the limitation is documented in the helper's doc comment together with a note about how to upgrade to (pluginPath, name) matching if needed.

The discovery log in vite-plugin-config-loader.ts is intentionally left untouched — it still reports what discovery found, which is the right thing for users debugging why their plugin is or isn't picked up.

Fixes #4706

Test plan

  • Unit tests for filterActivePluginInfo in packages/dashboard/vite/tests/plugin-hooks.spec.ts — 5 new tests covering: class-name match, empty plugins array, missing plugins key, NestJS DynamicModule unwrap, ignoring entries without resolvable class name.
  • Consumer behaviour tests — new filter-specific tests added to the dashboardMetadataPlugin and dashboardTailwindSourcePlugin describe blocks, asserting that only the active plugin's extension path appears in the generated runDashboardExtensions() and @source '...' lines respectively. The existing tests in those blocks were preserved unchanged by updating the fake config loader factory to auto-build a matching VendureConfig.plugins array from the supplied pluginInfo.
  • End-to-end fixture test in packages/dashboard/vite/tests/meta-package.spec.ts — exercises the real compile() pipeline against the existing meta-package fixture (MetaPlugin.init() returns [ChildPluginA, ChildPluginB], while transitive package.json dependency expansion also discovers ChildPluginC). Asserts that discovery still finds all 3 (no regression) but filterActivePluginInfo drops ChildPluginC — i.e. the exact bug scenario from Dashboard discovers and bundles extensions for plugins not present in runtime config #4706 is now caught.
  • npm test from packages/dashboard357 tests pass (53 in plugin-hooks.spec.ts, 3 in meta-package.spec.ts).
  • npm run check-types — clean.
  • npm run build:vite — clean.
  • Pre-commit lint (lint-staged on the changed files) — clean.

Notes for reviewers

  • This is a behavioural change visible to anyone whose dashboard was inadvertently displaying extensions for non-active plugins; for the documented bug pattern (Dashboard discovers and bundles extensions for plugins not present in runtime config #4706), the new behaviour matches user expectations. There is no documented case where users would want the previous behaviour, and the helper docstring outlines an escape hatch if one ever materialises.
  • The filter is called once per consumer plugin per build; it's an O(n) operation over a small set, so memoisation isn't worth the indirection at this stage.

View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

The dashboard's Vite plugin discovery walks the import graph from
`vendure-config.ts` and additionally expands `package.json` dependencies
of every imported package. Dashboard extensions, translations and
Tailwind sources were therefore bundled for plugins that were only
statically reachable but not actually present in the runtime
`VendureConfig.plugins` array (e.g. when plugins are conditionally
included based on options, env vars or feature flags, or come in via
wrapper/meta-packages).

Add a shared `filterActivePluginInfo` helper that intersects the
statically discovered `PluginInfo[]` with the plugin classes in
`vendureConfig.plugins`, and apply it in all four consumers:
`vite-plugin-dashboard-metadata`, `vite-plugin-translations`,
`vite-plugin-tailwind-source`, `vite-plugin-lingui-babel`.

Fixes vendurehq#4706
@vercel
Copy link
Copy Markdown

vercel Bot commented May 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
vendure-storybook Ready Ready Preview, Comment May 13, 2026 7:49am

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 21c4ca86-671d-4ce6-9266-0a45c983c035

📥 Commits

Reviewing files that changed from the base of the PR and between 563e6b3 and 18ff5b4.

📒 Files selected for processing (7)
  • packages/dashboard/vite/tests/meta-package.spec.ts
  • packages/dashboard/vite/tests/plugin-hooks.spec.ts
  • packages/dashboard/vite/utils/get-active-plugin-info.ts
  • packages/dashboard/vite/vite-plugin-dashboard-metadata.ts
  • packages/dashboard/vite/vite-plugin-lingui-babel.ts
  • packages/dashboard/vite/vite-plugin-tailwind-source.ts
  • packages/dashboard/vite/vite-plugin-translations.ts

📝 Walkthrough

Walkthrough

This PR implements runtime plugin filtering to ensure only explicitly configured plugins contribute bundled resources. A new filterActivePluginInfo utility extracts active plugin class names from VendureConfig.plugins and filters discovered PluginInfo accordingly. The filtering is applied across five Vite build plugins: dashboard metadata, Tailwind source, Lingui babel, translations, and the build pipeline itself. Test infrastructure is updated to support selective plugin activation, and new test cases validate filtering behavior for each plugin.

Possibly related issues

  • vendurehq/vendure#4706: Directly addresses the issue by implementing filterActivePluginInfo and applying it across the Vite plugins to prevent bundling inactive plugin resources.

Possibly related PRs

  • vendurehq/vendure#4182: Prior work on determining eligible third-party packages for Lingui macro transformation; this PR extends that filtering mechanism across additional Vite plugins.

Suggested reviewers

  • michaelbromley
  • biggamesmallworld
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: filtering dashboard extensions to only include plugins active in the runtime configuration.
Description check ✅ Passed The description is comprehensive and covers all required template sections including summary, breaking changes context, test plan, and checklist acknowledgment.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@michaelbromley
Copy link
Copy Markdown
Member

Hey @grolmus — the fix looks solid (filter is applied consistently, tests are thorough, the class-name-matching trade-off is well documented). One thing worth thinking about before/after merge: the current shape of getActivePluginNames works perfectly for the documented plugin patterns, but a handful of less common plugins array entries will be silently dropped from the active set, causing the dashboard extension to be absent without any warning.

Not blocking — just worth either documenting or guarding against. Cases I can think of:

Silent false-negatives (plugin active at runtime, extension stripped from bundle)

  1. Anonymous classes

    plugins: [class extends VendurePlugin { /* ... */ }]

    .name === '', so the entry never makes it into the Set.

  2. Factory-generated plugins that don't set .name

    function createPluginFor(kind: string) {
        return class { static kind = kind; }; // anonymous
    }
    plugins: [createPluginFor('Stripe')];

    The returned class has .name === ''. Factories like this need an explicit Object.defineProperty(Plugin, 'name', { value: 'StripePlugin' }) to be recognised.

  3. Promise- or instance-wrapped entries

    plugins: [Promise.resolve(StripePlugin)]   // typeof === 'object', no .module
    plugins: [new EmailPlugin()]               // instance, not a constructor

    Neither matches typeof === 'function' nor { module: ... }, so they're skipped.

  4. Class decorators returning a wrapping class
    If @VendurePlugin (or any meta-decorator layered on top) is ever changed to return class extends target { ... } instead of mutating the original class, the wrapper is anonymous and every plugin gets filtered out across every project. Modern TS decorators usually mutate in place so this is latent, but it's worth a regression test that locks the contract.

Silent false-positives (rare)

  1. Same-class-name collisions across packages — two packages both exporting class BillingPlugin are indistinguishable. The active one keeps its extension correctly, but the inactive one also slips through. Already acknowledged in the docstring; just noting it stays a known limitation.

Environmental

  1. Build-time vs runtime env mismatch — if the config reads process.env to decide which plugins to include, the dashboard build sees the build-time env, not the eventual server runtime env. Not this PR's bug, but a new failure mode the filter makes more visible (extensions and backend plugins can now diverge if envs differ).

Suggestion (one-liner, non-blocking)

The cheapest mitigation for cases 1–4 is a single warning log next to the silent drop:

const name = pluginClass?.name;
if (name) {
    names.add(name);
} else {
    // Don't have access to Vite's logger here, but even a console.warn
    // converts \"silent broken UI\" into \"check the build log\".
    console.warn(
        '[dashboard] Could not derive a class name for a plugins[] entry; ' +
        'its dashboard extension (if any) will not be bundled. ' +
        'Ensure plugin entries are named classes or DynamicModules with a named .module.'
    );
}

Or even better, plumb a Vite logger through filterActivePluginInfo so the warning shows up in the standard build output.

Either way, none of this blocks the merge — it's a real improvement over the status quo. Mostly flagging it so the failure modes are documented somewhere durable if you don't want to add the warning right now. Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dashboard discovers and bundles extensions for plugins not present in runtime config

2 participants