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.

Sign up for GitHub

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

Jump to bottom

Fixed ability to install from Github + Fixed typo #172

Merged
merged 7 commits into from
Aug 9, 2023
Merged

Fixed ability to install from Github + Fixed typo #172

merged 7 commits into from
Aug 9, 2023

Conversation

amerkay
Copy link
Contributor

@amerkay amerkay commented Jul 15, 2023 •
edited

I was trying to get acceptInvite, and inviteUsers functions to work, so:

  1. I needed to install with pnpm add github:Intevel/nuxt-directus. This is when I got the error that tsconfig.json extends does not exist. Fixed by having the minimal required config instead.

File ./playground/.nuxt/tsconfig.json does not exists.

  1. Fixed a typo: : inviteAccept renamed to acceptInvite.

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes (if not applicable, please state why)

@amerkay
Updated tsconfig.json.
a80fc92 
File ./playground/.nuxt/tsconfig.json does not exists.

Typo fix: inviteAccept renamed to acceptInvite.
@vercel
Copy link

vercel bot commented Jul 15, 2023 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nuxt-directus ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 3, 2023 3:16pm

@amerkay
Copy link
Contributor Author

amerkay commented Jul 16, 2023 •
edited

Hi again,

Another commit adds 2 config vars: sameSiteRefreshToken and isSecureRefreshToken, 013437d

This is to fix having Same-Site = "none" with secure=false, which shows a warning in console logs.

Docs will need to be updated accordingly. Update: Added to docs.

Cheers!

@amerkay
[BUG FIX] Run autoFetch + autoRefresh on plugin setup, without 'app:c…
262fc97 
…reated' hook.

As the 'app:created' hook is not called on SSR=true (static generation).
@amerkay
Renamed cookie params + update docs.
3305e89 
- cookieMaxAge (from maxAgeRefreshToken)
- cookieSameSite (from sameSiteRefreshToken)
- cookieSecure (from isSecureRefreshToken)
@amerkay
Copy link
Contributor Author

amerkay commented Jul 16, 2023

More commits:

  1. Bug fix: In nuxt generate SSR mode, the hook app:created is called after the auth middleware (as in the docs). Which made the auth middeware malfunction, as useDirectusUser() would still be null.
  2. Renamed the added cookie variables, and update docs:
    • cookieMaxAge (from maxAgeRefreshToken) <- Note, this is a breaking change
    • cookieSameSite (from sameSiteRefreshToken)
    • cookieSecure (from isSecureRefreshToken)

@Intevel Volta.net
Copy link
Owner

Intevel commented Jul 17, 2023

@amerkay Thanks for this, great work. I will review it in the next days, are there any breaking changes?

@amerkay
Copy link
Contributor Author

amerkay commented Jul 17, 2023

@amerkay Thanks for this, great work. I will review it in the next days, are there any breaking changes?

Thanks @Intevel!

Only this (see my last comment):

Renamed config var to cookieMaxAge (from maxAgeRefreshToken)

Cheers!

@casualmatt
Copy link
Collaborator

casualmatt commented Jul 18, 2023 •
edited

@amerkay
is the change from maxAgeRefreshToken to cookieMaxAge just a rename, or is there more to it?

@amerkay
Copy link
Contributor Author

amerkay commented Jul 18, 2023

@amerkay is the change from maxAgeRefreshToken to cookieMaxAge just a rename, or is there more to it?

Just a rename, as it's not really just the for the RefreshToken, it's for all 3 cookies. See modifications in useDirectusToken composable. Note this is before the rename.

@casualmatt
Copy link
Collaborator

casualmatt commented Jul 18, 2023 •
edited

@amerkay I have re-introduced the support for maxAgeRefreshToken to keep backward compatibility, we will remove it in future versions.

Can you proceed and double-test it so that we are sure with both settings used, the behavior is the same?

@amerkay
[FEATURE] Added an export for setAuthCookies().
9be624f 
This way I can create a custom login function.

> TODO: Add code and tutorial for passwordless login using Directus 10.
amerkay
amerkay commented Jul 19, 2023
View reviewed changes
Copy link
Contributor Author

@amerkay amerkay left a comment

Choose a reason for hiding this comment

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

Tested on my own project. All cookies are set to the correct date with the correct secure and SameSite params.

@amerkay
Copy link
Contributor Author

amerkay commented Aug 3, 2023

Fixed minor conflict.

@Intevel Intevel requested a review from casualmatt August 9, 2023 18:35
Intevel
Intevel approved these changes Aug 9, 2023
View reviewed changes
Copy link
Owner

@Intevel Intevel left a comment

Choose a reason for hiding this comment

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

LGTM, good work

@Intevel Intevel merged commit e7355bf into Intevel:main Aug 9, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Intevel Intevel Intevel approved these changes

@casualmatt casualmatt casualmatt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@amerkay @Intevel @casualmatt