-
Notifications
You must be signed in to change notification settings - Fork 6
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
base: main
Are you sure you want to change the base?
Conversation
adding config removing bak
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this 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 in37
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.
There was a problem hiding this 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 in2
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.
There was a problem hiding this 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 in4
files - Skipped
1
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. packages/ui/components/SchemaForm.tsx:140
- Draft comment:
Thevalidator
is defined but not used in theSchemaForm
component. Ensure that the validation logic is correctly implemented or remove the unusedvalidator
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 thedev
script. Ensure this change is intentional and doesn't negatively impact development performance. - Reason this comment was not posted:
Confidence changes required:50%
Thedev
script inapps/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.
There was a problem hiding this 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 in79
files - Skipped
1
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. tbd/db/upsert.ts:102
- Draft comment:
TheshallowMergeJsonbColumns
option defaults to merging all JSONB columns if set totrue
. Consider explicitly specifying which columns should be merged to avoid unintended behavior. - Reason this comment was not posted:
Confidence changes required:50%
ThedbUpsert
function inupsert.ts
has a potential issue with theshallowMergeJsonbColumns
option. It currently defaults to merging all JSONB columns if set totrue
, 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:
EnsurekeyColumns
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%
ThedbUpsert
function inupsert.ts
does not handle the case wherekeyColumns
is undefined. This could lead to runtime errors if the table does not have primary keys defined. A check should be added to ensurekeyColumns
is always defined.
3. tbd/db/upsert.spec.ts:99
- Draft comment:
Avoid usingas 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%
Inupsert.spec.ts
, the test forupsert param handling inc. jsonb
usesas 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 usingas never
.
Workflow ID: wflow_AaJVuuutkFV68mrD
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this 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 in19
files - Skipped
1
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. connectors/connector-splitwise/def.ts:80
- Draft comment:
TheformatAmount
function fromblossomfinance/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.
There was a problem hiding this 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 d89fe79 in 2 minutes and 3 seconds
More details
- Looked at
5997
lines of code in82
files - Skipped
2
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. tbd/db/index.ts:40
- Draft comment:
Usesql.identifier(schema)
instead of${sql.identifier(schema)}
for dynamic schema names to prevent SQL injection. - Reason this comment was not posted:
Comment was on unchanged code.
2. tbd/worker/functions.ts:306
- Draft comment:
Ensure to callpg.end()
to close the database connection and prevent resource leaks. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_ExM1L2bGvYafIEDz
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this 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 5c2aae0 in 54 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
1
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. apps/web/package.json:78
- Draft comment:
Thewrangler
package is listed in both dependencies and devDependencies. Consider keeping it only in devDependencies unless it's needed in production. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_H41nHcskO8BOLZFY
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this 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 ad92f19 in 44 seconds
More details
- Looked at
24
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. apps/web/patch-cloudflare.bin.js:85
- Draft comment:
Ensure that the indentation style matches the existing code inhandler.mjs
to avoid formatting issues. If the file uses tabs, adjust the insertion line accordingly. - Reason this comment was not posted:
Confidence changes required:50%
The insertion marker and line are indented with spaces, which might cause issues if the file uses tabs for indentation. It's better to match the existing indentation style.
Workflow ID: wflow_NOoxlV8eNr1tlhT2
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this 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 ceda247 in 1 minute and 25 seconds
More details
- Looked at
757
lines of code in19
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. packages/engine-backend/router/connectionRouter.ts:55
- Draft comment:
ReassigningconnUpdate
after_syncConnectionUpdate
might lead to unexpected behavior. Consider using a different variable name for the updated connection to avoid confusion. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
2. packages/engine-backend/services/sync-service.ts:439
- Draft comment:
LoggingconnUpdate
includes potentially sensitive information. Consider sanitizing logs to avoid leaking sensitive data. - Reason this comment was not posted:
Comment was on unchanged code.
3. tbd/db/package.json:10
- Draft comment:
Thedb:run-migration
script is a placeholder and does not perform any action. Consider implementing the actual migration logic. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The change deliberately replaced a working migration script with a placeholder, suggesting this is an intentional temporary state. The TODO message in the script itself already indicates this is temporary. The comment doesn't provide any additional value beyond what's obvious from the code itself.
The comment could be highlighting a potential issue where someone might rely on this script thinking it performs migrations when it doesn't.
The TODO message in the script itself already makes it clear this is temporary and non-functional. Anyone trying to use this script would immediately see it's not implemented.
Delete the comment as it only restates what's already obvious from the code and doesn't provide actionable feedback.
4. unified/unified-accounting/router.ts:16
- Draft comment:
Ensuremeta.path
does not already include the base path to avoid duplication when using theoapi
function. - Reason this comment was not posted:
Confidence changes required:50%
Inunified-accounting/router.ts
, theoapi
function is used to construct OpenAPI metadata. The function appends a base path to themeta.path
, which is a good practice for maintaining consistent API paths. However, ensure thatmeta.path
does not already include the base path to avoid duplication.
Workflow ID: wflow_AHek2pM0zcgzjKXK
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this 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 0ce5d12 in 45 seconds
More details
- Looked at
16
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. packages/engine-backend/router/connectionRouter.ts:59
- Draft comment:
Ensureconn.integrationId
is defined and in the expected format before usingextractId(conn.integrationId)[2]
to avoid potential runtime errors. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_vu9lhPtveFIK8F0q
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this 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 in2
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.
There was a problem hiding this 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 in1
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 likePOSTGRES_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.
There was a problem hiding this 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 in1
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 wrappingexecSync
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.
Important
Enables serverless functionality in Next.js app with Cloudflare Workers and removes unused connectors.
open-next.config.ts
andwrangler.toml
.connector-firebase
,connector-foreceipt
, andconnector-mongodb
fromconnectors.def.ts
,connectors.merged.ts
,connectors.server.ts
, andmeta.js
.@openint/connector-firebase
,@openint/connector-foreceipt
, and@openint/connector-mongodb
frompackage.json
inapp-config
,cli
, andweb
.package.json
scripts and dependencies for Cloudflare Workers..gitignore
to include.open-next/
and.next-bak
.next.config.js
to includecrypto
alias and Cloudflare settings.worker-aliases/crypto.js
andworker-aliases/critters.js
for module aliasing.bar()
infoo.py
.baz()
infoo.py
to describe input dict format.This description was created by for 25218a2. It will automatically update as commits are pushed.