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

Ignore _middleware routes #262

Merged

Conversation

johnsardine
Copy link
Contributor

@johnsardine johnsardine commented Jan 12, 2022

Next.js 12 introduced Middlewares, this consists in _middleware files existing in the pages folder. Currently next-sitemap does not know it should ignore these files because they don't necessarily live at the root folder like other next.js files, so this change updates the isNextInternalUrl to consider paths with _middleware at the end of the path as internal urls.

Copy link
Owner

@iamvishnusankar iamvishnusankar left a comment

Choose a reason for hiding this comment

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

@johnsardine Once again, Thanks a lot for one more awesome PR!!

Can you please add test case which identifies middleware routes with url params. /projects/[id]/_middleware

It would make the regex bit more solid.!

@johnsardine
Copy link
Contributor Author

@johnsardine Once again, Thanks a lot for one more awesome PR!!

Can you please add test case which identifies middleware routes with url params. /projects/[id]/_middleware

It would make the regex bit more solid.!

I added the exact path you suggested. Before comitting I tested the following folder structure

Screenshot 2022-01-12 at 16 55 07

And interestingly the sitemap only ended up containing one _middleware path, coming from /project/_middleware. Paths with query only contained the path with the id /project/one and /project/two in my example. No _middleware in this case. But it's good to know the regexp still handles it correctly

Copy link
Owner

@iamvishnusankar iamvishnusankar left a comment

Choose a reason for hiding this comment

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

@johnsardine Yes, it already takes care of url params, that's why the /projects/[id]/_middleware is being filtered out. I just suggested to add this test case to make it bit more solid :)

Thanks for adding the new test case and thanks for all your contributions so far. Approved!

@iamvishnusankar iamvishnusankar merged commit cc2036a into iamvishnusankar:master Jan 12, 2022
@johnsardine
Copy link
Contributor Author

@johnsardine Yes, it already takes care of url params, that's why the /projects/[id]/_middleware is being filtered out. I just suggested to add this test case to make it bit more solid :)

Thanks for adding the new test case and thanks for all your contributions so far. Approved!

Got it. You’re welcome. Thanks for the prompt review and merge

@johnsardine johnsardine deleted the ignore-middleware-paths branch January 12, 2022 17:44
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.

None yet

2 participants