docusaurus icon indicating copy to clipboard operation
docusaurus copied to clipboard

Allow configuration of `cleanUrls` option for `serve` command

Open SlicedSilver opened this issue 3 years ago • 16 comments

Have you read the Contributing Guidelines on issues?

Description

Allow configuration of the cleanUrls option passed to serve-handler package when running docusaurus serve.

Currently, Docusaurus sets the cleanUrls option for the serve-hander package to true and it isn't configurable by the user. https://github.com/facebook/docusaurus/blob/bd9f29a4698963d3b139f7419877a300640b09ff/packages/docusaurus/src/commands/serve.ts#L76-L82

Proposing that this should be a user-configurable option.

Has this been requested on Canny?

No response

Motivation

By default, at least a few major hosting providers (GH Pages, Netlify, Vercel) don't enable this 'feature'. Having this as an option would allow for more accurate testing of the documentation website before deploying.

A Use Case: It is possible to embed an iframe into a documentation page which serves as a self-contained example. The example file can be loaded using the file-loader web pack loader.

<iframe
     src={require('!!file-loader!./assets/example.html').default}
 ></iframe>
 <a href={require('!!file-loader!./assets/example.html').default} target="\_blank">
     View example in a new window
 </a>

This works correctly when using docusaurus start and when deploying the build onto a hosting provider. However, when running docusaurus serve, any url ending with .html is rewritten without the extension and a 404 error occurs.

API design

ServeCLIOptions could include cleanUrls?: boolean; which defaults to true. https://github.com/facebook/docusaurus/blob/bd9f29a4698963d3b139f7419877a300640b09ff/packages/docusaurus/src/commands/serve.ts#L20-L25

When the user wishes to disable cleanUrls then they would call the serve command as follows: docusaurus serve --no-clean-urls

