add windows test

[ahaoboy]

Description of changes

Added Windows platform test

Checklist

• Created unit tests in tests/unit and/or in Rust for my feature if needed
• Ran make fix to format JS and apply Clippy auto fixes
• Made sure my code didn't add any additional warnings: make check
• Added relevant type info in types/ directory
• Updated documentation if needed (API.md/README.md/Other)

[ahaoboy]

https://github.com/awslabs/llrt/actions/runs/10792507462/job/29932350859

Paths between platforms are a pain to deal with, especially when using them obfuscated, and at the moment there are some differences between llrt's behavior and node's, which will probably be fixed in a future update.

[Sytten]

I am a bit wary of doing manual path handling, I am wondering if we should transform all the string in Path and use the std or a library. It very complex to handle all the edge cases, especially windows that has a bunch of weird paths. I use dunce for normalization.

[ahaoboy]

I am a bit wary of doing manual path handling, I am wondering if we should transform all the string in Path and use the std or a library. It very complex to handle all the edge cases, especially windows that has a bunch of weird paths. I use dunce for normalization.

Mixing different styles and platform differences is very troublesome. However, there is no such thing as node_resolve or node_path. Deno uses js to implement path logic and also distinguishes win and unix. We only need to ensure that common scenarios can be consistent with node. In most cases, llrt runs the bundled code.

const path = require('path')

console.log(
  path.isAbsolute("C:/"),
  path.isAbsolute("/a"),
)

/*
win: true true
unix: false ture
 */

[richarddavison]

@ahaoboy thanks for the PR. Doesn't windows also support forward slash paths these days? Can we consolidate the path module by supporting forward slashes only?

[ahaoboy]

@ahaoboy thanks for the PR. Doesn't windows also support forward slash paths these days? Can we consolidate the path module by supporting forward slashes only?

There are three path separators that can be used on Windows: / , \ // :D

In most cases, using / is fine, and in import statements, only / can be used, but considering that we cannot get rid of the influence of node and npm, and the behavior of many third-party libraries may depend on this inconsistency, I think the best way is to be consistent with node. Adding win path support is just for developers(like me) using windows to test and develop.

[richarddavison]

I think that a lot of code from the unix and windows path module could be consolidated. From the looks of it it's mostly the strip_suffic or rfind closure that's different.
One option would be to define this closure as a function instead and use conditional compilation on windows/unix.
There is also some optimizations that can be applied cross the entire path module such as avoiding extra allocations. For example, in join_path we can do this instead:

pub fn join_path<'a, I>(parts: I) -> String
where
    I: Iterator<Item = &'a str>,
{
    let mut path_buf = parts.fold(PathBuf::new(), |mut acc, part| {
        if !part.is_empty() {
            for part in part.split("/") {
                if !part.is_empty() {
                    if part == ".." {
                        acc.pop();
                    } else if part != "." {
                        acc.push(part);
                    }
                }
            }
        }
        acc
    });
    

    path_buf.to_string_lossy().into_owned()
}

and similarly in resolve_path working with string slice iter instead of strings.

Stick to a shared function between linux and windows as much as possible even within functions to reduce maintenance burden and reduce error risks. Only if the same function has completely different implementations use a dedicated per OS.

[ahaoboy]

Stick to a shared function between linux and windows as much as possible even within functions to reduce maintenance burden and reduce error risks. Only if the same function has completely different implementations use a dedicated per OS.

Using a platform-dependent SEP_PAT variable for path segmentation seems to work well.

[richarddavison]

Thank you! Almost there! Just added a few optimization suggestions. I know you've based your implementation on our previous code but we've find more optimization by reviewing this code again :)