fix(shouldIgnore): filename normalization should be platform sensitive

[rozele]

Q	A
Bug fix?	yes
Breaking change?	no
New feature?	no
Deprecations?	no
Spec compliancy?	yes
Tests added/pass?	yes
Fixed tickets
License	MIT
Doc PR

shouldIgnore normalizes the path of the filename prior to running any "only" regexes or functions. The normalization uses "slash", which has some undesirable special cases for non-ASCII characters and longer paths. Changing the normalization behavior to always replace path separators.

There is no real need to add additional tests, because the tests are not run in an environment where path.sep !== '/' anyway.

[codecov-io]

Current coverage is 88.78% (diff: 100%)

Merging #4631 into master will increase coverage by 0.03%

@@             master      #4631   diff @@
==========================================
  Files           195        195          
  Lines         13773      13822    +49   
  Methods        1425       1434     +9   
  Messages          0          0          
  Branches       3171       3196    +25   
==========================================
+ Hits          12223      12272    +49   
  Misses         1550       1550          
  Partials          0          0          

Powered by Codecov. Last update 0e02a18...3a6c8f6

[rozele]

Any feedback on this?

[hzoo]

Don't know much about what it's doing there - would there be other places we want to do this?

Also this would be removing filename = slash(filename); in the default case?

[rozele]

@hzoo there may be other cases. In this specific case it has an impact on the babel register behavior. If I register babel to transform paths matching /foo\/bar/ then, I presume the use of slash is to ensure that the regex also matches foo\bar for Windows. Unfortunately, I don't know enough about the expected behavior of babel in general, and there are a few other uses I didn't investigate. All I know is that there are a number of issues on StackOverflow and a few GitHub issues on facebook/react-native (e.g., facebook/react-native#10033 and facebook/react-native#8899) that are sourced from this here.

[danez]

This looks overall valid to me, as we do not care about long paths and non-ascii characters in the filename, as everything we want to do here is simply doing string comparison with the strings and not use any path functionality.

[danez]

We probably can safely remove this check, as slash() is also always doing the replacement, not matter what path.sep is set to.
https://github.com/sindresorhus/slash/blob/master/index.js

[rozele]

Thanks @danez, removed the conditional as you suggested.