A similar options could be added to the configuration file (https://github.com/facebook/docusaurus/blob/bd9f29a4698963d3b139f7419877a300640b09ff/packages/docusaurus-types/src/config.d.ts).

Have you tried building it?

I've tried the changes locally and passing cleanUrls: false to serve-handler works as expected.

Self-service

  • [X] I'd be willing to contribute this feature to Docusaurus myself.

SlicedSilver avatar Aug 23 '22 15:08 SlicedSilver

Maybe we could let you pass a JSON string through a single command line option, which we directly merge into the default options? I don't have good API designs so far.

Josh-Cena avatar Aug 23 '22 15:08 Josh-Cena

If the goal is to make the entire serve-handler options object customisable from the command line then it may be a bit cumbersome to achieve that with a JSON string.

For example: docusaurus serve --serve-options='{"cleanUrls":false, "renderSingle": false, "directoryListing": ["/assets/**", "/!assets/private"]}'

Especially if trying to customise the headers option (https://github.com/vercel/serve-handler#headers-array) which can be rather lengthy.

Alternatives

  • Might be better to allow the loading of a JSON file from disk, much like the config cli option for the other commands: docusaurus serve --serve-config ./serve-config.json

  • or include these options within the main docusaurus.config.js file.

I'm happy to work on a PR if there is a decision on the preferred approach for the API.

SlicedSilver avatar Aug 23 '22 20:08 SlicedSilver

Yeah, I think a serveOptions in the config file makes more sense, just like we have several options for deploy as well. I'm not totally decided yet.

Josh-Cena avatar Aug 23 '22 20:08 Josh-Cena

To me this CLI command is just a convenience for non-technical users to easily test their Docusaurus site. If you don't like it or need something more advanced you can provide your own server.

What prevents you from running npx serve and have a serve.json config file for example? Docusaurus doesn't do much more than that.

Not against adding a --no-clean-urls options or something, but anything more advanced feels out of the scope of our CLI. I don't really want to provide a json string as a CLI arg respecting a third-party lib schema 🤷‍♂️

slorber avatar Aug 24 '22 09:08 slorber

Agreed, that makes sense. It would be out of scope to add all those options from a third-party lib.

I can use npx serve or create my own script to achieve what I'm looking for.

Would it make sense to change the default from true to false which will match the behaviour of the start command and the majority of hosting providers which I've checked?

Alternatively, could the script check for an environmental variable CLEAN_URLS=false ? No need to change the API then.

Happy that this issue can be closed if deemed out of scope.👍

SlicedSilver avatar Aug 24 '22 10:08 SlicedSilver

Would it make sense to change the default from true to false which will match the behaviour of the start command and the majority of hosting providers which I've checked?

As far as I know, Netlify has pretty URLs by default, although it tells you it doesn't (asset optimization disabled by default, but still pretty URLs enabled)

I've documented this here:

  • https://github.com/slorber/trailing-slash-guide
  • https://github.com/slorber/trailing-slash-guide/blob/main/docs/Hosting-Providers.md#netlify

This is confusing but it's what I see in practice. Also Netlify pretty URLs is not exactly the same as Vercel cleanUrls. All hosting providers have differences. Netlify pretty URLs will remove a trailing slash but won't redirect if you use HTML file extensions for example.

Here's a Netlify deployment with std settings: https://630608910ce7dd006e877866--docusaurus-with-netlify-defaults.netlify.app

Also, have you tried deploying a Docusaurus site without pretty URLs? Would our frontend SPA router work with URLs ending with xyz/index.html or xyz.html for example?

If you want to change this setting, I'd rather ask you to do the homework and show me a Docusaurus site hosted on multiple platforms with cleanUrls: false and trailingSlash true/false combinations.


Alternatively, could the script check for an environmental variable CLEAN_URLS=false ? No need to change the API then.

That's not particularly better to me. If this is a useful option we should expose it properly. But is it a useful option in the first place?


I'm still not sure to understand your iframe use-case. What is the easy actionable step-by-step guide for reproducing this problem?


Happy that this issue can be closed if deemed out of scope.👍

Feel free to close if you don't care much about getting this solved.

slorber avatar Aug 24 '22 11:08 slorber

I don't think it's quite out of scope. If some hosting providers have this behavior and others don't, we ought to provide an option to make the local testing env mirror the deployment env.

Josh-Cena avatar Aug 24 '22 12:08 Josh-Cena

I don't think it's quite out of scope. If some hosting providers have this behavior and others don't, we ought to provide an option to make the local testing env mirror the deployment env.

I'm ok to add an option for this

But if some hosting provider decides to do weird fancy things, I wouldn't make it part of docusaurus serve scope. For example GitHub Pages without a .nojekyll file would have a quite surprising static serving behavior, and we definitively don't want to add support for that

slorber avatar Aug 24 '22 12:08 slorber

Aye, yes, that makes sense

Josh-Cena avatar Aug 24 '22 12:08 Josh-Cena

I've created a stackbiltz example where the site functions correctly on npm run start, but not when running npm run serve.

https://stackblitz.com/edit/github-sifwoz?file=docusaurus.config.js

It appears that this behaviour only occurs when baseUrl is changed from the default of '/'.


The example:

  • Click 'View Example' on the landing page of the documentation site.
  • When run with start then you will see a light blue iframe
  • When run with serve then you will see the landing page within the iframe.

SlicedSilver avatar Aug 24 '22 13:08 SlicedSilver

can I try doing this?

rashidmya avatar Aug 24 '22 19:08 rashidmya

@SlicedSilver is it possible that it's related to our serve.ts code having some "extras"?

CleanShot 2022-08-26 at 17 46 16@2x


@rashidmya yes I'm ok to add a --no-clean-urls option. But let's wait first to be sure that it's needed, cf my comment above. You can help investigate the problem

slorber avatar Aug 26 '22 15:08 slorber

Within serve.ts, if I set cleanUrls: false then it works as expected.

However, if I remove the http 302 'found' redirect logic then it works as well (without changing the cleanUrls):

    // if (!req.url?.startsWith(baseUrl)) {
    //   res.writeHead(302, {
    //    Location: baseUrl,
    //  });
    //  res.end();
    //  return;
    // }

Changing the Location value within the redirect also works (without changing the cleanUrls):

Location: baseUrl + req.url.slice(1),

If changing the location value is the best option then obviously it might make sense to check if baseUrl ends with / and url starts with / if that is quietly allowed within the configuration.

const slicePoint = (baseUrl.endsWith('/') && req.url.startsWith('/')) ? 1 : 0;
// ...
Location: baseUrl + req.url.slice(slicePoint),

@slorber Hope this helps, and thanks for your assistance.

SlicedSilver avatar Aug 26 '22 19:08 SlicedSilver

This tripped us up too.

We have some .html files inside /static/examples/, and we put them in an <iframe> inside our docs:

<iframe src={useBaseUrl("/examples/demo.html")}></iframe>

This works fine with docusaurus start, but breaks with docusaurus build && docusaurus serve. We found that changing baseUrl from /docs/ to / fixes the issue, but unfortunately we can't do that for this website.

Reproduction case: https://github.com/MattiasBuelens/docusaurus-serve-html-asset-issue

What prevents you from running npx serve and have a serve.json config file for example? Docusaurus doesn't do much more than that.

That seems like a reasonable workaround for us. I'd prefer if docusaurus serve would have better support for baseUrl, but for now we'll use a custom serve script instead.

MattiasBuelens avatar Feb 19 '24 13:02 MattiasBuelens