Skip to content

Conversation

AdrianGonz97
Copy link
Member

@AdrianGonz97 AdrianGonz97 commented Oct 12, 2024

closes #68

Outstanding questions:

  1. What do we call it?
    It's effectively still 'Lucia', just without the library. We could call it 'Oslo', but it has less name recognition and I don't think that would be technically correct either. Oslo provides some of the auth utilities (mainly the encoding and crypto bits), but it doesn't touch sessions.

Copy link

changeset-bot bot commented Oct 12, 2024

⚠️ No Changeset found

Latest commit: da14504

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

pkg-pr-new bot commented Oct 12, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/sveltejs/cli/sv@98

commit: da14504

Copy link
Member

@manuel3108 manuel3108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting the dev server gives me the following error, though i'm unable to locate what it thinks is wrong: I was currently testing on a JS project, if that helps

Details

4:14:18 PM [vite] Pre-transform error: Expected '{', got 'type'
4:14:19 PM [vite] Error when evaluating SSR module /src/lib/server/auth.js:
|- RollupError: Expected '{', got 'type'
at getRollupError (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:392:41)
at convertProgram (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:1070:26)
at parseAstAsync (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:1924:93)
at async ssrTransformScript (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:52319:11)
at async loadAndTransform (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:51917:72)
at async instantiateModule (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:52817:44)

4:14:19 PM [vite] Error when evaluating SSR module /src/hooks.server.js:
|- RollupError: Expected '{', got 'type'
at getRollupError (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:392:41)
at convertProgram (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:1070:26)
at parseAstAsync (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:1924:93)
at async ssrTransformScript (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:52319:11)
at async loadAndTransform (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:51917:72)
at async instantiateModule (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:52817:44)

Error [RollupError]: Expected '{', got 'type'
at getRollupError (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:392:41)
at convertProgram (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:1070:26)
at parseAstAsync (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:1924:93)
at async ssrTransformScript (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:52319:11)
at async loadAndTransform (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:51917:72)
at async instantiateModule (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:52817:44) {
code: 'PARSE_ERROR',
pos: 518
}
Error [RollupError]: Expected '{', got 'type'
at getRollupError (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:392:41)
at convertProgram (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:1070:26)
at parseAstAsync (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/rollup@4.22.5/node_modules/rollup/dist/es/shared/parseAst.js:1924:93)
at async ssrTransformScript (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:52319:11)
at async loadAndTransform (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:51917:72)
at async instantiateModule (file:///D:/dev/web/svelte-cli-temp/node_modules/.pnpm/vite@5.4.8_@types+node@22.7.5/node_modules/vite/dist/node/chunks/dep-CDnG8rE7.js:52817:44) {
code: 'PARSE_ERROR',
pos: 518
}

Additionally it generates this while running in a js project

// src\lib\server\db\schema.js
export type Session = typeof session.$inferSelect;
export type User = typeof user.$inferSelect;

@AdrianGonz97
Copy link
Member Author

This is in a better state now. Opened for further reviews and can merge once ready

@benmccann
Copy link
Member

Perhaps we should move some of the logic into a session.ts to match the example repo, which I find to help with organization: https://github.com/lucia-auth/example-sveltekit-email-password-2fa/blob/7f9f22d48fb058e4085a11c52528e781c4bc70df/src/lib/server/session.ts

As far as name, I agree Oslo doesn't feel right. I think it would make sense in the short-term, but as we add more functionality like OAuth, etc. we'll start using more libraries and then it wouldn't fit anymore. I think we could probably just call it "Auth". It wouldn't make sense to have "Auth" under "Auth" with the current main screen, but I think we could tweak that. E.g. put it under "Additional functionality" and then it would be fine

@AdrianGonz97
Copy link
Member Author

hmmm, I don't know. I think there's still value in calling it "Lucia" even if lucia isn't a direct dependency anymore. We're still following their model and guidance.

I'd like to think of it as if I were to make a shadcn-svelte extension/integration. Sure, there's no shadcn-svelte dependency, but the parts being installed are still coming from the project. I certainly wouldn't call it bits-ui or styled-component-library.

@benmccann
Copy link
Member

hmmm, I don't know. I think there's still value in calling it "Lucia" even if lucia isn't a direct dependency anymore. We're still following their model and guidance.

It's true that we are for now, but what if we want to diverge a bit? E.g. we may offer things that aren't in their guide, change how things are setup, etc.

@benmccann
Copy link
Member

I wonder actually if we should just go ahead and setup https://svelte.dev/docs/kit/auth as a URL to point to. It kind of feels like a nice thing for us to own. Let me try prototyping that real quick and see what it would look like

Copy link
Member

@manuel3108 manuel3108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the generated +page.svelte and +page.server.js have two line breaks after the imports.

I have never seen a one liner for jsdoc comments (expect for @type) but it works, so I personally wouldn't bother.

/** @returns {string} */
function generateSessionToken() {

Apart from that everything seems to be working. Also tested the demo which ran just fine

@AdrianGonz97 AdrianGonz97 merged commit e1c31d7 into main Oct 14, 2024
5 checks passed
@AdrianGonz97 AdrianGonz97 deleted the chore/update-lucia branch October 14, 2024 21:05
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.

Remove Lucia from the lucia adder

3 participants