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

fs-routes: Fixes backslash issue on Windows #877

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

renkei
Copy link

@renkei renkei commented Jun 2, 2023

Fixes #868 and #871. It fixes the issue at an early stage of the scan-path-for-routes-procedure. Because fs-routes is responsible for resolving routes it should take care that routes are well-formed. This ensures that consuming packages don't need to care about this.

@renkei
Copy link
Author

renkei commented Jun 5, 2023

I've found out that downgrading glob from version 10 to 7 also fixes the issue. Currently, package express-openapi comes with a "glob": "*" dependency, changing this into "glob": "7" would also solve the issue. Anyway, I still prefer this PR to be independent of the format returned by glob.

renkei added 3 commits June 8, 2023 12:01
glob >= 10.2.7
set glob version to 10.2.7
Improved backslash fix on Windows platforms based on glob option "posix: true"
@renkei
Copy link
Author

renkei commented Jun 8, 2023

I've updated my PR because I've probably found a more elegant solution. Instead of calling replace(/\\/, '/') to get rid of backslashes for routes on Windows, we can use the glob option posix: true that was introduced with glob 10.1.0, this I've found in the glob changelog

Add posix: true option to return / delimited paths, even on Windows.

I've also updated package.json and package-lock.json to request the latest glob version >= 10.2.7

renkei added 3 commits June 8, 2023 12:46
use pulbic npmjs registry
peer: true restored
@alex77h
Copy link

alex77h commented Dec 5, 2023

would love to see this merged. hope this project isnt abandoned - long time no update...

@anthonyloukinas
Copy link

Would also love this merged. I tested this on Windows and it fixes the issues I was having using {id}.js file formatting (Using the curly braces).

.sync(options.glob, { cwd: dir, posix: true })

Setting that posix: true works.

@mystiker
Copy link

mystiker commented Oct 4, 2024

Please merge this!

@mystiker
Copy link

mystiker commented Oct 4, 2024

I've found out that downgrading glob from version 10 to 7 also fixes the issue. Currently, package express-openapi comes with a "glob": "*" dependency, changing this into "glob": "7" would also solve the issue. Anyway, I still prefer this PR to be independent of the format returned by glob.

Can you please elaborate how you done this? I tried to set the following in my package.json in a project referencing express-openapi:

  "overrides": {
    "express-openapi": {
      "glob": "7"
    }
  },

But to no avail. Might be because I am using a somewhat complex monorepo with npm modules and maybe the resolution does not work as I expect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SyntaxError: Invalid regular expression during path-to-regexp conversion
4 participants