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

Suggestion: Add initial support for Lerna monorepos #11

Merged
merged 4 commits into from
May 18, 2019

Conversation

OriMarron
Copy link
Contributor

@OriMarron OriMarron commented May 6, 2019

This is a suggestion to support searching the node_modules of child packages in lerna monorepos.

(related to #10)

background

Workspaces that use lerna usually have the following file structure:

/node_modules
/package.json
/lerna.json

/packages/project-a 
  /node_modules
  /package.json

/packages/project-b/
  /node_modules
  /package.json

Currently, for workspaces with this structure, the extension will search only in the root /node_modules, leaving the user unable to search in one of the child projects.

With this PR, the user will be prompted to select which project to search in.
image

Logic

The logic that is used is as follows:

  1. If there is no lerna.json file at the root level - continue as before
  2. read lerna.json file, and look at the packages field.
    Example json config file:
{
  "packages": [
    "packages/*"
  ],
  "version": "0.0.0"
}
  1. Search for packages matching the pattern(s) using glob.
    a package is a folder containing a pakage.json file

  2. If no packages are found - search the root /node_modules as before

  3. Prompt the user to choose a project to search in: either the root or one of the packages

  4. Search the selected project's /node_modules folder using the existing logic

Note: patterns containing ** are ignored for now

Please tell me what you think.
If you agree with this direction, I would gladly extend the support to also include ** and yarn projects.
@jasonnutter

@jasonnutter
Copy link
Owner

This looks great, thanks for the contribution! I tried a couple times to get this working and kept getting stuck, nice to see someone get it figured out.

I have some small pieces of feedback, would you like that now, or were you thinking of adding the yarn support to this PR (if you think yarn support may drastically change this PR, I'll wait to provide any detailed feedback until that's ready)?

Thanks!

@OriMarron
Copy link
Contributor Author

Please provide any feedback! I will address it as soon as possible :)

@OriMarron
Copy link
Contributor Author

I just added a commit with support for Yarn workspaces.

It works exactly the same, apart from looking in package.json/workspaces instead of lerna.json/packages

@jasonnutter
Copy link
Owner

Awesome, I'll take a closer look today!

One question: one thing I kept getting hung up on was how to properly implement support for when VS Code / yarn / lerna workspaces are nested, e.g. a lerna workspace inside a VS Code workspace. Would that be supported with this implementation?

@jasonnutter
Copy link
Owner

Ultimately not a blocker if not, as that is something that could be added incrementally. Just wanted to see if that was considered.

@OriMarron
Copy link
Contributor Author

I actually kept it in mind, and this case is fully supported.

Let's say your workspace looks like this:

/regular-repo
    /node_modules
    /package.json

/lerna-repo
    /node_modules
    /lerna.json
    /packages/project1
        /package.json
        /node_modules

    /packages/project2
        /package.json
        /node_modules

you will first be prompted to choose between regular-repo and lerna-repo. If you chose lerna-repo, only then you will be prompted to choose between project1 and project2

Copy link
Owner

@jasonnutter jasonnutter left a comment

Choose a reason for hiding this comment

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

Left a couple comments, otherwise this looks great, thanks again for the contribution!

find-child-packages.js Outdated Show resolved Hide resolved
find-child-packages.js Outdated Show resolved Hide resolved
find-child-packages.js Outdated Show resolved Hide resolved
find-child-packages.js Outdated Show resolved Hide resolved
@jasonnutter jasonnutter self-assigned this May 8, 2019
@OriMarron
Copy link
Contributor Author

Thanks for your quick review and feedback!
I committed the requested changes

@jasonnutter
Copy link
Owner

Apologies for the delay, have been on vacation and forgot to circle back on this. I'll get this merged and published this weekend. Thanks again!

@jasonnutter jasonnutter merged commit 4e36234 into jasonnutter:master May 18, 2019
@OriMarron
Copy link
Contributor Author

@jasonnutter Hi, any news on publishing a new version?

@jasonnutter
Copy link
Owner

Sorry for the delay, working on it! See my comment on #13

@OrenZak
Copy link

OrenZak commented Aug 13, 2023

@jasonnutter when will be a release ?

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.

3 participants