Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pnpm store usages <package>... #1529

Merged
merged 14 commits into from Dec 7, 2018
Merged

Conversation

nareddyt
Copy link
Contributor

@nareddyt nareddyt commented Dec 2, 2018

Closes #1527

Overview

Adds a new CLI command to find all pnpm projects that link to specific packages in the global store.

#1527 outlines the user experience, use case, and limitations. This PR implements the feature exactly as described.

Implementation Details

The commits below describe the changes made to each package to enable this feature.

When a user makes a query against pnpm store usages <query>, the query flows in the following format:

  1. pnpm user query via the CLI
  2. supi programmatic query against the store API or the store server
  3. package-store internal traversal of store to fulfill query

Testing Done

Added minimal tests to a few of the submodules. Plan on adding more tests after the code in src is approved.

Comments

I added a few comments to my PR about areas I'm unsure about.

- Add documentation for `pnpm store usages`
- Refactor docs for `pnpm store` to be more readable
Introduces functionality to search the package store for packages.
Reports all matching packages and the node projects they are used in.
Allows for searching a specific version, or all versions.
Exposes an API for finding package usages in the store.
…nality

Publicly exposes the findPackageUsages function in the store-controller.
Also exposes the expected response type, allowing clients to be type-safe.
Publicly exposes the findPackageUsages function SUPI.
Enabled CLI command to find usages of a package in the global store.
Pretty-prints a tree with queries and results.
packages/package-store/src/storeController/index.ts Outdated Show resolved Hide resolved
packages/package-store/src/storeController/index.ts Outdated Show resolved Hide resolved
packages/pnpm/src/cmd/help.ts Show resolved Hide resolved
packages/pnpm/package.json Show resolved Hide resolved
packages/supi/test/storeUsages.ts Outdated Show resolved Hide resolved
Removed semicolons to follow style guide.
@nareddyt nareddyt closed this Dec 3, 2018
@nareddyt nareddyt deleted the pnpm-store-package-info branch December 3, 2018 04:52
@nareddyt nareddyt reopened this Dec 3, 2018
@nareddyt nareddyt restored the pnpm-store-package-info branch December 3, 2018 04:53
…info

# Conflicts:
#	packages/pnpm/package.json
#	packages/pnpm/src/cmd/store.ts
#	packages/server/src/createServer.ts
pnpm#1528 was merged, modified test to use new SUPI API.
@nareddyt
Copy link
Contributor Author

nareddyt commented Dec 3, 2018

Everything should be merged and all issues should be fixed. I can add some more tests if this code structure looks good to you.

Could you take a look at the failing AppVeyor build? I'm not entirely sure what that error means, I don't think I touched that part of the project.

@zkochan
Copy link
Member

zkochan commented Dec 3, 2018

Could you take a look at the failing AppVeyor build? I'm not entirely sure what that error means, I don't think I touched that part of the project.

AppVeyor was failing on master as well. I fixed it

(For some reason tslint catches more issues on Windows. I don't know why)

@zkochan
Copy link
Member

zkochan commented Dec 3, 2018

@etamponi is it OK for Glitch to have this new command or do you need a config to disable it?

Like the ignore-stop-requests, ignore-upload-requests configs

@etamponi
Copy link
Member

etamponi commented Dec 4, 2018

If it is just informative, it's okay... I would still be happier with a config option to disable it but it's not super urgent :) thanks!

@nareddyt
Copy link
Contributor Author

nareddyt commented Dec 4, 2018

@etamponi I'll create an issue for that and work on it right after this PR is merged in.

@zkochan I'll go ahead and add some tests for my changes. I'll let you know when I'm done

Add tests to `supi` to ensure datamodel returned by `package-store` is correct.
Fix test messages in `server`
@nareddyt
Copy link
Contributor Author

nareddyt commented Dec 6, 2018

@zkochan added tests, should be ready to merge if all checks pass

@zkochan zkochan merged commit ef3adc9 into pnpm:master Dec 7, 2018
zkochan pushed a commit that referenced this pull request Dec 7, 2018
* docs(pnpm): docs for the package usages feature

- Add documentation for `pnpm store usages`
- Refactor docs for `pnpm store` to be more readable

* feat(package-store): implement findPackageUsages

Introduces functionality to search the package store for packages.
Reports all matching packages and the node projects they are used in.
Allows for searching a specific version, or all versions.

* feat(server): expose findPackageUsages API

Exposes an API for finding package usages in the store.

* feat(store-controller-types): Exposes findPackageUsages store functionality

Publicly exposes the findPackageUsages function in the store-controller.
Also exposes the expected response type, allowing clients to be type-safe.

* feat(supi): implement findStoreUsages functionality, add API

Publicly exposes the findPackageUsages function SUPI.

* feat(pnpm): pnpm store usages

Enabled CLI command to find usages of a package in the global store.
Pretty-prints a tree with queries and results.

* style: remove semicolons

Removed semicolons to follow style guide.

* refactor(supi): change test after merge of #1528

* refactor: rename findPackageUsages return type

Better naming of the types returned in the pnpm store usages feature.

* tests: add tests for `pnpm store usages`

Add tests to `supi` to ensure datamodel returned by `package-store` is correct.
Fix test messages in `server`

PR #1529
@zkochan
Copy link
Member

zkochan commented Dec 7, 2018

I was too fast to merge it. I will fix some things myself because I don't want to revert now

zkochan added a commit that referenced this pull request Dec 8, 2018
zkochan added a commit that referenced this pull request Dec 8, 2018
zkochan added a commit that referenced this pull request Dec 8, 2018
zkochan added a commit that referenced this pull request Dec 8, 2018
@zkochan zkochan mentioned this pull request Dec 8, 2018
@nareddyt
Copy link
Contributor Author

nareddyt commented Dec 8, 2018

@zkochan do you have a list of things that were broken? Just curious so I can prevent making the same mistakes next time

zkochan added a commit that referenced this pull request Dec 8, 2018
@zkochan
Copy link
Member

zkochan commented Dec 9, 2018

you can see in this PR the changes I've made.

using parseWantedDependencies() for parsing the package specs seemed unreliable to me. For instance, that method would parse a spec like foo@npm:bar@1.0.0 and return foo as alias. Though the real name of the package is bar. It is better to not support that syntax at all to avoid confusion.

And I somewhat simplified the search function in the store controller

Anyway, next time I will spend more time on review

Thanks for the contribution!

@nareddyt
Copy link
Contributor Author

Thanks, looked over them and that looks much cleaner. This is my first big project with js/ts, so glad to see those changes and I'm learning a lot from them!

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.

None yet

3 participants