fix(pi-fff): isomorphic bun/node SDK lazy-load#669
Open
gustav-fff wants to merge 1 commit into
Open
Conversation
Collaborator
Author
|
[triage-bot] CI note: Honk-Honk 🪿 |
Detect bun vs node at runtime and dynamically import the matching SDK (@ff-labs/fff-bun on bun, @ff-labs/fff-node on node) via a variable package name so oh-my-pi's static extension validator does not chase ffi-rs' optional native binaries through the module graph. Closes #668
a265a46 to
fbdf9f8
Compare
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.
Closes #668
Root cause
packages/pi-fff/src/index.ts:21had a top-levelimport { FileFinder } from "@ff-labs/fff-node".@ff-labs/fff-nodepullsffi-rsand the optional per-platform native binary packages (@ff-labs/fff-bin-*) into its module graph. oh-my-pi statically resolves every extension import to validate the extension; the optional native package for a foreign platform is not on disk, so validation fails even though the runtime install would be fine.Reporter also flagged that pi runs under both bun and node — but pi-fff only ever loaded the node SDK, so bun hosts paid the ffi-rs cost instead of using
@ff-labs/fff-bunwhich shares the sameFileFinderApi.Fix
FileFindervalue import; keep@ff-labs/fff-nodefor type-only imports (erased at compile time, invisible to the static validator).detectRuntime()checksglobalThis.Bunandprocess.versions.bun.loadSdk()does a variable-nameawait import(pkg)for the runtime-appropriate SDK, with a fallback to the other one. Variable-name import defeats static resolvers.sdkPromiseso a single load survives concurrent tool calls.ensureFinder()nowawait loadSdk()beforeFileFinder.create(...); the existing in-flight-promise guard from pi-fff: parallel ffgrep calls can hang indefinitely due to unsynchronized ensureFinder singleton #403 still serializes creation.package.json: add@ff-labs/fff-bunas an optional peer dependency so bun-hosted pi pulls it in without forcing node-hosted pi to install it.Both SDKs implement the identical
FileFinderApi(packages/shared/fff-api.ts:528), so the call sites don't change.Steps to reproduce
Expected: extension loads under oh-my-pi.
Actual: validator errors on the unresolved optional native package.
How verified
cd packages/pi-fff && bun run typecheck— clean.bun test test/— 18/18 pass (added mock for@ff-labs/fff-bunalongside@ff-labs/fff-nodesince bun-hosted tests now prefer the bun SDK).bun run format— no changes.Not verified end-to-end against a real oh-my-pi install — that would need the pi validator in the loop. @dmtrKovalenko please smoke-test under omp before releasing.
Automated triage via Gustav. Honk-Honk 🪿