Skip to content

Add equivalent of yarn why - #9051

Closed
HurricanKai wants to merge 7 commits into
oven-sh:mainfrom
HurricanKai:main
Closed

HurricanKai wants to merge 7 commits into
oven-sh:mainfrom
HurricanKai:main

Conversation

@HurricanKai

@HurricanKai HurricanKai commented Feb 22, 2024 •

Copy link
Copy Markdown

What does this PR do?

Adds a subcommand bun why <package> this shows why a package is referenced. See also #692. This isn't 100% equivalent to yarn why but it fulfills some of it's purpose already & provides code to build on later.

  • Documentation or TypeScript types (it's okay to leave the rest blank in this case)
  • Code changes

How did you verify your code works?

I wrote automated tests (in-progress)

  • I checked the lifetime of memory allocated to verify it's (1) freed and (2) only freed when it should be
  • I included a test for the new code, or an existing test covers it
  • JSValue used outside outside of the stack is either wrapped in a JSC.Strong or is JSValueProtect'ed
  • I wrote TypeScript/JavaScript tests and they pass locally (bun-debug test test-file-name.test)

Are tests required for this kind of feature? Where to add them?

@jdalton jdalton changed the title bun why? Add equivalent of yarn why Feb 22, 2024
@HurricanKai

Copy link
Copy Markdown
Author

Looks like the CI failures are not due to code but some infra problem. Seems like tests are passing (which is expected as I have not modified any tested code, and not added tests for my new code).

No comment has been found with asked pattern. Creating a new comment.
Error: Resource not accessible by integration

@paperclover

Copy link
Copy Markdown
Contributor

exciting to see this being worked on, but it doesn't seem to work well

image

bun pm why with no arguments crashes.

i also think this should be bun why, as we currently hvae this subcommand reserved specifically for this feature.

image

(ls is in a similar situation here. it needs to work as bun ls unless we want top-level bun ls to do something else)

@paperclover

Copy link
Copy Markdown
Contributor

tests should be written, they can be done so in a new file. the tests would mostly use bun.spawn() to run the bun why command. look for uses of bunExe() and bunEnv and how theyre used to spawn instances of bun in test/cli/*

@paperclover paperclover left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in addition to the above:

Comment thread src/cli/package_manager_command.zig Outdated
@HurricanKai

Copy link
Copy Markdown
Author

Moved this into it's own command. Not sure if I've done it right - I've done it similar to bun pm, bun install, etc. so it goes through PackageManager. This isn't the most best solution imho, given a bunch of files that have little to do with this are involved.
Also had to fill out some functions/lookups in turn, please take care when reviewing if I've done it correctly, I simply had a guess.

Will look into changing it a bit so PackageManager isn't involved, and the command simply picks up the lockfile directly.

@HurricanKai

Copy link
Copy Markdown
Author

I've not found a way to do this - loading the Lockfile directly seems to lead to some unreachable code, and I'm not sure how to solve this.

@paperclover

Copy link
Copy Markdown
Contributor

i have done some light testing on this but i feel like it is not ready enough to be shipped as a real command. The blank "due to " is still present (seems to be certain lockfiles dont have name info of the root), and i think we can colorize the output and make it look.

i think it would be cool to draw a tree like pnpm does
image

i also like how yarn colorizes it, but i think the tree should start with the
image

though neither of these trees include the root, which i think is helpful information to add an extra line for. i noticed in your implementation the root is only shown if it is a single item (i think this is a bug in your usage of 0 to represent the root).

definetly on the right track overall but need alot more work. going to mark as draft.

@paperclover
paperclover marked this pull request as draft February 27, 2024 12:28
@arnorhs

arnorhs commented Jan 7, 2025

Copy link
Copy Markdown

I see that this PR has been closed, is there an alternative implementation of this that got added instead or is this looking for a new implementation? This is a very useful command in a monorepo, which I've used yarn for forever - to eg. see which packages are depending on a particular package, track why you have 2-3 versions of a package, which package depends on that version etc.. bun pm ls etc doesn't quite do it justice

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.

5 participants