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

client-redirects plugin should not be baseUrl sensitive #2982

Closed
slorber opened this issue Jun 23, 2020 · 1 comment · Fixed by #3010
Closed

client-redirects plugin should not be baseUrl sensitive #2982

slorber opened this issue Jun 23, 2020 · 1 comment · Fixed by #3010
Labels
bug An error in the Docusaurus core causing instability or issues with its execution mlh Major League Hacking Fellowship

Comments

@slorber
Copy link
Collaborator

slorber commented Jun 23, 2020

🐛 Bug Report

Discussed here:
#2969 (comment)

Have you read the Contributing Guidelines on issues?

Yes

To Reproduce

The conf is currently:

    [
      '@docusaurus/plugin-client-redirects',
      {
        fromExtensions: ['html'],
        createRedirects: function (path) {
          // redirect to /docs from /docs/introduction,
          // as introduction has been made the home doc
          if (allDocHomesPaths.includes(path)) {
            return [`${path}/introduction`];
          }
        },
      },
    ],

Expected behavior

Using a baseUrl should not require the user to reconfigure the plugin

Here we need to modify the behavior of createRedirects, becase the path argument includes the baseUrl.

For the extensions it has been fixed in #2969

Actual Behavior

createRedirects is baseUrl sensitive


What we should probably do?

The postBuild hook receives props.routePaths, but these paths includes the baseUrl, and we forward these paths to other plugin methods:

    async postBuild(props: Props) {
      const pluginContext: PluginContext = {
        routesPaths: props.routesPaths,
        ...
      };
   // ...
}

We should probably instead make these paths relative to the baseUrl so that the plugin config is not baseUrl sensitive.

    async postBuild(props: Props) {
      const pluginContext: PluginContext = {
        relativeRoutesPaths: props.routesPaths.map(path => removePrefix(path,props.baseUrl)),  
        ...     
      };
   // ...
}

(this probably requires reverting some changes from #2969 , as passing the baseUrl as arg to some methods would not be needed anymore)

@slorber slorber added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers mlh Major League Hacking Fellowship v2 and removed status: needs triage This issue has not been triaged by maintainers labels Jun 23, 2020
@teikjun
Copy link
Contributor

teikjun commented Jun 23, 2020

I observed two related issues while reproducing this issue.

  1. baseUrl field fails validation when there is no ending slash
    While building with baseUrl: /build, I receive Validation Errors from the client-redirects plugin.
Error: Some created redirects are invalid:
- {"from":"//blog.html","to":"//blog"} => Validation error: "from" is not a valid pathname. Pathname should start with / and not contain any domain or query string
// followed by many more lines of similar errors 

To resolve this, we can add a slash at the end. i.e. baseUrl: /build/ works. However, I'm not sure if this is the intended behavior.
(Edit: This is noted in issue #2978 already, PR #2987 will resolve the issue)

  1. No redirect occurs even when using default baseUrl
    Even when we use baseUrl: '/', no redirection occurs when we visit http://localhost:5000/docs/introduction (or https://v2.docusaurus.io/docs/introduction). Instead, we go to the 404 page.
    To resolve this in D2, we can add an ending slash to every element in allDocHomePaths. (e.g For the link above to work in particular, we should append a slash to /docs)
// append an ending slash to all array elements like this
const allDocHomesPaths = [
  '/docs/', 
  '/docs/next/',
  ...versions.slice(1).map((version) => `/docs/${version}/`),
];

I will make a PR to resolve the website redirection issue in point 2 together with the original issue for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution mlh Major League Hacking Fellowship
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants