fix: file path normalization wasn't working right (#238)

* remove normalization from analyze function Signed-off-by: Chang Wang <garnwraly@gmail.com> * perform normalization in getLocalFileDetails Signed-off-by: Chang Wang <garnwraly@gmail.com> * add test Signed-off-by: Chang Wang <garnwraly@gmail.com>
bundlewatch · Sep 22, 2020 · f856167 · f856167
1 parent 08a84bb
commit f856167
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 57 deletions.
diff --git a/__testdata__/test.bundle.x1y2z3h4sh.js b/__testdata__/test.bundle.x1y2z3h4sh.js
@@ -0,0 +1,2 @@
+// this file exists to test filepath normalization (hashes in paths)
+// https://github.com/bundlewatch/bundlewatch/issues/30
diff --git a/src/app/analyze/analyze.test.js b/src/app/analyze/analyze.test.js
@@ -1,8 +1,4 @@
-import {
-    getOverallDifference,
-    getPercentageChangeString,
-    normalizeFilename,
-} from '.'
+import { getOverallDifference, getPercentageChangeString } from '.'
 import { mockFileResults } from './analyze.test.mockdata'
 
 describe('getOverallDifference', () => {
@@ -78,31 +74,3 @@ describe('getPercentageChangeString', () => {
         expect(getPercentageChangeString(1.2345)).toEqual('+1.2%')
     })
 })
-
-describe('normalizeFilename', () => {
-    const toResult = (name) => ({ filePath: name })
-
-    // anything(.hash).js
-    const removeHashRegex = /^.+?(\.\w+)\.js$/
-    // anything(test)anything(test).js
-    const testRegex = /^.+?(test).+?(test)\.js$/
-
-    // prettier-ignore
-    const cases = [
-        // filename,                regex,              result
-        ['file.js',                 removeHashRegex,    'file.js'],
-        ['something.js',            removeHashRegex,    'something.js'],
-        ['wow.js',                  removeHashRegex,    'wow.js'],
-        ['main.6c70c5d5.js',        removeHashRegex,    'main.js'],
-        ['debugger.7bad0121.js',    removeHashRegex,    'debugger.js'],
-        ['finished.74119b14.js',    removeHashRegex,    'finished.js'],
-        ['file.js',                 testRegex,          'file.js'],
-        ['lalatestlalatest.js',     testRegex,          'lalalala.js'],
-    ]
-
-    it.each(cases)('%s + %p -> %s', (filename, regex, result) => {
-        expect(normalizeFilename(regex)(toResult(filename)).filePath).toBe(
-            result,
-        )
-    })
-})
diff --git a/src/app/analyze/index.js b/src/app/analyze/index.js
@@ -1,5 +1,4 @@
 import bytes from 'bytes'
-import { basename } from 'path'
 import analyzeFiles, { STATUSES } from './analyzeFiles'
 
 const getOverallStatus = (fileResults) => {
@@ -83,37 +82,17 @@ const getSummary = ({ overallStatus, fullResults, baseBranchName }) => {
     return `Everything is in check ${differenceSummary}`
 }
 
-export const normalizeFilename = (normalizeFilenames) => (result) => {
-    let filename = basename(result.filePath)
-    const [, ...matches] = filename.match(normalizeFilenames) ?? []
-
-    let normalized = filename
-    matches.forEach((match) => {
-        normalized = normalized.replace(match, '')
-    })
-
-    return {
-        ...result,
-        filePath: result.filePath.slice(0, -filename.length) + normalized,
-    }
-}
-
 const analyze = ({
     currentBranchFileDetails,
     baseBranchFileDetails,
     baseBranchName,
-    normalizeFilenames,
 }) => {
     let fileResults = analyzeFiles({
         currentBranchFileDetails,
         baseBranchFileDetails,
         baseBranchName,
     })
 
-    if (normalizeFilenames != null) {
-        fileResults = fileResults.map(normalizeFilename(normalizeFilenames))
-    }
-
     const overallStatus = getOverallStatus(fileResults)
     const summary = getSummary({
         overallStatus,

diff --git a/src/app/getLocalFileDetails/index.js b/src/app/getLocalFileDetails/index.js
@@ -3,7 +3,11 @@ import glob from 'glob'
 import getSize from './getSize'
 import logger from '../../logger'
 
-const getLocalFileDetails = ({ files, defaultCompression }) => {
+const getLocalFileDetails = ({
+    files,
+    defaultCompression,
+    normalizeFilenames,
+}) => {
     const fileDetails = {}
 
     files.forEach((file) => {
@@ -22,9 +26,24 @@ const getLocalFileDetails = ({ files, defaultCompression }) => {
                     filePath,
                     compression,
                 })
+                const normalizedFilePath = normalizeFilenames
+                    ? // remove matched capture groups
+                      filePath
+                          // find all matching segments
+                          .split(normalizeFilenames)
+                          .reduce(
+                              (partiallyNormalizedPath, matchingSegment) =>
+                                  // remove matching segment from normalized path
+                                  partiallyNormalizedPath.replace(
+                                      matchingSegment,
+                                      '',
+                                  ),
+                              filePath,
+                          )
+                    : filePath
 
                 if (size) {
-                    fileDetails[filePath] = {
+                    fileDetails[normalizedFilePath] = {
                         maxSize,
                         size,
                         compression,

diff --git a/src/app/index.js b/src/app/index.js
@@ -16,6 +16,7 @@ const main = async ({
     const currentBranchFileDetails = getLocalFileDetails({
         files,
         defaultCompression: defaultCompression,
+        normalizeFilenames,
     })
 
     const bundlewatchService = new BundleWatchService({
@@ -38,7 +39,6 @@ const main = async ({
         currentBranchFileDetails,
         baseBranchFileDetails,
         baseBranchName: ci.repoBranchBase,
-        normalizeFilenames,
     })
 
     const url = await createURLToResultPage({

diff --git a/src/app/index.test.js b/src/app/index.test.js
@@ -102,4 +102,22 @@ describe(`bundlewatch Node API`, () => {
         }
         expect(error).toMatchSnapshot()
     })
+
+    it('Normalizes hash when given a normalizeFilenames option', async () => {
+        const result = await bundlewatchApi({
+            files: [
+                {
+                    path: './__testdata__/*.js',
+                    maxSize: '100kB',
+                },
+            ],
+            defaultCompression: 'none',
+            normalizeFilenames: '^.+?(\\.\\w+)\\.(?:js|css)$',
+        })
+
+        delete result.url
+        expect(result.fullResults[0].filePath).toMatchInlineSnapshot(
+            `"./__testdata__/test.bundle.js"`,
+        )
+    })
 })