Tahul / nuxt-edgedb

💽 Nuxt 3 integration for EdgeDB
71 stars 7 forks source link

Server modules not getting auto-imported #2

Closed juni0r closed 1 year ago

juni0r commented 1 year ago

When using auth api handlers (such as login) fails due to server modules not being auto-imported. Reproduction repo.

[nuxt] [request error] [unhandled] [500] useEdgeDbPKCE is not defined
  at <anonymous> (./node_modules/nuxt-edgedb-module/dist/runtime/api/auth/login.mjs:3:16)  
  at Object.handler (./node_modules/h3/dist/index.mjs:1851:28)  
  at Object.handler (./node_modules/h3/dist/index.mjs:1675:31)  
  at async Server.toNodeHandle (./node_modules/h3/dist/index.mjs:1885:7)

https://github.com/Tahul/nuxt-edgedb/blob/b8338ed322dd14aba0cc9f7a2e0402f356e1d27e/src/runtime/api/auth/login.ts#L2-L6

The modules in src/runtime/server should be auto-imported since the module adds them to Nitro's import paths, but apparently something goes wrong. I'd love to investigate further but I'm not too familiar with Nuxt 3 modules yet. I suspect it's to do with the import folder containing .mjs files instead of .ts.

Tahul commented 1 year ago

Hey @juni0r ; thanks for reporting the issue!

I just published 0.0.9 that uses raw imports in runtime/ instead of relying on auto-imports.

That should avoid this kind of issue!

Would you mind keeping me updated on this, I'm very happy you are planning on using the module!

juni0r commented 1 year ago

Hey @Tahul, thanks again for the swift release! I'm afraid you've missed a few instances, like this one:

https://github.com/Tahul/nuxt-edgedb/blob/09242595daf424e27532ef94d35f3577535f3c5b/src/runtime/api/auth/signup.ts#L10-L11

I'm also getting an 'unknown path' error from the auth server when using api/auth/login and I need to investigate if it's an issue with my setup or the module. I'll keep you posted.

Tahul commented 1 year ago

Hey @juni0r ; I should have fixed the issue with useEdgeDbPKCE() on all occurrences, sorry for the mismatch.

The fix is released under 0.0.11.

Tahul commented 1 year ago

https://github.com/Tahul/nuxt-edgedb/blob/f3b4fd59219af1dd1dc38b020aa2cf568d7e1076/src/module.ts#L450-L453

Here is the part of the. code that injects /api/auth/login.

I don't see why, if options.auth set to true, these routes would not be available.

If needed, you can try to debug it by injecting the routes on your own by doing that:

import { createResolver, addServerHandler } from '@nuxt/kit'

const { resolve } = createResolver(import.meta.url)

export default defineNuxtConfig({
    modules: [
        function (nuxt) {
            addServerHandler({
                handler: resolve('./node_modules/nuxt-edgedb-module/dist/runtime/api/login'),
                route: '/api/auth/login',
            })
        }
    ]
})

That should inject the route and let you debug it.

You can also copy the content of that route to another file and map it the same way to go further.

Let me know if I can help any further, and do not hesitate to provide a reproduction, I would happily go through it!

juni0r commented 1 year ago

I don't see why, if options.auth set to true, these routes would not be available.

The server route is available. It contacts the edgedb auth server which returns 'path not found'.

I tried to investigate further, but I'm still struggling to run the module playground. From a quick glance at the code, I noticed that the auth server URL is built differently from what's in the docs:

http[s]://{edgedb_host}[:port]/db/{db_name}/ext/auth/

The /db/ part is missing when building the base URL:

https://github.com/Tahul/nuxt-edgedb/blob/f3b4fd59219af1dd1dc38b020aa2cf568d7e1076/src/module.ts#L433-L434

However, the default value for the auth URL does include /db/

https://github.com/Tahul/nuxt-edgedb/blob/f3b4fd59219af1dd1dc38b020aa2cf568d7e1076/src/runtime/server/useEdgeDbEnv.ts#L9

Tahul commented 1 year ago

Damn, good catch @juni0r

I just fixed it in 0.0.12, thanks!

juni0r commented 1 year ago

I got the playground running but I had to install a few packages in the playground project and even one in the root project. These were all dependencies of @nuxt/ui and I'm still getting errors occasionally.

It might be a consideration to remove @nuxt/ui as a dependency and go with plain css. It could be an obstacle to contributors when setting up their local dev project. The playground should focus on the module being tested and not have too many moving parts. Nuxt UI with Tailwind is quite a heavy dependency and in my case just caused trouble.

I also looked at the import paths again and I would suggest to use a path alias for the server module, such as #edgedb/server and use that consistently.

If you would like to have a look at my changes, I'd be happy to send you a PR.

Tahul commented 1 year ago

I would love to see your PR.

I just updated the aliases paths to be consistent (0.0.13).

Totally agree with you on the @nuxt/ui part, I would love a PR that removes it and switches to plain CSS.

I first built the playground using @nuxt/ui just for the sake of doing a demo and also trying myself that UI kit, then included it into the repository.

I think we would be better with a very minimal playground, built on top of PicoCSS or any other 1-liner CSS framework.

juni0r commented 1 year ago

After doing a clean install of the playground, it suddenly worked without issues. Must have been a glitch in my installation. So it turns out @nuxt/ui wasn't the culprit after all.

I submitted the PR yesterday. It's nice how it fits with the aliases you've added in the meantime.

Tahul commented 1 year ago

Hey @juni0r ; could you take a look at 0.0.15 and let me know if it fixes your issue?

Thank you so much for both PRs, I left a comment on #5, you might want to answer here. :)

I'm happy to count you as first contributor on this module, let's prove out the potential of EdgeDB x Nuxt together!