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

fix: update lucia adder #254

Merged
merged 13 commits into from
Oct 31, 2024

Conversation

pilcrowonpaper
Copy link
Contributor

  • Adds setSessionTokenCookie() and deleteSessionTokenCookie() (closes lucia: add setSessionTokenCookie and deleteSessionTokenCookie methods #122)
  • Fixes sessions to be consistent with lucia-auth.com. This isn't a security issue, but the hashing part wasn't implement properly which negated the benefits it provided.
  • Update generateUserId() to be consistent with generateSessionToken() and easier to deal with.
  • Removed return types to be consistent with the rest of the codebase.

I haven't added change sets yet and I'm not sure how to test the adder.

Copy link

changeset-bot bot commented Oct 30, 2024

⚠️ No Changeset found

Latest commit: 7e39333

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 30, 2024

Open in Stackblitz

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

commit: 7ff677f

@manuel3108
Copy link
Member

Cool, thanks!

You can test the adder by running pnpm sv add lucia insider the pnpm workspace.
Or use pnpx https://pkg.pr.new/sveltejs/cli/sv@254 add lucia which was published instants ago

@pilcrowonpaper
Copy link
Contributor Author

The generated TypeScript code works. I can't seem to get sv to generate JSDoc code tho. Is this expected?

@AdrianGonz97
Copy link
Member

AdrianGonz97 commented Oct 30, 2024

That's expected. The JSDoc in this instance is only included when typescript isn't present in the project (or when using JSDoc just for type-checking).

@pilcrowonpaper
Copy link
Contributor Author

Tested out with the published package and all the types seems to be working in both the TS and JSDoc version

Copy link
Member

@AdrianGonz97 AdrianGonz97 left a comment

Choose a reason for hiding this comment

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

much appreciated!

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.

Any reason this is still a draft?

@AdrianGonz97 AdrianGonz97 marked this pull request as ready for review October 31, 2024 22:19
@AdrianGonz97 AdrianGonz97 merged commit 99f3e04 into sveltejs:main Oct 31, 2024
5 checks passed
@github-actions github-actions bot mentioned this pull request Oct 31, 2024
@benmccann
Copy link
Member

thank you!! this is a big help

we're hoping to add more things like oauth in the future. we're working on cleaning up some of our APIs to hopefully make it easier to add and maintain add-ons.

really appreciate you taking a look!

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.

lucia: add setSessionTokenCookie and deleteSessionTokenCookie methods
4 participants