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

Remove hoisting of dependencies to prevent global scope packages #310

Merged
merged 1 commit into from
Mar 8, 2025

Conversation

openint-bot
Copy link
Collaborator

@openint-bot openint-bot commented Mar 8, 2025

Important

Remove hoisting configurations and update zod and other dependencies across multiple packages, with minor code refactoring.

  • Dependency Management:
    • Remove .npmrc file, eliminating all hoisting configurations.
    • Update zod dependency to ^3.24.2 in apps/app-config/package.json, connectors/cnext/package.json, connectors/connector-postgres/package.json, kits/connect/package.json, packages/db/package.json, packages/meta-service-airbyte/package.json, packages/util/package.json.
    • Add remeda dependency in connectors/connector-postgres/package.json.
    • Add next dependency in packages/engine-frontend/package.json and packages/ui/package.json.
  • Code Refactoring:
    • Replace TRPCError import from @trpc/server to @openint/trpc in packages/api/authRouter.ts and packages/api/createRouterHandler.ts.
    • Adjust zStandard.connection.omit call in pipelineRouter.ts to remove settings omission.
    • Change TRPCError import in unified/unified-file-storage/adapters/sharepoint-adapter/index.ts to @openint/vdk.

This description was created by Ellipsis for cccefc0. It will automatically update as commits are pushed.

Copy link

vercel bot commented Mar 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
openint ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 8, 2025 0:42am

@openint-bot openint-bot changed the title get rid of all shameless hoisting Remove hoisting of deps to root Mar 8, 2025
@openint-bot openint-bot changed the title Remove hoisting of deps to root Remove hoisting of dependencies for a cleaner setup Mar 8, 2025
Copy link

recurseml bot commented Mar 8, 2025

😱 Found 1 issue. Time to roll up your sleeves! 😱

@openint-bot openint-bot changed the title Remove hoisting of dependencies for a cleaner setup Remove hoisting of dependencies to prevent global scope packages Mar 8, 2025
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to cccefc0 in 2 minutes and 21 seconds

More details
  • Looked at 232 lines of code in 16 files
  • Skipped 1 files when reviewing.
  • Skipped posting 36 drafted comments based on config settings.
1. .npmrc:1
  • Draft comment:
    Removal of public hoist patterns is intentional; please ensure dependencies that were hoisted do not break resolution.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. apps/app-config/constants.ts:9
  • Draft comment:
    Good enhancement: adding VERCEL_URL and localhost fallback improves robustness in determining the server URL.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
3. apps/app-config/package.json:75
  • Draft comment:
    Updated Zod version to ^3.24.2 for consistency; verify that any breaking changes are handled.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. connectors/cnext/package.json:11
  • Draft comment:
    Added Zod dependency update; consistency maintained.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
5. connectors/connector-postgres/package.json:16
  • Draft comment:
    Dependency changes: Addition of 'remeda' and Zod update; confirm no unintended side effects.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
6. kits/connect/package.json:13
  • Draft comment:
    Zod version update to ^3.24.2 looks good; consistent dependency versioning across packages.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
7. packages/api/authRouter.ts:14
  • Draft comment:
    Switching the import of TRPCError to '@openint/trpc' aligns with internal customizations; ensure all error handling is consistent.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment suggests ensuring consistency in error handling, which is a form of asking the author to double-check their work. This violates the rule against asking the author to ensure behavior is intended or tested.
8. packages/api/createRouterHandler.ts:23
  • Draft comment:
    Using fetchRequestHandler from '@openint/trpc' instead of '@trpc/server/adapters/fetch' centralizes the abstraction; ensure the new adapter is compatible.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the author to ensure compatibility, which violates the rule against asking the author to ensure behavior is intended or tested. It does not provide a specific suggestion or ask for a specific test to be written.
9. packages/db/package.json:20
  • Draft comment:
    Zod version updated to ^3.24.2; maintain consistency across packages.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
10. packages/engine-backend/router/pipelineRouter.ts:130
  • Draft comment:
    Changed omit from removing both 'id' and 'settings' to only 'id'. Verify that sensitive settings are not inadvertently exposed; deletion later in parseConnection mitigates this.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
