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

Enabling serverless in new next app #152

Draft
wants to merge 62 commits into
base: main
Choose a base branch
from
Draft

Enabling serverless in new next app #152

wants to merge 62 commits into from

Conversation

pellicceama
Copy link
Collaborator

@pellicceama pellicceama commented Dec 14, 2024

Important

Enables serverless functionality in Next.js app with Cloudflare Workers and removes unused connectors.

  • Behavior:
    • Integrates Cloudflare Workers for serverless functionality in open-next.config.ts and wrangler.toml.
    • Removes connector-firebase, connector-foreceipt, and connector-mongodb from connectors.def.ts, connectors.merged.ts, connectors.server.ts, and meta.js.
    • Deletes related files and directories for these connectors.
  • Dependencies:
    • Removes @openint/connector-firebase, @openint/connector-foreceipt, and @openint/connector-mongodb from package.json in app-config, cli, and web.
    • Updates package.json scripts and dependencies for Cloudflare Workers.
  • Configuration:
    • Updates .gitignore to include .open-next/ and .next-bak.
    • Updates next.config.js to include crypto alias and Cloudflare settings.
    • Adds worker-aliases/crypto.js and worker-aliases/critters.js for module aliasing.
  • Miscellaneous:
    • Fixes off-by-one error in bar() in foo.py.
    • Updates docstring for baz() in foo.py to describe input dict format.

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

Copy link

vercel bot commented Dec 14, 2024

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

Name Status Preview Comments Updated (UTC)
openint ❌ Failed (Inspect) Feb 25, 2025 2:48pm

Copy link

@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 8712890 in 44 seconds

More details
  • Looked at 2606 lines of code in 37 files
  • Skipped 2 files when reviewing.
  • Skipped posting 7 drafted comments based on config settings.
1. apps/app-config/connectors/connectors.def.ts:8
  • Draft comment:
    The removal of the Firebase, Foreceipt, and MongoDB connectors from this file suggests they are no longer needed. Ensure these connectors are not used elsewhere in the codebase to avoid runtime errors.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. apps/app-config/connectors/connectors.merged.ts:15
  • Draft comment:
    The removal of the Firebase, Foreceipt, and MongoDB connectors from this file suggests they are no longer needed. Ensure these connectors are not used elsewhere in the codebase to avoid runtime errors.
  • Reason this comment was not posted:
    Marked as duplicate.
3. apps/app-config/connectors/connectors.server.ts:8
  • Draft comment:
    The removal of the Firebase, Foreceipt, and MongoDB connectors from this file suggests they are no longer needed. Ensure these connectors are not used elsewhere in the codebase to avoid runtime errors.
  • Reason this comment was not posted:
    Marked as duplicate.
4. apps/app-config/connectors/meta.js:81
  • Draft comment:
    The removal of the Firebase, Foreceipt, and MongoDB connectors from this file suggests they are no longer needed. Ensure these connectors are not used elsewhere in the codebase to avoid runtime errors.
  • Reason this comment was not posted:
    Marked as duplicate.
5. apps/app-config/package.json:24
  • Draft comment:
    The removal of the Firebase, Foreceipt, and MongoDB dependencies suggests they are no longer needed. Ensure these connectors are not used elsewhere in the codebase to avoid runtime errors.
  • Reason this comment was not posted:
    Marked as duplicate.
6. apps/cli/package.json:11
  • Draft comment:
    The removal of the Firebase dependency suggests it is no longer needed. Ensure this connector is not used elsewhere in the codebase to avoid runtime errors.
  • Reason this comment was not posted:
    Marked as duplicate.
7. package.json:115
  • Draft comment:
    The removal of the Firebase patch suggests it is no longer needed. Ensure this connector is not used elsewhere in the codebase to avoid runtime errors.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_Tn3ew9S36MpBmAhy


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

Copy link

@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! Incremental review on 374ab1d in 20 seconds

More details
  • Looked at 156 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_Hc5FliPnYwUGk63X


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

Copy link

@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! Incremental review on 2c9c532 in 20 seconds

More details
  • Looked at 158 lines of code in 4 files
  • Skipped 1 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. packages/ui/components/SchemaForm.tsx:140
  • Draft comment:
    The validator is defined but not used in the SchemaForm component. Ensure that the validation logic is correctly implemented or remove the unused validator to avoid confusion.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. apps/web/package.json:13
  • Draft comment:
    The --turbo flag was removed from the dev script. Ensure this change is intentional and doesn't negatively impact development performance.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The dev script in apps/web/package.json was modified to remove --turbo, which might affect performance. It's important to ensure this change is intentional and doesn't degrade the development experience.

