Improve `normalizePath` performance

[scolladon]

Improve `normalizePath` performance

[mojavelinux]

I'm prepared to move forward with that idea. I don't think we should be trying to workaround bugs in old versions of external software. If that were to happen again, it needs to be fixed in that project, not this one.

[jcubic]

It's a good idea, but note that if you do not create a PR probably no one will.
There are also a lot of unit tests so if they pass probably the function is ok.
Sorry, I'm not an expert to see if the function is current or not by looking at the code. I also don't know what the old code was doing.

[scolladon]

Thanks for your advice @jcubic
I just created a PR with the performance improvement.
I hope it could be integrated and release soon

[isomorphic-git-bot]

🎉 This issue has been resolved in version 1.25.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[mojavelinux]

I would much prefer if we used path.posix.normalize when running in Node.js rather than any sort of custom implementation. I don't like the use of a global memorization map. That's a poor practice in Node.js. And this function is already accelerated in Node.js. In the browser, it's not as much of an issue since the script is running per page load (thus naturally scoped).

There are two slight differences when using path.posix.normalize, though I think those could be added as a check in a wrapper. I'd be fine with that.

If we want to proceed, I'd be willing to figure what needs to be done to make use of the function from Node.js when running in that environment.

[scolladon]

Hi @mojavelinux, I see your point, I was able to make the test test-utils-join.js path with this implementation:

const path = require('path').posix || require('path')

export function normalizePath(str) {
  let n = path.normalize(str)

  if (n === './') {
    n = '.'
  }

  return n
}

Performance wise it was roughly the same as the implementation proposed (the actual one merged recently, not the previous one)

I looked a bit more deeply how this method is used and why it exist and it seems it is only used by the join.js file.
And when looking at the join.js file it seems it has been created because path.posix.join seemed to be undefined in webpack at that time.

Maybe we could get rid of both join.js and normalizePath.js if the path.posix.join method works with current webpack.
I have not tested that approach yet but I think it will go the way you are suggesting @mojavelinux.