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

[Bug?]: dup signIn import from @solid-mediaket/auth -> build error #1515

Closed
2 tasks done
billmartschenko opened this issue May 28, 2024 · 5 comments
Closed
2 tasks done
Labels
bug Something isn't working

Comments

@billmartschenko
Copy link

billmartschenko commented May 28, 2024

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Doing a signIn on the top level index page for a site.

Works fine if I import this way.

import { Session } from '@solid-mediakit/auth';
import { createSession, signIn } from '@solid-mediakit/auth/client';

Fails if I import this way.

import { Session, signIn } from '@solid-mediakit/auth';
import { createSession } from '@solid-mediakit/auth/client';

I didn't see documentation describing 2 ways to signIn. The change in import was inadvertent and happened via automatic import recommendation in vscode. Maybe doc issue or misread on my part. Seems subtle.

The failure is the attempted use of a server side node module in the client when I do pnpm build which is the same as vinxi build.

 ERROR  node_modules/.pnpm/vinxi@0.3.11_@opentelemetry+api@1.8.0_@types+node@20.12.12_ioredis@5.4.1_terser@5.31.0/node_modules/vinxi/runtime/http.js (70:9): "AsyncLocalStorage" is not exported by "__vite-browser-external", imported by "node_modules/.pnpm/vinxi@0.3.11_@opentelemetry+api@1.8.0_@types+node@20.12.12_ioredis@5.4.1_terser@5.31.0/node_modules/vinxi/runtime/http.js".

The import definitions are different.
index.d.ts (fails)
/Users/billm/dev/neutron/hoffa/frontend/dash/webapp/sstart/node_modules/.pnpm/@solid-mediakit+auth@2.0.11_@auth+core@0.29.0_@solidjs+meta@0.29.4_solid-js@1.8.17__@solidjs+_po46vxxkmqdaywwmto3pszjmbi/node_modules/@solid-mediakit/auth/index.d.ts

export declare function signIn(provider: SignInParams[0], options: SignInParams[1], authorizationParams: SignInParams[2], config: SolidAuthConfig, event: APIEvent): Promise<any>;

client.d.ts (works fine)
/Users/billm/dev/neutron/hoffa/frontend/dash/webapp/sstart/node_modules/.pnpm/@solid-mediakit+auth@2.0.11_@auth+core@0.29.0_@solidjs+meta@0.29.4_solid-js@1.8.17__@solidjs+_po46vxxkmqdaywwmto3pszjmbi/node_modules/@solid-mediakit/auth/client.d.ts

export declare function signIn<P extends RedirectableProviderType | undefined = undefined>(providerId?: LiteralUnion<P extends RedirectableProviderType ? P | BuiltInProviderType : BuiltInProviderType>, options?: SignInOptions, authorizationParams?: SignInAuthorizationParams): Promise<Response>;

Expected behavior 🤔

I expected a single way to import the signIn function or better doc to show 2 ways to do it. Intuitively, makes sense to have 2 ways for client side and server side.

For me, I didn't read as an import from @solid-mediakit/auth and @solid-mediakit/auth/client to capture the distinction between server-side auth and client-side auth.

Steps to reproduce 🕹

Steps:

  1. pnpm create solid. (project name: test1; is solid start: Yes; template: with-authjs; typescript: Yes)
  2. cd test1. Edit src/routes/index.tsx to import signIn from "@solid-mediakit/auth" instead of the provided "@solid-mediakit/auth/client".
  3. pnpm i && pnpm build

Context 🔦

I was doing code refactoring after getting the with-authjs template working. I wasn't even working with auth at the time. I had introduced some other components and needed to refactor. As I imported signIn in a .tsx, the recommendation from vscode was the wrong one. I didn't even notice and had to bisect current code vs the original template to isolate.

Your environment 🌎

**System:**
- OS: macOS 13.6.7 (22G720)
- CPU: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz

**Binaries:**
- Node: 20.12.2
- pnpm: 9.1.2

**npmPackages:**

dependencies:
@auth/core 0.29.0
@babel/core 7.24.4
@solid-mediakit/auth 2.0.11
@solidjs/meta 0.29.4
@solidjs/router 0.13.3
@solidjs/start 1.0.0
solid-js 1.8.17
vinxi 0.3.11

devDependencies:
@types/node 20.12.12
esbuild 0.20.2
next-auth 4.24.7
postcss 8.4.38
typescript 5.4.5
vite 5.2.11
@billmartschenko billmartschenko added the bug Something isn't working label May 28, 2024
@LukasGerm
Copy link
Contributor

I mean, is this now a bug in @solid-mediakit or just unclear behaviour? Because as far as I can understand now it works as expected but you were just not aware that two exports exist?

@davedbase
Copy link
Member

Hi there, pinging @OrJDev to support on this. Was a similar issue opened in MediaKit?

@OrJDev
Copy link
Contributor

OrJDev commented May 28, 2024

Basically when importing from "/" - @solid-mediakit/auth is a server file, functions imported from here are server functions we use under the hood, you shouldn't import such functions from there.

the signIn function on /client dir is the actual function you should be using, it fully supports ssr, just use it - @solid-mediakit/auth/client

Related issue

solidjs-community/mediakit#60

This has nothing to do with the solid start repo, its just me creating 2 functions with the same name and exporting both /:

@billmartschenko
Copy link
Author

billmartschenko commented May 28, 2024

Yep, @OrJDev , I agree. The / equals internal and /client for both server and client functionality was not my expectation. It still wouldn't be, ha ha. Is there not a way to prevent the internal stuff from being visible? This all turned into a snakebite when I was just typing import statements and the autocompletion in vscode pulled from /.

Yes, @LukasGerm, I was not aware. More specifically, per @OrJDev, I was not aware to use the 2nd but not the 1st. That's what helped me not discover quickly what had happened.

I appreciate the feedback. I'm not pressing for a bug. This cost me a bit of time and my understanding was not clear even though I found a solution. I'm not a fan--just my opinion--that / is internal and /client is for both client and server functionality.

@ryansolid
Copy link
Member

Ok since this is not related to SolidStart directly I will close. Follow up in that repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants