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

docs: document trailingSlash #2546

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

docs: document trailingSlash #2546

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/content/3.options/2.routing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ Internal suffix added to generated route names for default locale, if strategy i

Internal separator used for generated route names for each locale. You shouldn't need to change this.

## `trailingSlash`

- type: `boolean`
- default: `false`

Whether routes have a trailing slash
Comment on lines +116 to +121
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very sparse. How does that interact with Nuxt's own trailingSlash option? Does it apply to all routes or only those handled by i18n?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree the description is lacking, still need to familiarize myself with the routing logic more so I can't answer those questions yet! I figured this is still better than leaving it undocumented 😅..

The code it uses is ported from your code in v7 to vue-i18n-routing, unless it has changed a lot, you may be better able to answer those questions than I am.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Version 7 of this module (for Nuxt 2) doesn't use vue-i18n-routing so I'm not familiar with it.

I'm a bit surprised that trailingSlash option was introduced in context of Nuxt module since IMO this should all be controlled through Nuxt's own setting instead of having two, potentially conflicting, options.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I understand that v7 doesn't use vue-i18n-routing, it's vue-i18n-routing that is based on routing in v7.

As far as I can tell, the reason for configuring it in the module is because Nuxt 3 got rid of the trailingSlash option. Now it's up to users to create a custom NuxtLink component see: https://nuxt.com/docs/api/components/nuxt-link#overwriting-defaults.

Though 3.8 introduced a way to set these NuxtLink defaults in the nuxt config, so we could consider using that config.

Also not sure what happens if users have multiple custom NuxtLink components with different trailingSlash settings, when and why this would be used, and how we would handle such edge cases 🤷‍♂️.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. Then maybe it's just the way to go and I'm making unnecessary noise.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you follow the history in blame, you can find this github issue.
nuxt/nuxt#18806

That issue is added by NuxtLink to improve SEO for internal links.

Follow the issue further and you can find this issue.
nuxt/nuxt#15462

That issue is a proposal to support trailing Slash from Nuxt 2.
https://v2.nuxt.com/docs/configuration-glossary/configuration-router#trailingslash

As you can see from reading the issue, there are several ways we plan to improve trailing Slash, and the trailing Slash support added to NuxtLink appears to be one of them.

nuxt/nuxt#15462 (comment)

As far as I can understand from reading the issue, Nuxt's policy is to support trailing slashes only at the minimum, and other cases will be handled in userland.

So I think it makes sense to provide a tranilingSlash option in nuxt i18n.

/cc @manniL @danielroe
if you have anything to additional comment, please comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I can understand from reading the issue, Nuxt's policy is to support trailing slashes only at the minimum, and other cases will be handled in userland.

That's correct. We leave that up to the user (either via defineNuxtLink + options, or via setting defaults for the NuxtLink in the nuxt.config.ts) ☺️

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder what trailingSlash would necessarily do in the module (compared to setting it via above mentioned options)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wonder what trailingSlash would necessarily do in the module (compared to setting it via above mentioned options)

It influences head tags, the generated routes and the way routes are matched and resolved if I understand correctly.

I will be looking into the use of this option, where it is being used and why, and if its usage can be reduced so it won't conflict with the flexible nature of trailingSlash prop of NuxtLink (if it does). We will probably support setting this option in the future via https://github.com/harlan-zw/nuxt-site-config as well (related #2474).

Let's not get distracted, this PR only (barely) documents an old option, motivation behind it is because someone on Discord asked how to enable trailing slashes in this module and enabling this option satisfied their use case.

Just trying to prevent issues as they pour in 😅..


## `rootRedirect`

- type: `string` or `object` or `null`
Expand Down