chore: remove unnecessary fs dependency from example package.json files - #654
Closed
Harsh-H-Shah wants to merge 1 commit into
Closed
Conversation
The npm package literally named "fs" is not Node's built-in fs module — it's npm's official security-holding placeholder for a name no longer in use (per the registry: "This package name is not currently in use..."). It resolves to a do-nothing stub and was never imported by any of these examples' code. Node's built-in fs/node:fs module (used by several of these examples to read keypair fixtures in tests) is compiled into the Node runtime and is unaffected by this change, since built-ins never resolve through node_modules. Verified locally across all 22 affected examples: pnpm install succeeds in every one, and cargo build-sbf + test execution passes for all examples this repo's CI currently tracks.
Greptile SummaryRemoves the unused
Confidence Score: 5/5The PR appears safe to merge because the removed package is unused and Node built-in The manifest and lockfile changes consistently remove an inert registry placeholder, while affected source code that performs filesystem operations imports Node’s built-in Important Files Changed
Reviews (1): Last reviewed commit: "chore: remove unnecessary fs dependency ..." | Re-trigger Greptile |
Collaborator
|
Thanks @Harsh-H-Shah — closing as superseded by #656 ( |
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.
Summary
22 example
package.jsonfiles list"fs": "^0.0.1-security"as a dependency. This isn't Node's built-infsmodule — it's npm's official security-holding placeholder for a package name no longer in use. Per the registry:It resolves to a do-nothing stub and was never imported by any of these examples' source code — likely leftover from an old template that was copy-pasted forward.
Node's built-in
fs/node:fsmodule — which several of these examples do use in their tests to read keypair fixtures off disk — is compiled into the Node runtime and resolves independently ofnode_modules, so it's completely unaffected by removing this npm entry.Changes
fsline fromdependenciesin 22package.jsonfiles (basics/andtokens/, across native/pinocchio/asm variants)pnpm-lock.yamlfor each (pure removal — verified each lockfile diff is exactly the 8-linefspackage entry, nothing else)Test plan
Verified locally across all 22 affected examples:
pnpm installsucceeds in all 22cargo build-sbf+ full test execution passes for all 12 examples this repo's CI actively tracks (not in.github/.ghaignore)tokens/create-token/nativeandtokens/pda-mint-authority/native) show the same pre-existing behavior asmain; confirmed viagit diff main -- Cargo.toml Cargo.lockthat this PR touches neitherfs.readFileSync, 5 ran their full test suite successfully post-change (the other 2 hit the pre-existing, unrelated Cargo workspace issue before reaching that code path)grepconfirms no remaining"fs":references in anypackage.jsonin the repo