Skip to content

Wildcard glob support #178

@ghost

Description

It looks like passing a glob to wildcard was a breaking change in the 4.x release (#175), but it's unclear why it was removed. It's an extremely valuable feature and something I've used across multiple applications. Is it possible to revert that change and keep the functionality, or is there a workaround to keep similar behavior without having to rework directory structures?

For one of the use cases specifically, by using globs, it's possible to keep static assets next to the relevant code they're used with rather than having to maintain a separate assets directory. It's in a similar vein of keeping your tests next to the code versus having a separate tests directory, etc.

It'd be awesome to keep this functionality if there wasn't a strong reason to remove it.

EDIT: I did see that #172 was added, but wasn't sure if that would actually allow the same behavior or if there were differences.

Activity

mcollina

mcollina commented on Mar 5, 2021

@mcollina
Member

The problem with the wildcard: 'globPattern' option was that it disabled some features, most notably adding it would fail to detect new files.

If somebody would like to work on this problem, adding another option in to specify the glob pattern would be good: it should work with wildcard both true and false.

ghost

ghost commented on Mar 5, 2021

@ghost

The problem with the wildcard: 'globPattern' option was that it disabled some features, most notably adding it would fail to detect new files.

I thought that was a feature, not a bug :P

To clarify, I'm not saying it needs to be added back if there is a different way to do it. It appears the allowedPaths option could be a viable alternative, but I didn't know if there were drawbacks to using that versus the original wildcard option.

E.g.

- wildcard: '*.{css,png,woff2}',
+ allowedPath: (path) => /\.(?:css|png|woff2)$/.test(path),

Could you confirm if the two lines are effectively the same? In testing, it appears to work as expected, but didn't want to get hit with a gotcha later because I misunderstood something.

On a side note, allowedPaths struck me as an odd name (it's actually why I didn't notice it when I first read through everything). It only dawned on me when I noticed the PR / old linked issue of mine that talked about filtering. I can create a separate issue if you'd be open to it, but is it possible to rename allowedPaths to filter, or possibly make filter an alias?

Appreciate the help :)

mcollina

mcollina commented on Mar 8, 2021

@mcollina
Member

Calling it filter() would actually be way better! Would you mind to send a PR adding it as an alias for allowedPath?

mcollina

mcollina commented on Mar 8, 2021

@mcollina
Member

The problem with the wildcard: 'globPattern' option was that it disabled some features, most notably adding it would fail to detect new files.

I thought that was a feature, not a bug :P

Well it was but we got a few issue reports that "it failed to detect new files" and it was really confusing for a few users. So, we removed it. It seems it's easy to add it back via filter or similar if you or somebody else is willing to put the effort to work on it.

AndersCan

AndersCan commented on Dec 22, 2021

@AndersCan
Contributor

I was the one who originally added the wildcard: globPattern feature and never thought it would be so confusing 😅. I fully understand why it was removed though.

I have now upgraded to 4.x and use something like this:

  const prefix = '/public';
  const assetRoot = path.join(process.cwd(), '/assets');
  app.register(fastifyStatic, { root: assetRoot, wildcard: true, decorateReply: true });
  const assetPaths = glob.sync(path.join(assetRoot, '/**/*.{css,js}'));

  const assets = assetPaths.map((path) => {
    return {
      diskPath: path.replace(assetRoot, ''),
      routePath: path.replace(assetRoot, prefix),
    };
  });

  for (const asset of assets) {
    app.get(asset.routePath, (_request, reply) => {
      reply.sendFile(asset.diskPath, assetRoot);
    });
  }

I think this should work similar to before, but maybe one of the maintainers can confirm.

I was considering using allowedPath as mentioned above, but I was worried about the performance cost and potentially opening the server to a Regular expression Denial of Service.

yaneony

yaneony commented on Apr 18, 2023

@yaneony

Would it be okay to add something like globOverride for people who want make own behavior?

mcollina

mcollina commented on Apr 19, 2023

@mcollina
Member

Calling it globPattern would be good.

yaneony

yaneony commented on Apr 19, 2023

@yaneony

Wouldn't it be better to allow to pass additional(all) options to glob too?!

mcollina

mcollina commented on Apr 19, 2023

@mcollina
Member

That would also be ok

yaneony

yaneony commented on Apr 19, 2023

@yaneony

Need help: #375 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    good first issueGood for newcomerssemver-minorIssue or PR that should land as semver minor

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Participants

      @mcollina@AndersCan@yaneony

      Issue actions

        Wildcard glob support · Issue #178 · fastify/fastify-static