[TypeORM adapter] refresh_token type suggests non-nullable but column is nullable

[trptcolin]

Adapter type

@next-auth/typeorm-legacy-adapter

Environment

System: OS: macOS 12.1 CPU: (8) arm64 Apple M1 Memory: 84.88 MB / 16.00 GB Shell: 5.8 - /opt/homebrew/bin/zsh Binaries: Node: 17.1.0 - ~/.asdf/installs/nodejs/17.1.0/bin/node npm: 8.1.2 - ~/.asdf/plugins/nodejs/shims/npm Browsers: Chrome: 98.0.4758.102 Firefox: 94.0.2 Safari: 15.2 npmPackages: next: ^12.0.10 => 12.0.10 next-auth: ^4.2.1 => 4.2.1 react: ^17.0.2 => 17.0.2

Reproduction URL

https://github.com/nextauthjs/next-auth

Describe the issue

I believe refresh_token should have a type of string | null (similar to access_token), since it's designated as nullable:

https://github.com/nextauthjs/next-auth/blob/83a9867cbcca95a50e4df3a6b8666ae8391db078/packages/adapter-typeorm-legacy/src/entities.ts#L59-L66

Otherwise, consumers can assume it's safe to process that value as a string, but it might not even be present.

I hit this when providing my own entity (as described in the docs) and making sure the types and database schema lined up as expected. And as you'd expect, when I attempt to use string | null which I think is the correct typing, there's a type mismatch:

Types of property 'refresh_token' are incompatible.
  Type 'string | null' is not assignable to type 'string'.
    Type 'null' is not assignable to type 'string'.

How to reproduce

Create a SessionEntity row with a NULL value for refresh_token.

Attempt to call string methods on null, and it'll fail at runtime.

Stripped-down example ignoring the database altogether to focus on the types which are the issue:

let x: string = " asdf " as any; // put a real string into a string variable
let y: string = null as any; // put a null into a string variable
x.trim(); // typechecks and works at runtime
y.trim(); // typechecks and FAILS at runtime

Expected behavior

Again using the stripped-down example,

let x: string | null = " asdf " as any;
let y: string | null = null as any;
x.trim(); // doesn't typecheck - consumer must account for null case
y.trim(); // doesn't typecheck - consumer must account for null case

x?.trim() // typechecks; works at runtime
y?.trim() // typechecks; works at runtime

[balazsorban44]

This seems like a minor typo in the docs. Yes refresh_token is optional. Want to open a quick PR to fix?

[trptcolin]

It's not just docs though, right? Because any custom types need to type check against the default entities:

https://github.com/nextauthjs/next-auth/blob/83a9867cbcca95a50e4df3a6b8666ae8391db078/packages/adapter-typeorm-legacy/src/index.ts#L9-L18

(note that the type refers to the default entities)

and because the type string | null (the correct type) isn't substitutable for the current type string, clients would need to type custom implementations for this field as any (or alternatively, use the given string and use casts) in order to get it to typecheck.

I did see that there's an additional listing in docs where these entities appear, I'm just calling out that it for sure creates either compile-time errors, or opens the door for potential runtime errors for this use case. I'm typing as any and casting at call sites for now, so I'm not blocked or anything. I won't have a chance to open a PR soon for this, but may get to it eventually.

[trptcolin]

Actually looks like I've got some availability today, so will have a PR up for code and docs shortly.