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

Inconsistent File Format Handling Between Windows and POSIX in fast-glob #425

Closed
saoudi-h opened this issue Sep 29, 2023 · 7 comments · Fixed by #427
Closed

Inconsistent File Format Handling Between Windows and POSIX in fast-glob #425

saoudi-h opened this issue Sep 29, 2023 · 7 comments · Fixed by #427
Milestone

Comments

@saoudi-h
Copy link

Environment

  • OS Version: windows 11
  • Node.js Version: 20.7

Actual behavior

Hello,

I'd like to report an issue that was initially raised on the Prettier repository prettier/prettier#15426, but ultimately led us to examine Fast-glob. It turns out that Prettier uses Fast-glob to resolve patterns, especially for Prettier-cli. Specifically, in my case, it's about the following command: npx prettier --write [...nextauth].ts, which seems to be a filename format accepted on all operating systems.

It appeared to me that Fast-glob uses two different patterns, one for POSIX systems and another for Windows:

See: https://github.com/mrmlnc/fast-glob/blob/28a3d61e44d5d9193ba97de4f21df6dc7725f7c0/src/utils/path.ts#L13C1-L14C94

which ends up giving two different behaviors depending on whether you are on Linux or Windows.

I don't know the full implications of this, but I know that for my example the brackets "[]" in Windows are not escaped when we call the escapePath() method

This returns on Linux

./myfolder/\[.myFile\].ts 

and on Windows.

./myfolder/[.myfile].ts 

Expected behavior

I believe that if Windows supports this naming format for these files, then Fast-glob should ensure it conforms to it.

Steps to reproduce

on windows:

  1. Create a small project:
mkdir fg-issue && cd fg-issue
npm init
npm i fast-glob
  1. Create an index.js file with the following content:
// index.js

const fg = require('fast-glob')

console.log(fg.escapePath('.[.any].any'))
  1. execute :
    pwsh or cmd:
>node index

out on windows: .[.any].any

Out on linux : .\[.any\].any

Code sample

A faster way to see the difference, although you'll have to trust me for the code I obtained from Fast-glob.
I've included only the essentials to reproduce the problem:

const path = "./any/[.any].ts";

const POSIX_UNESCAPED_GLOB_SYMBOLS_RE = /(\\?)([()*?[\]{|}]|^!|[!+@](?=\()|\\(?![!()*+?@[\]{|}]))/g;
const WINDOWS_UNESCAPED_GLOB_SYMBOLS_RE = /(\\?)([(){}]|^!|[!+@](?=\())/g;

function escapeWindowsPath(pattern) {
    return pattern.replace(WINDOWS_UNESCAPED_GLOB_SYMBOLS_RE, '\\$2');
}
function escapePosixPath(pattern) {
    return pattern.replace(POSIX_UNESCAPED_GLOB_SYMBOLS_RE, '\\$2');
}

console.log( "Posix : \n", escapePosixPath( path ) )
console.log( "Windows : \n", escapeWindowsPath( path ) )
@DamianGlowala
Copy link

This also affects my Nuxt project, where I use Prettier to format files on commit.

For instance, a file named server/api/[...].ts throws an error. This is a syntax Nuxt uses as a server-side catch-all route.

@JetBoom
Copy link

JetBoom commented Sep 30, 2023

Having this issue on an astro project

@kostia1st
Copy link

Using husky + prettier with Next.js routing patterns like pages/home/[[...slugs]]/index.tsx just got complicated. Still possible with git commit -m "message" -n though 😁

@mrmlnc mrmlnc added this to the 3.3.2 milestone Oct 29, 2023
@mrmlnc
Copy link
Owner

mrmlnc commented Oct 29, 2023

Fixed in the main branch and was backported to 3.x.x.

@saoudi-h
Copy link
Author

saoudi-h commented Oct 29, 2023

The reported issue has been fixed. I'll go ahead and close this now. Thanks to @mrmlnc for the quick resolution

@lapa182
Copy link

lapa182 commented Nov 2, 2023

Was this released on npm under 3.3.2 or 3.3.1? I can't see in NPM :(.

@mrmlnc
Copy link
Owner

mrmlnc commented Nov 6, 2023

https://github.com/mrmlnc/fast-glob/releases/tag/3.3.2

@mrmlnc mrmlnc linked a pull request Nov 23, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants