modul8 icon indicating copy to clipboard operation
modul8 copied to clipboard

Using `path` to fully support windows

Open Raynos opened this issue 13 years ago • 5 comments

Code like ref

reqStr[0...2] is './' or reqStr[0...3] is '../'

will break in windows because of / vs \\.

One should favor using methods from path like path.relative

Raynos avatar Dec 08 '11 17:12 Raynos

Ah, yes, I remember thinking about that ages ago, then forgot. Thanks, will have a look at that again this weekend.

clux avatar Dec 09 '11 11:12 clux

Several path functions are very useful and have allowed me to cut down much of my own resolving code!

However, by introducing windows resolvability on the server, they will still fail on the client. There are 2 (good) ways in which this can be solved that I see:

  • Regex replace all require strings on compilation with path.relative(domainPath, fullFilePath) by passing down resolver results to the bundler in its sorted list (can avoid adding something to require.js, however, debuggers will have to use current UNIX file string format when using with M8.require())
  • Fork path.js from node and try to make it work on the client (bloating app file, but could expose path functionality)

Currently undecided on what is the best way to proceed.

clux avatar Dec 12 '11 10:12 clux

path has already been forked for the client by SubStack

Raynos avatar Dec 12 '11 11:12 Raynos

substack's fork is posix only. I might take the win32 bits from node and include it/posix version conditionally. I'm not sure whether posix and windows slashes can coexist in the same codebase yet though. Surely they must, as npm modules are mostly posix, but if windows code use other slashes, and path determines slashtype by checking process.platform === 'win32', I am curious how anything will work at windows at all.

Anyway, been a bit busy over xmas, but things are making progress again.

clux avatar Jan 05 '12 10:01 clux

Using substack's posix fork now. Can write a w32 version and include it conditionally. But due to above reasons, not sure how to proceed yet, so I'm putting it on hold for 0.15.0

clux avatar Jan 07 '12 18:01 clux