Skip to content

Conversation

@MagnumGoYB
Copy link

Removed the manual setting of id when creating records via the Drizzle Adapter to prevent the following warning:

ScreenShot_2025-11-11_220437_041

[Better Auth]: [Drizzle Adapter] - You are trying to create a record with an id. This is not allowed as we handle id generation for you. The id will be ignored.
Copilot AI review requested due to automatic review settings November 11, 2025 14:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a warning from the Drizzle Adapter by removing the manual ID setting when creating database records during file uploads. The adapter auto-generates IDs and rejects manual ID specification.

Key changes:

  • Removed explicit id field from adapter.create() call during file upload
  • Captured the returned database record to access the adapter-generated ID
  • Updated response to use the new database-generated ID instead of the pre-generated one

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +651 to +654
// You are trying to create a record with an id.
// This is not allowed as we handle id generation for you.
// The id will be ignored.
// id: fileMetadata.id,
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

This multi-line comment explaining why the ID is commented out should be removed. Commented-out code with explanatory notes is typically not appropriate for production code. If the ID field should not be set, it should simply be omitted rather than left as a commented line with an explanation.

Consider removing lines 651-654 entirely.

Suggested change
// You are trying to create a record with an id.
// This is not allowed as we handle id generation for you.
// The id will be ignored.
// id: fileMetadata.id,

Copilot uses AI. Check for mistakes.
Copy link
Owner

Choose a reason for hiding this comment

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

this would need to be removed, yes

@zpg6
Copy link
Owner

zpg6 commented Nov 14, 2025

Hi @MagnumGoYB ! Sorry for delay busy work week will check this out during the weekend!

@zpg6
Copy link
Owner

zpg6 commented Dec 6, 2025

Hi @MagnumGoYB - thanks again for your contribution! There are a few more things to consider about this change:

The file id set from the metadata comes from the process of storing in R2 - the file id in R2 and the file id in D1 are meant to match. We could switch the order of the calls (store metadata in D1, then store file in R2) so that drizzle id is made first, then use that drizzle id in the R2 upload. But that ordering of calls is not the best design in my opinion - you would need to consider rolling back D1 inserts if the R2 upload failed to keep things synced.

Open to more feedback, I could be convinced otherwise :)

@zpg6
Copy link
Owner

zpg6 commented Dec 6, 2025

Removed the manual setting of id when creating records via the Drizzle Adapter to prevent the following warning:
ScreenShot_2025-11-11_220437_041

Side note: in the future consider posting the error or warning text using the backticks (`) instead of a screenshot. This makes it more searchable by others so they can see our resolution.

```
heres my error
more lines of it
```

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