Skip to content

fix(config): avoid dedup collisions for file:// plugins in node_modules#15598

Open
cyberprophet wants to merge 2 commits intoanomalyco:devfrom
cyberprophet:fix/plugin-file-url-dedup-15591
Open

fix(config): avoid dedup collisions for file:// plugins in node_modules#15598
cyberprophet wants to merge 2 commits intoanomalyco:devfrom
cyberprophet:fix/plugin-file-url-dedup-15591

Conversation

@cyberprophet
Copy link

@cyberprophet cyberprophet commented Mar 1, 2026

Issue for this PR

Closes #15591
Fixes #8759
Fixes #10115
Fixes #11159
Fixes #12285
Fixes #14304

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Config.getPluginName() previously identified file:// plugins by filename only (e.g. index).
When npm plugins are resolved to .../node_modules/<pkg>/dist/index.js, different plugins collapsed to the same identity and only the last plugin survived deduplication.

This PR updates plugin identity extraction to:

  • detect node_modules file URLs and use package identity (foo, @scope/pkg), not filename identity
  • keep existing filename fallback for non-node_modules local plugins
  • handle parsing in a cross-platform way so Windows test runners don't throw on synthetic POSIX-style file URLs

This directly matches the reproduction pattern reported in #10115 (order-dependent "only last plugin" behavior).

How did you verify your code works?

  • bun test test/config/config.test.ts --test-name-pattern "extracts name from file:// URL|prefers local file over npm package with same name|extracts package name from node_modules file:// URL|keeps multiple npm plugins resolved as file:// URLs"
  • bun run typecheck

Also addressed CI feedback:

  • Updated PR description to match required template sections (issue-compliance action)
  • Fixed Windows CI failures caused by fileURLToPath() throwing for POSIX-like file URLs in tests

Screenshots / recordings

N/A (non-UI change)

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Normalize file:// plugin names from node_modules paths to package names so multiple plugins with index.js entrypoints can co-exist.

Adds regression tests for package-name extraction and multi-plugin dedup behavior.
@github-actions github-actions bot added the needs:compliance This means the issue will auto-close after 2 hours. label Mar 1, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

The following comment was made by an LLM, it may be inaccurate:

I found one potentially related PR:

PR #8758: "fix: use full URL for file:// plugin deduplication"
#8758

This PR appears to address a similar issue with file:// plugin deduplication. It may be related to or a predecessor of the current PR #15598, as both deal with preventing false deduplication collisions for file:// plugins. However, PR #15598 specifically targets the Config.getPluginName() function and package identity extraction from node_modules, which may be a more targeted fix or a different approach to the same problem.

You should verify if PR #8758 is already merged or if the current PR is intended to supersede it.

Avoid fileURLToPath crashes on Windows for synthetic POSIX-style file URLs used in tests and fallback parsing. Keep node_modules package-name extraction for deduping plugins with index.js entrypoints.
@github-actions github-actions bot removed the needs:compliance This means the issue will auto-close after 2 hours. label Mar 1, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

Thanks for updating your PR! It now meets our contributing guidelines. 👍

@cyberprophet
Copy link
Author

Addressed the automation feedback in this update:

Latest commit: 0ee3a2e

@cyberprophet
Copy link
Author

@rekram1-node This fixes the getPluginName filename collision bug that's been reported across multiple issues (#8759, #10115, #11159, #12285, #14304). All CI checks pass. Would appreciate a review when you have a moment.

@cyberprophet
Copy link
Author

@thdxr This is a 16-line fix for a bug in getPluginName() that has 6 open issues (#8759, #10115, #11159, #12285, #14304, #15591) and 3 competing PRs (#8758, #11161, this one).

The bug: import.meta.resolve rewrites npm plugin specifiers to file://…/node_modules/<pkg>/dist/index.js. Then getPluginName() extracts only the filename "index", colliding with any project plugin that also has an index.ts entry point. The result: plugins silently disappear at runtime with no error.

This fix: parse the package name from node_modules paths — pure string manipulation, zero fs I/O, no new dependencies. All CI green (Linux + Windows).

Would appreciate a look when you get a chance.

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