11. packages/engine-frontend/package.json:16
  • Draft comment:
    Explicitly adding 'next' version (15.2.1) ensures a consistent runtime environment for front-end code.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
12. packages/meta-service-airbyte/package.json:18
  • Draft comment:
    Zod version updated to ^3.24.2 here as well; great for version consistency.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
13. packages/trpc/index.ts:6
  • Draft comment:
    No issues; maintaining exports from '@trpc/server' is fine and supports existing consumers.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
14. packages/ui/package.json:43
  • Draft comment:
    Ensure the explicit next version is replicated in both frontend packages for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
15. packages/util/package.json:41
  • Draft comment:
    Zod update to ^3.24.2 supports consistent validations across utilities.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
16. unified/unified-file-storage/adapters/sharepoint-adapter/index.ts:2
  • Draft comment:
    Importing TRPCError from '@openint/vdk' instead of '@trpc/server' fits with internal patterns; double-check that error handling and messaging are compatible.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment suggests checking compatibility of error handling and messaging, which violates the rule against asking the PR author to double-check things. It does not provide a specific code suggestion or ask for a specific test to be written.
17. .npmrc:1
  • Draft comment:
    Deletion of the .npmrc file removes custom hoisting. Ensure that this change doesn't impact dependency resolution or module layout in your build system.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
18. apps/app-config/constants.ts:10
  • Draft comment:
    Switching to bracket notation for accessing the 'host' header improves consistency. Verify that the header is always available and consider a fallback if it’s missing.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    The code already has multiple layers of fallbacks if the req.headers['host'] part fails - it will try environment variables and eventually default to localhost. The change from .host to ['host'] notation is just a style change. The type definition shows headers is Record<string, string> so TypeScript ensures it's safe.
    Maybe there's a case where having req but missing the host header could cause different behavior than having no req at all?
    Even in that case, the existing fallback chain would handle it gracefully by moving to the environment variables and localhost fallbacks.
    The comment should be deleted because the code already has robust fallback handling, and the suggestion would actually make the fallback logic less clear.
19. apps/app-config/package.json:75
  • Draft comment:
    Zod version has been updated to ^3.24.2. Confirm that all schema validations across dependent packages remain compatible.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to confirm compatibility across dependent packages, which violates the rule against asking for confirmation or ensuring compatibility. It does not provide a specific code suggestion or point out a specific issue.
20. connectors/connector-postgres/package.json:14
  • Draft comment:
    A new dependency 'remeda' has been added and zod updated to ^3.24.2. Ensure this version of remeda aligns with the rest of the monorepo to prevent potential version conflicts.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is about dependencies, specifically mentioning a new dependency 'remeda' and an update to 'zod'. It advises ensuring version alignment within the monorepo. This falls under the rule of not commenting on dependency changes or library versions that are not recognized. The comment does not provide a specific code suggestion or ask for a test to be written, which would make it useful.
21. kits/connect/package.json:13
  • Draft comment:
    Zod version updated to ^3.24.2. Verify that consumers of this package are not affected by any breaking changes in the new version.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to verify that consumers are not affected by breaking changes, which falls under the rule of not asking the author to ensure behavior is intended or tested. It does not provide a specific suggestion or point out a specific issue with the code.
22. packages/api/authRouter.ts:14
  • Draft comment:
    TRPCError import has been moved from '@trpc/server' to '@openint/trpc'. Ensure that the new re-export or wrapper maintains the same behavior for error handling.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is about a change in the import path for TRPCError. It suggests ensuring that the new import maintains the same behavior, which is a request for confirmation of behavior. This violates the rule against asking the PR author to ensure behavior is intended.
23. packages/api/createRouterHandler.ts:20
  • Draft comment:
    The fetchRequestHandler import has been switched to use '@openint/trpc'. Confirm that its API and error handling remain fully compatible with previous usage.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to confirm compatibility, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code change.
24. packages/db/package.json:23
  • Draft comment:
    Zod version bumped to ^3.24.2. Ensure that all database schema validations and related logic continue to function correctly with the new version.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to ensure that the database schema validations and related logic continue to function correctly with the new version of Zod. This falls under the category of asking the author to double-check things, which is against the rules. The comment does not provide a specific code suggestion or point out a specific issue with the code.
