fix: isFilePath not working on windows

[zacharygolba]

Closes #3

Uses path.sep to generate a RegExp that works for both Windows and UNIX based systems.

Windows: /^\.?\\/
Linux/OSX: /^\.?\//

[cdunford]

This looks like a good change. It's not entirely clear what the expectation of isFilePath is - perhaps using path.parse and processing the output would be easier to understand.

[cdunford]

My issue seems to stem from the fact that I have absolute paths on windows. I added the following outside of if(isFilePath(updatedId)) and it resolves my issue:

 var match = resolve.map(function (ext) {
    return updatedId + ext;
  }).find(exists);

  if (match) {
    return match;
  }

[coveralls]

Coverage remained the same at 97.727% when pulling 6f1f484 on zacharygolba:fix-windows-file-paths into 7e33d7c on frostney:master.

[gaearon]

@Rich-Harris Could you take a look? Seems like without a fix for this (I’m not sure if it’s the right fix) we can’t build React on Windows: facebook/react#9350 (comment).

[Rich-Harris]

So, I created a dummy PR (#16) to try to coax Appveyor into testing this branch, and some tests are failing — possibly unrelated to #3. I haven't yet had a chance to investigate, will try and take a look soon. Appveyor output is here if anyone is interested.

[zacharygolba]

So, I created a dummy PR (#16) to try to coax Appveyor into testing this branch, and some tests are failing — possibly unrelated to #3.

Nice! Is this a matter of tests needing an update or is there an issue with the implementation?

After a quick glance at the Appveyor output, it almost seems like this plugin assumes the existence of rollup-plugin-node-resolve.

Here is a link to an example of the resulting regex on windows. I added the g and m flags for demonstration purposes.

Let me know if there is anything I can do to help.

[Rich-Harris]

Nice! Is this a matter of tests needing an update or is there an issue with the implementation?

I assume it's an issue(s) with the implementation. Not entirely sure, though — if anyone can check out the gh-3 branch (includes this branch, updated to latest master) on a Windows machine and see if they can get their head round it I'd be very grateful.

[zacharygolba]

@Rich-Harris I got this working on macOS 10.12.4 as well as Windows 10 (running in a vm).

I changed the implementation to first convert windows paths into the posix equivalent and then further parsing/updates to the ids use the path.posix module to get the posix behavior regardless of the platform. This may allow for better compatibility with other plugins down stream.

If this looks promising, I can do some cleanup on the implementation and tests.

[zacharygolba]

Here is a diff against the gh-3 branch for better context.

[Rich-Harris]

@zacharygolba thank you! looks like green lights all round. I'll cut a new release now so that we can finally close #3, I don't think there's any great need for further cleanup but totally up to you

🍻

[zacharygolba]

@Rich-Harris Anytime! I'm happy to help out. 😃