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

Fix hardcoded 'normalizedEmail' database field #33

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kdcokenny
Copy link

Summary

  • Added normalizedEmailField option to allow configuring the database field name
  • Replaced all hardcoded 'normalizedEmail' references with the configurable field name
  • Fixed import order to be consistent

Fixes #31

Test plan

  • Existing tests should pass
  • Email harmony plugin should work with custom database field names

Make the normalized email field name configurable via normalizedEmailField
option to fix compatibility with databases that use different field names.

Fixes GeKorm#31
@GeKorm
Copy link
Owner

GeKorm commented Mar 17, 2025

/release:pr d01b7de

Copy link
Contributor

A new release has been made for this PR. You can install the package you need using

  • npm i better-auth-harmony@0.0.0-commit-release.0.1741811462.d01b7de

@GeKorm
Copy link
Owner

GeKorm commented Mar 17, 2025

Hi @kdcokenny thank you very much for both the bug report and PR! I took some time to look at how internal better-auth plugins handle this case and they seem to be leveraging the fieldName field in the schema declaration. I'd greatly appreciate if you checked that approach works for you by installing better-auth-harmony@0.0.0-commit-release.0.1742214020.b6a54e8. The code change is here in PR #35.

@kdcokenny
Copy link
Author

Sorry about the delay, thanks for pushing this so quickly. I will go over it shortly.

@kdcokenny
Copy link
Author

When using the package as you suggested I get the following error: module is not defined at eval (/Users/kenny/workspace/firestorm/node_modules/mailchecker/platform/node/index.js?v=caadbbaa:31:1) at instantiateModule (file:///Users/kenny/workspace/firestorm/node_modules/vite/dist/node/chunks/dep-CHZK6zbr.js:52974:11 Click outside, press Esc key, or fix the code to dismiss. You can also disable this overlay by setting server.hmr.overlay to false in vite.config.ts.

This is in a react-router app using bun as a package manager and ts set to ESNEXT.

@kdcokenny
Copy link
Author

This looks like this is not your fault though. My old package I used zipped with .tgz stopped working with the same error mentioned before. Not sure why that's happening lol.

@kdcokenny
Copy link
Author

Update: I got the issue beforehand fixed it was an issue with my Vite config. I also downloaded your package with the cmd above and used it like this:

      emailHarmony({
        schema: {
          user: {
            fields: {
              normalizedEmail: "normalized_email"
            }
          }
        }
      }),

However whenever I put a temp email in like foo+test@gmail.com the database contents were always null. When I switched back to my pkg in this it worked properly but with the less-optimal design:

      emailHarmony({
        normalizedEmailField: "normalized_email"
      })

@GeKorm
Copy link
Owner

GeKorm commented Mar 24, 2025

I think we'll have to do it your way then, as it might be a better-auth bug, similar to better-auth/better-auth#1414 where the mappings don't work correctly.

May I ask what database and/or adapter you're using please? Asking because the tests pass on SQLite and I want to add the failing case to the test suite.

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.

Custom field name panic
2 participants