25. packages/engine-backend/router/pipelineRouter.ts:130
  • Draft comment:
    The omission of the 'settings' property from the connection schema has been removed. Ensure that exposing settings does not leak sensitive configuration details.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    The code already handles settings security by explicitly deleting them at line 178 before returning the data. The .omit() change doesn't create a security risk because settings are handled elsewhere. The comment's suggestion is also technically incorrect - .omit() takes an object of keys to omit, not boolean values.
    I could be wrong about the relationship between the .omit() call and the later deletion of settings. Maybe there's a reason both were needed originally.
    The explicit deletion with a clear comment about security at line 178 shows this is the intended place to handle sensitive data. Having redundant security measures could make the code more confusing.
    Delete this comment. The security concern is already properly handled elsewhere in the code, and the suggested fix uses incorrect syntax.
26. packages/engine-frontend/package.json:16
  • Draft comment:
    An explicit dependency on 'next' version 15.2.1 has been added. Confirm that this version is compatible with the rest of your frontend setup.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to confirm compatibility of a dependency version, which is against the rules. It does not provide a specific suggestion or point out a specific issue with the code.
27. packages/meta-service-airbyte/package.json:15
  • Draft comment:
    Zod has been updated to ^3.24.2. Verify that schema validations in the airbyte service remain correct after this upgrade.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to verify the correctness of schema validations after a dependency update. This falls under the rule of not asking the author to ensure behavior is intended or tested, especially related to dependencies.
28. packages/trpc/index.ts:7
  • Draft comment:
    The export for fetchRequestHandler has been added. This centralizes its usage; ensure that all consumers are updated to import from this consolidated module.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is purely informative and asks the author to ensure that all consumers are updated, which violates the rules. It doesn't provide a specific code suggestion or ask for a specific test to be written.
29. packages/ui/package.json:42
  • Draft comment:
    A dependency on 'next' version 15.2.1 has been added. Make sure this version is consistent with other packages in the monorepo.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is about a dependency change, specifically mentioning a version of a package. The rules state not to comment on pure dependency changes or library versions that are not recognized. This comment is asking to ensure consistency with other packages, which is similar to asking for confirmation or verification, which is not allowed.
30. packages/util/package.json:41
  • Draft comment:
    Zod has been upgraded to ^3.24.2. Verify that there are no breaking changes affecting utility functions or type validations.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to verify that there are no breaking changes, which is against the rules. It does not provide a specific suggestion or point out a specific issue with the code.
31. unified/unified-file-storage/adapters/sharepoint-adapter/index.ts:2
  • Draft comment:
    TRPCError is now imported from '@openint/vdk'. Ensure that the error handling behavior is consistent and that the new source correctly maps error codes.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to ensure consistency in error handling behavior and correct mapping of error codes. This falls under asking the author to double-check things, which is against the rules. The comment does not provide a specific suggestion or point out a specific issue.
32. apps/app-config/constants.ts:25
  • Draft comment:
    Consider renaming 'getGraphqlEndpoint' to 'getGraphQLEndpoint' for clarity and consistency with common GraphQL naming conventions.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
33. apps/app-config/package.json:25
  • Draft comment:
    Typographical error: The package '@openint/connector-foreceipt' appears to have a possible misspelling. Please check if 'foreceipt' is intended or if it should be corrected to something like 'connector-for-receipt' or another proper spelling.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
34. packages/api/authRouter.ts:114
  • Draft comment:
    Typographical Error: On line 114, the code mistakenly uses 'usr.publicMetadata' when parsing unsafeMetadata. Please change it to 'usr.unsafeMetadata' to correctly reflect the intended data.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
35. packages/api/createRouterHandler.ts:111
  • Draft comment:
    Typographical error: 'ignoroing' should be corrected to 'ignoring' in the commented-out console.warn statement.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
36. unified/unified-file-storage/adapters/sharepoint-adapter/index.ts:397
  • Draft comment:
    Typographical error: Change 'Sharepoint' to 'SharePoint' in the error message of exportFile for correct product naming.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_efhKM2ljjDpfupcL


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@openint-bot openint-bot merged commit b8e5282 into main Mar 8, 2025
8 of 9 checks passed
@openint-bot openint-bot deleted the deps-upgrade branch March 8, 2025 00:56
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.

1 participant