Workflow ID: wflow_i6PqFF1B9zy10Ns1


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

Copy link

@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! Incremental review on 1b2e842 in 1 minute and 1 seconds

More details
  • Looked at 5686 lines of code in 79 files
  • Skipped 1 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. tbd/db/upsert.ts:102
  • Draft comment:
    The shallowMergeJsonbColumns option defaults to merging all JSONB columns if set to true. Consider explicitly specifying which columns should be merged to avoid unintended behavior.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The dbUpsert function in upsert.ts has a potential issue with the shallowMergeJsonbColumns option. It currently defaults to merging all JSONB columns if set to true, which might not be the intended behavior. It would be better to explicitly specify which columns should be merged.
2. tbd/db/upsert.ts:99
  • Draft comment:
    Ensure keyColumns is always defined to prevent runtime errors. Consider adding a check to handle cases where the table does not have primary keys defined.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The dbUpsert function in upsert.ts does not handle the case where keyColumns is undefined. This could lead to runtime errors if the table does not have primary keys defined. A check should be added to ensure keyColumns is always defined.
3. tbd/db/upsert.spec.ts:99
  • Draft comment:
    Avoid using as never to bypass type checks. Consider using more appropriate types or restructuring the test to ensure type safety.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    In upsert.spec.ts, the test for upsert param handling inc. jsonb uses as never to bypass type checks. This is not ideal as it can hide potential type errors. Consider using more appropriate types or restructuring the test to avoid using as never.

Workflow ID: wflow_AaJVuuutkFV68mrD


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

Copy link

@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! Incremental review on d973e1c in 28 seconds

More details
  • Looked at 689 lines of code in 19 files
  • Skipped 1 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. connectors/connector-splitwise/def.ts:80
  • Draft comment:
    The formatAmount function from blossomfinance/iso-4217-currencies has been removed, but it is still referenced in the code. This may cause runtime errors. Consider replacing it with an alternative implementation or removing the reference.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_R40gezWink3tjzJ1


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

Copy link

@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.

❌ Changes requested. Incremental review on c649c94 in 1 minute and 12 seconds

More details
  • Looked at 51 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. packages/engine-backend/router/customerRouter.ts:315
  • Draft comment:
    Avoid logging sensitive information in production environments. Consider using a logging library that can filter out sensitive data.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The log message is just a simple status message indicating that inngest is being skipped in edge runtime. It doesn't contain any sensitive data like user info, tokens, or business logic details. The comment seems to be a generic warning that doesn't apply to this specific case.
    I could be wrong about what constitutes sensitive information - perhaps the mere fact that we're in edge runtime could be considered sensitive by some security standards.
    The runtime environment is typically not considered sensitive information, and this log message is clearly intended for debugging purposes with no security implications.
    The comment should be deleted because it warns about a security issue that doesn't exist in this code - the log message contains no sensitive information.

Workflow ID: wflow_brLt9Qt2QKtmtk4K


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

@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! Incremental review on 4c18d66 in 51 seconds

More details
  • Looked at 27 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. apps/web/wrangler.toml:12
  • Draft comment:
    Sensitive variables like POSTGRES_URL, JWT_SECRET, and others should be removed from version control to prevent security risks. Consider using environment variables or a secure vault.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_nG6qZniVT3jL64Jh


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

Copy link

@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! Incremental review on 25218a2 in 1 minute and 30 seconds

More details
  • Looked at 21 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. apps/web/patch-cloudflare.bin.js:53
  • Draft comment:
    Consider wrapping execSync calls for git commands in a try-catch block to handle potential errors gracefully.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The execSync calls for git commands are already within a try-catch block that handles errors appropriately. The error handling is actually quite thorough, with proper error logging and process exit. The comment is suggesting something that's already implemented, making it redundant.
    Maybe there's value in having separate error handling specifically for git commands vs file operations? The current try-catch block is somewhat broad.
    The existing error handling is sufficient because it properly logs the error and exits the process, which is appropriate behavior for a build script where any failure should stop the process.
    The comment should be deleted because it suggests implementing error handling that already exists in the code.

Workflow ID: wflow_vDxDSDLICWdm6ou6


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

@pellicceama pellicceama marked this pull request as draft February 5, 2025 21:58
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