Skip to content

fix: replace find-up dependency with native @libs/find-up helper#2722

Open
tolzhabayev wants to merge 3 commits into
mainfrom
fix/remove-find-up
Open

fix: replace find-up dependency with native @libs/find-up helper#2722
tolzhabayev wants to merge 3 commits into
mainfrom
fix/remove-find-up

Conversation

@tolzhabayev

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:

Removes the find-up runtime dependency from @grafana/create-plugin and @grafana/sign-plugin (introduced in #1597) by replacing it with a ~20 line native helper in a new private @libs/find-up workspace.

find-up was only used for two synchronous, files-only lookups:

  • @libs/version — walking up from the bundled module to find the consuming package's package.json (this is why both CLIs carried find-up in dependencies: the lib is bundled into their dist while imports of published packages stay external).
  • create-plugin's utils.packageManager.ts — finding the closest lockfile to detect the package manager.

The native helper preserves the two find-up semantics that matter for these call sites: all candidate names are checked in each directory before moving up (lockfile precedence), and the walk stops at the filesystem root. It is covered by 7 unit tests.

With this change both CLIs bundle the helper directly, so find-up and its transitive dependencies (locate-path, p-locate, yocto-queue, unicorn-magic) disappear from the published packages' install footprint. The remaining find-up entries in package-lock.json are unrelated transitive dependencies of the docusaurus website tooling.

Which issue(s) this PR fixes:

n/a

Special notes for your reviewer:

  • Verified in the built output: dist no longer contains any import ... from 'find-up'; @libs/version and utils.packageManager now import the bundled helper via relative paths.
  • libs/version previously declared find-up as a ^7.0.0 peerDependency while consumers shipped 8.0.0 — that drift goes away too.
  • The fix: prefix means release-please will cut patch releases for both CLIs, which seems right for a change to the published artifacts.

The find-up package was only used for two synchronous, files-only
lookups: finding the consuming package's package.json in @libs/version
and finding the closest lockfile in create-plugin. A ~20 line native
walk-up helper in a new private @libs/find-up workspace covers both,
preserving find-up's per-directory name precedence.

Since @libs/version is bundled into consumers while its imports of
published packages stay external, dropping find-up there removes the
runtime dependency from both create-plugin and sign-plugin.
@tolzhabayev tolzhabayev self-assigned this Jun 11, 2026
@tolzhabayev tolzhabayev requested a review from jackw June 11, 2026 09:34
@tolzhabayev tolzhabayev moved this from 📬 Triage to 🔬 In review in Grafana Catalog Team Jun 11, 2026
@tolzhabayev tolzhabayev marked this pull request as ready for review June 11, 2026 13:31
@tolzhabayev tolzhabayev requested review from a team as code owners June 11, 2026 13:31
mckn
mckn previously requested changes Jun 16, 2026

@mckn mckn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition! Overall it looks great but there is one change you need to apply to be consistent with the original package:

The original find-up package delegates to locate-path, which defaults to type: 'file' and checks stat.isFile() using statSync — so it only matches files, not directories, by default.

The @libs/find-up implementation currently uses existsSync, which matches both files and directories. To mirror the original behavior, existsSync should be replaced with a statSync + .isFile() check, exactly as locate-path does.

@github-project-automation github-project-automation Bot moved this from 🔬 In review to 🧑‍💻 In development in Grafana Catalog Team Jun 16, 2026
@tolzhabayev tolzhabayev requested a review from mckn June 16, 2026 12:55
@tolzhabayev

Copy link
Copy Markdown
Collaborator Author

Nice addition! Overall it looks great but there is one change you need to apply to be consistent with the original package:

The original find-up package delegates to locate-path, which defaults to type: 'file' and checks stat.isFile() using statSync — so it only matches files, not directories, by default.

The @libs/find-up implementation currently uses existsSync, which matches both files and directories. To mirror the original behavior, existsSync should be replaced with a statSync + .isFile() check, exactly as locate-path does.

Good point, changed it here
5c1d071

@mckn mckn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! The only thing we should change is to sync the version pinning across the consumers. I think the "^1.0.0" is okay here since this is a library living in the same workspace but I guess both works.

Minor — version pin inconsistency:
// create-plugin/package.json
"@libs/find-up": "1.0.0" // exact

// sign-plugin/package.json
"@libs/find-up": "^1.0.0" // range

// libs/version/package.json
"@libs/find-up": "^1.0.0" // range

@mckn mckn self-requested a review June 17, 2026 09:26
@tolzhabayev

Copy link
Copy Markdown
Collaborator Author

LGTM! The only thing we should change is to sync the version pinning across the consumers. I think the "^1.0.0" is okay here since this is a library living in the same workspace but I guess both works.

Minor — version pin inconsistency: // create-plugin/package.json "@libs/find-up": "1.0.0" // exact

// sign-plugin/package.json "@libs/find-up": "^1.0.0" // range

// libs/version/package.json "@libs/find-up": "^1.0.0" // range

it's following the existing pattern. Each consumer pins its @libs/* deps the same way: create-plugin uses exact (@libs/output 1.0.3, @libs/version 1.0.2), while sign-plugin and libs/version use ranges. I am fine either way but it would then make this lib follow a different convention.

@jackw jackw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

The only thing we should change is to sync the version pinning across the consumers.

We can discuss that in a future PR I think. The devDeps don't need to be pinned, it's the prod ones that we must pin.

@tolzhabayev

Copy link
Copy Markdown
Collaborator Author

@mckn need you to approve or remove request changes otherwise I can't merge

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

Labels

None yet

Projects

Status: 🧑‍💻 In development

Development

Successfully merging this pull request may close these issues.

3 participants