Revert "refactor: use path.join from stdlib instead of custom join (isomorphic-git#1861)"

This reverts commit bbcdda7.

Author: jayree

__tests__/test-utils-join.js

@@ -4,8 +4,7 @@ const path = require('path').posix || require('path')
 const { join } = require('isomorphic-git/internal-apis')
 
 describe('utils/join', () => {
-  describe('when "internal join" generates paths the same as "path.join"', () => {
-    // Tests adapted from path-browserify
+  it('should join "good" paths the same as path.join', async () => {
     const fixtures = [
       ['/foo/bar', 'baz'],
       ['foo/bar', 'baz'],
@@ -19,39 +18,60 @@ describe('utils/join', () => {
       ['/', '.'],
       ['/', '.git'],
       ['.', '.git'],
-      [],
-      ['foo/x', './bar'],
-      ['foo/x/', './bar'],
-      ['foo/x/', '.', 'bar'],
-      ['.', '.', '.'],
-      ['.', './', '.'],
-      ['.', '/./', '.'],
-      ['.', '/////./', '.'],
-      ['.'],
-      ['', '.'],
-      ['foo', '/bar'],
-      ['foo', ''],
-      ['foo', '', '/bar'],
-      ['/'],
-      ['/', '.'],
-      [''],
-      ['', ''],
-      ['', 'foo'],
-      ['', '', 'foo'],
-      [' /foo'],
-      [' ', 'foo'],
-      [' ', '.'],
-      [' ', ''],
-      ['/', '/foo'],
-      ['/', '//foo'],
-      ['/', '', '/foo'],
     ]
-    fixtures.forEach(fixture => {
-      it(`"${JSON.stringify(fixture)}" should join to "${path.join(
-        ...fixture
-      )}"`, () => {
-        expect(join(...fixture)).toEqual(path.join(...fixture))
-      })
-    })
+    for (const fixture of fixtures) {
+      expect(join(...fixture)).toEqual(path.join(...fixture))
+    }
+  })
+  it('should join degenerate paths the same as path.join in these cases', async () => {
+    // Tests adapted from path-browserify
+    const fixtures = [
+      [[], '.'],
+      [['foo/x', './bar'], 'foo/x/bar'],
+      [['foo/x/', './bar'], 'foo/x/bar'],
+      [['foo/x/', '.', 'bar'], 'foo/x/bar'],
+      [['.', '.', '.'], '.'],
+      [['.', './', '.'], '.'],
+      [['.', '/./', '.'], '.'],
+      [['.', '/////./', '.'], '.'],
+      [['.'], '.'],
+      [['', '.'], '.'],
+      [['foo', '/bar'], 'foo/bar'],
+      [['foo', ''], 'foo'],
+      [['foo', '', '/bar'], 'foo/bar'],
+      [['/'], '/'],
+      [['/', '.'], '/'],
+      [[''], '.'],
+      [['', ''], '.'],
+      [['', 'foo'], 'foo'],
+      [['', '', 'foo'], 'foo'],
+      [[' /foo'], ' /foo'],
+      [[' ', 'foo'], ' /foo'],
+      [[' ', '.'], ' '],
+      [[' ', ''], ' '],
+      [['/', '/foo'], '/foo'],
+      [['/', '//foo'], '/foo'],
+      [['/', '', '/foo'], '/foo'],
+    ]
+    for (const [args, result] of fixtures) {
+      expect(join(...args)).toEqual(result)
+      expect(join(...args)).toEqual(path.join(...args))
+    }
+  })
+  it('should join degenerate paths differently from path.join in these cases', async () => {
+    // Tests adapted from path-browserify
+    const disagreeFixtures = [
+      [['./'], '.'],
+      [['.', './'], '.'],
+      [['', '/foo'], 'foo'],
+      [['', '', '/foo'], 'foo'],
+      [['foo/', ''], 'foo'],
+      [['', '/', 'foo'], 'foo'],
+      [['', '/', '/foo'], 'foo'],
+    ]
+    for (const [args, result] of disagreeFixtures) {
+      expect(join(...args)).toEqual(result)
+      expect(join(...args)).not.toEqual(path.join(...args))
+    }
   })
 })

src/internal-apis.js

@@ -41,6 +41,7 @@ export * from './utils/mergeFile'
 export * from './utils/mergeTree'
 export * from './utils/modified'
 export * from './utils/padHex'
+export * from './utils/path'
 export * from './utils/pkg'
 export * from './utils/resolveTree'
 export * from './utils/shasum'

src/utils/join.js

@@ -1 +1,7 @@
-export { join } from 'path'
+// For some reason path.posix.join is undefined in webpack
+// Also, this is just much smaller
+import { normalizePath } from './normalizePath.js'
+
+export function join(...parts) {
+  return normalizePath(parts.map(normalizePath).join('/'))
+}

src/utils/normalizePath.js

@@ -0,0 +1,27 @@
+const memo = new Map()
+export function normalizePath(path) {
+  let normalizedPath = memo.get(path)
+  if (!normalizedPath) {
+    normalizedPath = normalizePathInternal(path)
+    memo.set(path, normalizedPath)
+  }
+  return normalizedPath
+}
+
+function normalizePathInternal(path) {
+  path = path
+    .split('/./')
+    .join('/') // Replace '/./' with '/'
+    .replace(/\/{2,}/g, '/') // Replace consecutive '/'
+
+  if (path === '/.') return '/' // if path === '/.' return '/'
+  if (path === './') return '.' // if path === './' return '.'
+
+  if (path.startsWith('./')) path = path.slice(2) // Remove leading './'
+  if (path.endsWith('/.')) path = path.slice(0, -2) // Remove trailing '/.'
+  if (path.length > 1 && path.endsWith('/')) path = path.slice(0, -1) // Remove trailing '/'
+
+  if (path === '') return '.' // if path === '' return '.'
+
+  return path
+}

src/utils/path.js

@@ -0,0 +1,5 @@
+// This module is necessary because Webpack doesn't ship with
+// a version of path-browserify that includes path.posix, yet path-browserify IS path.posix
+import _path from 'path'
+
+export const path = _path.posix === undefined ? _path : _path.posix