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

wondering about the User type in examples/hono/github-oauth/routes/index.ts #38

Open
bkerin opened this issue Jul 2, 2024 · 12 comments

Comments

@bkerin
Copy link

bkerin commented Jul 2, 2024

I'm trying to get this example working with Deno and Hono while being pretty new to typescript etc. so sorry if this issue is obvious.

I first changed some tiny things in lib to get it compiling with Deno. Then I started on routes/, and I now get this warning from deno check:

error: TS2339 [ERROR]: Property 'username' does not exist on type 'User'.
    const html = template.replaceAll("%username%", user.username).replaceAll("%user_id%", user.id)

From that it seems like deno is picking up at least some of the types for Lucia but the User type is not as expected or is not correct somehow. I am wondering what is the mechanism by which the username property is expected to be pulled in? I see that username is a db field but don't understand the mechanism by which it becomes a property of User in this context. I think maybe in this context User is actually known to be of a descendant type and I also need to tell typescript about this somehow?

Any advice appreciated

@pilcrowonpaper
Copy link
Member

Did it work before you switched to Deno? I've never used Deno so I'm not sure if I can help

@bkerin
Copy link
Author

bkerin commented Jul 4, 2024

I've never tried it not on deno. https://lucia-auth.com says it works on deno but admittedly I'm pretty new to deno (as well as javascript generally) so maybe I'm missing something that normally required to get the types imported right

@pilcrowonpaper
Copy link
Member

Did you properly rewrite declare module? Should be npm:lucia instead of lucia.

declare module "npm:lucia" {
	interface Register {
		Lucia: typeof lucia;
		DatabaseUserAttributes: Omit<DatabaseUser, "id">;
	}
}

@bkerin
Copy link
Author

bkerin commented Jul 4, 2024

Yes, Following the definition chain (via :ALEGoToDefinition) I eventually hit this chunk in my version of lib/auth.ts:

declare module "npm:lucia" {
	interface Register {
		Lucia: typeof lucia;
		DatabaseUserAttributes: Omit<DatabaseUser, "id">;
	}
}

There were a couple other places in my code where as a result of following the examples on the lucia doc pages I had this though:

declare module "npm:lucia" {
	interface Register {
		Lucia: typeof lucia;
	}
}

ALE offered them as options when following the definition chain from User type (with :ALEGoToDefinition). My understanding of what declare modules is doing is pretty poor but I guess it's declaring ambient types so I guess there's really only supposed to be one of these. So I went through and nuked the other ones in my code, and that make the error vanish from ALE, but it still shows up in deno check. There's still the root definition from node_modules/.deno/[email protected]/node_modules/lucia/dist/index.d.ts offered:

export interface Register {
}

So maybe deno is for some reason stuck on that one?

I really appreciate your taking time to look at this, I haven't fealt this noob at software in a long time as I have trying to get a grip on the modern javascript/typescript scene. The sad thing is deno looks a heck of a lot easier and cleaner from new person perspective, but I get a little bit of a scary lonely feeling on it. I'm curious how many people you know have lucia working on deno?

@pilcrowonpaper
Copy link
Member

Yeah I'm not sure if I can help further, especially since I got mine working in Deno after fixing the module declaration

@bkerin
Copy link
Author

bkerin commented Jul 4, 2024

I'll try the example exactly

@bkerin
Copy link
Author

bkerin commented Jul 4, 2024

Is your working setup on better-sqlite3? I couldn't get that to work on deno even independently: WiseLibs/better-sqlite3#1205

If you ever get a chance to tar up your working setup I'd love to look at it

@pilcrowonpaper
Copy link
Member

better-sqlite3 only works in Node

@bkerin
Copy link
Author

bkerin commented Jul 7, 2024

I looked at this a little more and type User in routes/index.ts seems right at that point anyway. I added a type expander type thingy as described here: https://stackoverflow.com/questions/57683303/how-can-i-see-the-full-expanded-contract-of-a-typescript-type and modified routes/index.ts to look like this:

//import fs from "node:fs/promises";
import { Hono } from "hono";

import type { User } from "npm:[email protected]";
import type { Context } from "../lib/context.ts";

export const mainRouter = new Hono<Context>();

// expands object types one level deep
type Expand<T> = T extends infer O ? { [K in keyof O]: O[K] } : never;

mainRouter.get("/", async (c) => {
	const user = c.get("user");
	if (!user) {
		return c.redirect("/login");
	}
	const template = "fake_template"
        let expandedUser: Expand<User>
	const html = template.replaceAll("%username%", user.username).replaceAll("%user_id%", user.id);
	return c.html(html, 200);
});

:ALEHover on expandedUser then shows:

let expandedUser: {
    id: string;
    githubId: number;
    username: string;
}

But deno check routes/index.tx still says:

Check file:///home/bkerin/tmp/deno_test_project/routes/index.ts
error: TS2339 [ERROR]: Property 'username' does not exist on type 'User'.
	const html = template.replaceAll("%username%", user.username).replaceAll("%user_id%", user.id);
	                                                    ~~~~~~~~
    at file:///home/bkerin/tmp/deno_test_project/routes/index.ts:19:54

It seems like deno check is somehow seeing a different version of the User type. Note that I had to add an import line
import type { User } from "npm:[email protected]"; to directly use the User type, so maybe it somehow ends up with a different meaning at this point. But it's also strange that the editor linter doesn't show the error, while deno check does. I have only deno linter enabled in ALE configuration, so this seems like a flat contradiction between the deno linter and deno check.

Does any of that suggest any next thing to try?

@pilcrowonpaper
Copy link
Member

Ah ok deno check not the editor. Strange

@pilcrowonpaper
Copy link
Member

Does running deno check index.ts --all fix the issue?

@bkerin
Copy link
Author

bkerin commented Jul 8, 2024

No, same result. I hit a crash in deno while trying to do all the equivalent stuff with import maps rather than npm specifiers in the import statements, between that, the fact that the User type looks right, and the fact that the linter and check seem to disagree I suspect at this point that this is some deno issue probably in the npm compat layer. I don't know what my odds are of isolating it well enough that they'll look at it though.

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

No branches or pull requests

2 participants