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

feat(app): add include to RedirectOptions #336

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

jojomatik
Copy link
Contributor

Closes #268

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

Add an include option to RedirectOptions. If set, only routes matching the supplied regex are considered for redirects.

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)

Copy link
Contributor

nuxt-studio bot commented Mar 4, 2024

Live Preview ready!

Name Edit Preview Latest Commit
supabase Edit on Studio ↗︎ View Live Preview 037f804

Copy link

netlify bot commented Mar 4, 2024

👷 Deploy request for n3-supabase pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 726ba14

@jojomatik jojomatik changed the title feat: add include to RedirectOptions feat(app): add include to RedirectOptions Mar 4, 2024
Copy link
Collaborator

@larbish larbish left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! LGTM except two little things. If you can just update, I'll merge then

const { cookieName, cookieOptions } = config

// Redirect only on included routes (if defined)
if (include && include.length > 0) {
const isIncluded = [...include].some((path) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const isIncluded = [...include].some((path) => {
const isIncluded = include.some((path) => {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you create pages/protected.vue instead? (just to be consistent with the unprotected route)

@jojomatik
Copy link
Contributor Author

Thanks for the feedback. I've force pushed with the two changes :)

@jojomatik jojomatik requested a review from larbish March 6, 2024 19:27
@larbish larbish merged commit f9aad55 into nuxt-modules:main Mar 7, 2024
@jojomatik jojomatik deleted the feat/redirect-include branch March 7, 2024 22:16
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.

Everything to be excluded in redirectOptions except one folder
2 participants