actionhero / actionhero

Actionhero is a realtime multi-transport nodejs API Server with integrated cluster capabilities and delayed tasks
https://www.actionherojs.com
Apache License 2.0
2.4k stars 259 forks source link

[Bug]: actionhero generate does nothing on windows #2275

Closed YamCrack closed 2 years ago

YamCrack commented 2 years ago

Is there an existing issue for this?

Current Behavior

when i run command

npx actionhero generate

nothing happens.

WindowsTerminal_rb21nChhHC

Expected Behavior

New project created

Stack Trace

No response

Environment

OS: Windwos 11, Window 10 Actionhero Verision: 28.1.11 Nodejs: 14.20.0

Additional context

No response

YamCrack commented 2 years ago

the command works fine on actionhero 28.1.9

evantahler commented 2 years ago

Hi!

Can you confirm that the directory you tried actionhero generate in is empty? Also, run npx actionhero --version to confirm which version you have locally.

I have no access to a windows machine to do debug this... so let me know if you are able to work on this issue yourself!

YamCrack commented 2 years ago

yes i have tried with empty directory and after npm i -s actionhero, none of them works, i can try to fix it.

YamCrack commented 2 years ago

I found the problem is the update of glob from 7.20 to 8.0.1

Since glob 7.2.1 backslash “\” is not supported and it only uses slash “/”, everytime something like this is used:

glob.sync(path.join(configPath, “**“, “**/*(*.js|*.ts)“))

path.join adds a backslash. Therefore, the path is no longer valid. If you agree, we can change it to:

glob.sync(path.join(configPath, “**“, “**/*(*.js|*.ts)“), { windowsPathsNoEscape: true })

or join the paths manually.

Please let me know your thoughts.

evantahler commented 2 years ago

That's a shame that windowsPathsNoEscape isn't automatically enabled if a windows environment is detected. Ideally, that would be a feature added to glob itself. If that isn't possible, I guess we could wrap it in a utility...

Can you try working with the glob maintainers first to create a "windows compat" mode?

YamCrack commented 2 years ago

There are issues related to this problem and they are refusing to change it. They recommend doing something like this:

path.join(__dirname, '*.txt').split(path.sep).join("/")

sorry for my bad english

evantahler commented 2 years ago

Can you link the conversation in the Glob repo please? I wonder if they make good points that we should follow as well.

YamCrack commented 2 years ago

sure, there is:

https://github.com/isaacs/node-glob/issues/480 https://github.com/isaacs/node-glob/issues/468 https://github.com/isaacs/node-glob/issues/419

-- we can write a utility function that check por path.sep, and change the \ to /. like this: image

evantahler commented 2 years ago

This conversation does a good job of explaining why replacing the slashes probably isn't a good idea - https://github.com/isaacs/node-glob/issues/468

I think the path forward is to replace all uses in the project of glob with safeGlob() which internally attempts to detect which OS the project is running on. If windows is detected, then we enable windowsPathsNoEscape as we call into glob.


function safeGlobSync(match: string, ...args) { 
  const isWindows: process.platform === "win32" || process.platform === "win64";
  return glob.sync(match, {...args, windowsPathsNoEscape: isWindows})
} 
YamCrack commented 2 years ago

i did a PR, let me know your thoughts

https://github.com/actionhero/actionhero/pull/2278/commits/398241e44f55e9004745d254dec90d3ff4a7daed

YamCrack commented 2 years ago

Ia have been pull new changes: