fastify-static icon indicating copy to clipboard operation
fastify-static copied to clipboard

Documentation on options root and list is slightly misleading. Clarification on the term multi-root needed.

Open LaKing opened this issue 3 years ago • 1 comments

Prerequisites

  • [X] I have written a descriptive issue title
  • [X] I have searched existing issues to ensure the issue has not already been raised

Issue

The readme for options - root states "You can also provide an array of directories ..."

.. however, using an array in the options root arguments renders the server unresponsive without any error messages. Connections simply get refused.

root: "/my/static/path", list: true - is ok. root: [ "/my/static/path" ], list: true - is breaking fastify.

This is ok, as long as the documentation is clear about this. This seems to apply only if list is true. Not sure what multi-root means, in the options list context, ... does using an array in the root argument create a multi-root instance?

LaKing avatar Jul 09 '22 21:07 LaKing

I think you have spotted a bug. I think the list command is not really aware of having more than one root. Would you like to send a Pull Request to address this issue? Remember to add unit tests.

mcollina avatar Jul 11 '22 08:07 mcollina

I don't get this, it works perfectly fine for me when I pass the following:

await fastify.register(import("@fastify/static"), {
  root: [new URL("src/assets/", import.meta.url), new URL("src/models/", import.meta.url)],
  prefix: "/p/assets",
  maxAge: process.env.NODE_ENV === "production" ? "10m" : "0m",
});

Is it because the paths listed in the issue description are relative? If so, the README states that they must be absolute.

gurgunday avatar Jul 16 '23 11:07 gurgunday

The issue is very specific to the implementation of list. Basically this option is incompatible with having multiple roots at this point in time.

We can either fix that or add a note on the docs that they are incompabile (and throw a noisy exception).

mcollina avatar Jul 17 '23 09:07 mcollina

The commit https://github.com/fastify/fastify-static/commit/c70c25dfb6495b8ed4684d04b32b23326c69ba7a has actually fixed this issue by throwing if list is used with an array of roots

Also, the README was updated as well:

Note: Multi-root is not supported within the list option.

gurgunday avatar Jul 27 '23 09:07 gurgunday

@Eomm the issue can be closed 😁🧹

gurgunday avatar Jul 28 '23 05:07 gurgunday