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

Refactor: find up #1019

Merged
merged 6 commits into from
Feb 22, 2024
Merged

Refactor: find up #1019

merged 6 commits into from
Feb 22, 2024

Conversation

TGTGamer
Copy link
Contributor

@TGTGamer TGTGamer commented Feb 20, 2024

Goal

Starts #1008

Refactor to make better use of this function and ensure multiple directories can be found such as needed for node_module directories to ensure support for mono-repos

Details as follows.

findUp (): String | String[] | undefined

Summary

The findUp function is used to search for file paths by recursively searching parent directories based on a given list of names and the current working directory. It can return a single path or multiple paths, depending on the specified flag. If no paths are found, it returns undefined.

Example Usage

const names = ['node_modules'];
const cwd = '/path/to/parent/child/project/current/directory';
const result = findUp(names, cwd, true);
console.log(result);
// Output: ['/path/to/parent/child/project/current/directory/node_modules', '/path/to/parent/node_modules']

Code Analysis

Inputs

  • names (array of strings): An array of names to search for within the directory.
  • cwd (string, optional): The current working directory. Defaults to the process's current working directory.
  • multiple (boolean, optional): A flag indicating whether to search for multiple levels of files. Defaults to false.
  • result (array of strings, optional): An array to store the accumulated results when searching for multiple paths.

Flow

  1. Check if any of the names in the list are empty. If all names are empty, return undefined.
  2. Find the first name in the list that exists in the current working directory. If found and multiple is false, return the path as a string.
  3. If multiple is true and a name is found, add the path to the result array.
  4. Get the parent directory path of the current working directory.
  5. If the parent directory is the same as the current working directory, return the result array if multiple is true and it contains paths, otherwise return undefined.
  6. Recursively call the findUp function with the parent directory as the new current working directory.
  7. Return the result of the recursive call.

Outputs

  • If multiple is false and a path is found, it returns the path as a string or undefined if no path is found.
  • If multiple is true and at least one path is found, it returns an array of paths or undefined if no path is found.

Tests

findup.spec.js
Total: 7 milliseconds
✓ 9

✓ should return the name of the first matching file when multiple is false and a match is found - 3 milliseconds
✓ should return undefined when no matches are found - 1 millisecond
✓ should search for specified files in parent directories starting from the given current working directory (cwd) - 0 milliseconds
✓ should accumulate results when multiple is true and matches are found - 2 milliseconds
✓ should return undefined when names array is empty - 0 milliseconds
✓ should return undefined when cwd is not a valid directory - 0 milliseconds
✓ should return undefined when no matches are found and multiple is true - 1 millisecond
✓ should return undefined when no matches are found and multiple is false - 0 milliseconds
✓ should return undefined when names array contains an empty string - 0 milliseconds

Tests can be found here: https://bit.cloud/tgtgamer/experiments/findup/~tests


Summary by CodeRabbit

  • Refactor
    • Enhanced file search functionality within the schema generation process for more efficient file path resolution.
  • Chores
    • Updated internal utilities to support more flexible file searching capabilities and introduced a deprecation notice for outdated methods.

multiple paths and add detailed JSDoc comments for better understanding and
documentation
…of findUp function for improved clarity and consistency

Signed-off-by: Jonathan Stevens <jonathan.stevens@resnovas.com>
…utility function to locate package.json file in the directory hierarchy (respecting deprecation a171002)

Signed-off-by: Jonathan Stevens <jonathan.stevens@resnovas.com>
…n instead of findPackageJson for better clarity and consistency (respecting deprecation a171002)

Signed-off-by: Jonathan Stevens <jonathan.stevens@resnovas.com>
…better clarity and consistency with other utility functions. This change improves code readability and maintainability. (respecting deprecation a171002)

Signed-off-by: Jonathan Stevens <jonathan.stevens@resnovas.com>
Copy link
Contributor

coderabbitai bot commented Feb 20, 2024

Walkthrough

Walkthrough

The recent updates primarily focus on improving how package.json files are located within a project. The introduction of the findUp function from the pkg-utils module replaces the older findPackageJson method, offering enhanced search capabilities and flexibility. This change is applied across multiple files, streamlining the process of determining file paths, especially for the default Prisma output file. Additionally, pkg-utils.ts sees the findUp function becoming more versatile and marking findPackageJson as deprecated.

Changes

File(s) Summary
.../cli/cli-util.ts, .../plugins/prisma/schema-generator.ts Replaced findPackageJson with findUp from pkg-utils for improved package.json file search.
.../utils/pkg-utils.ts Enhanced findUp with a generic type and support for multiple paths; Added deprecation notice for findPackageJson.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 4398231 and 9633b04.
Files selected for processing (3)
  • packages/schema/src/cli/cli-util.ts (2 hunks)
  • packages/schema/src/plugins/prisma/schema-generator.ts (2 hunks)
  • packages/schema/src/utils/pkg-utils.ts (3 hunks)
Additional comments: 7
packages/schema/src/utils/pkg-utils.ts (3)
  • 7-16: The introduction of the generic type FindUp is a significant improvement, allowing the findUp function to return different types based on the multiple flag. This change enhances the function's versatility, enabling it to accommodate various use cases more effectively.
  • 32-39: The modifications to the findUp function incorporate the new FindUp generic type and add support for searching multiple paths. The logic appears sound, correctly handling both single and multiple search scenarios. However, it's important to ensure that the recursive call on line 39 properly accumulates results in multiple search scenarios without losing context or duplicating entries.
  • 110-114: Adding a deprecation notice for findPackageJson and recommending the use of findUp instead is a good practice. It helps guide developers towards using the more versatile and improved findUp function. Ensure that all references to findPackageJson within the project are updated to use findUp, and consider adding a timeline or version number for when findPackageJson will be removed to help with planning.
packages/schema/src/cli/cli-util.ts (2)
  • 16-16: The replacement of findPackageJson with findUp from ../utils/pkg-utils is a positive change, aligning with the project's goal to improve file search functionality. This modification leverages the enhanced capabilities of findUp, making the file search more versatile and efficient.
  • 283-283: The usage of findUp to locate the package.json file within getDefaultSchemaLocation function is correctly implemented. This change ensures that the function benefits from the improved search capabilities of findUp, potentially handling more complex directory structures more effectively.
packages/schema/src/plugins/prisma/schema-generator.ts (2)
  • 51-51: The replacement of findPackageJson with findUp in schema-generator.ts is consistent with the project's efforts to enhance file search functionality. Utilizing findUp for locating the package.json file in getDefaultPrismaOutputFile function is a logical step that leverages the improved capabilities of findUp.
  • 453-453: The implementation of findUp within getDefaultPrismaOutputFile to locate the package.json file is correctly done. This change ensures that Prisma schema generation can benefit from the more versatile and efficient file search capabilities provided by findUp.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 9633b04 and 1b01c31.
Files selected for processing (1)
  • packages/schema/src/utils/pkg-utils.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
  • packages/schema/src/utils/pkg-utils.ts

@TGTGamer
Copy link
Contributor Author

Looks like all the tests for this passed now, I think merge of this is the only think left to do before I can rebase #1021 and mark that ready to review :)

@ymc9
Copy link
Member

ymc9 commented Feb 22, 2024

Sure. I'm merging it now.

@ymc9 ymc9 merged commit 4bf812e into zenstackhq:dev Feb 22, 2024
3 checks passed
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.

2 participants