fix(metro-resolver): use proper isRelativeImport check that works with Windows paths

[byCedric]

Summary

Found an issue with the isRelativeImport from the resolver on Windows, resulting in the following issues:

• [web] Expo and Expo-router causing application to fail when opening on web expo/expo#29631
• https://www.reddit.com/r/expo/comments/1dd43gb/unable_to_resolve_module_node/

The summary of this is that Expo resolves the main field from the package during static rendering here. On Windows, it results in a path like .\\node_modules\\expo-router\\entry, which is correct. Node can resolve this path using require.resolve:

Unfortunately, the metro-resolver can't handle this properly because of a single reason: isRelativeImport returns false for .\\node_modules\\expo-router\\entry, because the regex only tests posix paths, not Windows paths. It results in the following resolve chain:

1. link - resolveModulePath is never called, as this path is not absolute, and incorrectly marked as not relative either
2. link - realModuleName is also not false, as the module path is not redirected
3. link - isDirectImport also results in false, for the same reasons as 1
4. link - context.allowHaste is disabled during Expo static rendering
5. link - Both disableHierarchicalLookup and extraNodeModules aren't set by default in Expo projects.
6. link - allDirPaths already contain <project>\\node_modules, and when joining with .\\node_modules\\expo-router\\entry, it creates paths with <project>\\node_modules\\node_modules\\..., which are also incorrect.

Overview of all values through the debugger

This change adds the Windows path, as a valid path separator, to the isRelativeImport test - making it compatible with Windows.

Changelog: [Fix] Detect relative imports in the Metro Resolver correctly on Windows

Test plan

Only reproducible on Windows.

• $ npm create expo ./test-metro-resolve
• $ cd ./test-metro-resolve
• $ npx expo export --platform web
• Failure due to the resolve issue I just mentioned

[byCedric]

I see that Node 18.12 just timed out, it didn't fail. Also, might be nice to drop Node 16 (EOL version) some time, and start testing on Node 20 (current LTS version).

[robhogan]

Interesting... afaik, import/require paths should only ever use posix separators, including on Windows, so my first instinct is that they aren't valid module specifiers and the caller should be normalising to posix. Even Node's algorithm spec defines a relative path as one beginning ./ (or == .), but from your demo I guess it's more lenient than its own spec.

I wouldn't be surprised if there are more issues than just this one though, given it's not something the resolver is designed or tested for generally. Have you tried these paths with Webpack or Vite?

[byCedric]

Node mostly uses the node:path module to resolve modules, which is built for interoperability between Windows and Linux/MacOS, e.g. there is path.sep, path.resolve and platform-specific path.win32.resolve/path.posix.resolve APIs.

I think that's the reason why it succeeds in Node, but I'm not sure if Node qualifies / as a "separator" or exact character in the spec.

• If they do qualifies it as "separator", then the assumption that it only can be / is wrong, can also be \\.
• If they only match on exact character, then we have a bigger issue here since we also C:\\some\\project as project root, right?

But right now, I'm leaning towards the first option, as it would mean that Node violates it's own module resolution spec.

[byCedric]

To summarize, Metro assumes that all module specifiers (from 'module-specifier' / require('module-specifier') are notated in posix format, even on Windows.

This does not apply to folder references passed to Metro, e.g. the project root.