Fix issue with using non-page files inside pages folders (#733)

(patch)

Author: ryardley

packages/server/src/stages/pages/find-duplicates.test.ts

@@ -2,29 +2,29 @@ import {fullTransformer, findDuplicates, filterBy} from "."
 import {normalize} from "path"
 
 test("should filter by path", () => {
-  expect(fullTransformer(normalize("app/foo/pages/api/bar"))).toBe(normalize("pages/api/bar"))
-  expect(fullTransformer(normalize("app/foo/api/bar"))).toBe(normalize("pages/api/bar"))
+  expect(fullTransformer(normalize("app/foo/pages/api/bar.ts"))).toBe(normalize("pages/api/bar.ts"))
+  expect(fullTransformer(normalize("app/foo/api/bar.ts"))).toBe(normalize("pages/api/bar.ts"))
 
   const dupes = findDuplicates(
     [
-      "app/foo/pages/api/bar",
-      "app/pages/api/bar",
-      "app/api/bar",
-      "pages/bar",
-      "app/ding/pages/bar",
+      "app/foo/pages/api/bar.ts",
+      "app/pages/api/bar.ts",
+      "app/api/bar.ts",
+      "pages/bar.ts",
+      "app/ding/pages/bar.ts",
     ].map(normalize),
     fullTransformer,
   )
 
   expect(dupes).toEqual([
-    ["app/foo/pages/api/bar", "app/pages/api/bar", "app/api/bar"].map(normalize),
-    ["pages/bar", "app/ding/pages/bar"].map(normalize),
+    ["app/foo/pages/api/bar.ts", "app/pages/api/bar.ts", "app/api/bar.ts"].map(normalize),
+    ["pages/bar.ts", "app/ding/pages/bar.ts"].map(normalize),
   ])
   expect(filterBy(dupes, "api")).toEqual([
-    ["app/foo/pages/api/bar", "app/pages/api/bar", "app/api/bar"].map(normalize),
+    ["app/foo/pages/api/bar.ts", "app/pages/api/bar.ts", "app/api/bar.ts"].map(normalize),
   ])
   expect(filterBy(dupes, "pages", "api")).toEqual([
-    ["pages/bar", "app/ding/pages/bar"].map(normalize),
+    ["pages/bar.ts", "app/ding/pages/bar.ts"].map(normalize),
   ])
 })
 

packages/server/src/stages/pages/index.ts

@@ -5,7 +5,7 @@ import {handleErrors, DuplicatePathError} from "./errors"
 import flow from "lodash/flow"
 
 export function pagesPathTransformer(path: string) {
-  const regex = /(?:[\\/]?app[\\/].*?[\\/]?)(pages[\\/].+)$/
+  const regex = /(?:[\\/]?app[\\/].*?[\\/]?)(pages[\\/].+\.m?[tj]sx?)$/
   return (regex.exec(path) || [])[1] || path
 }
 

packages/server/src/stages/pages/pages-path-transformer.test.ts

@@ -8,6 +8,36 @@ describe("createPagesPathTransformer", () => {
       input: normalize("app/users/pages/one/two/three.tsx"),
       expected: normalize("pages/one/two/three.tsx"),
     },
+    {
+      name: "Transforms js",
+      input: normalize("app/users/pages/one/two/three.js"),
+      expected: normalize("pages/one/two/three.js"),
+    },
+    {
+      name: "Transforms ts",
+      input: normalize("app/users/pages/one/two/three.ts"),
+      expected: normalize("pages/one/two/three.ts"),
+    },
+    {
+      name: "Transforms jsx",
+      input: normalize("app/users/pages/one/two/three.jsx"),
+      expected: normalize("pages/one/two/three.jsx"),
+    },
+    {
+      name: "Transforms mjs",
+      input: normalize("app/users/pages/one/two/three.mjs"),
+      expected: normalize("pages/one/two/three.mjs"),
+    },
+    {
+      name: "Ignores non tjsx paths: css",
+      input: normalize("app/users/pages/one/two/three.css"),
+      expected: normalize("app/users/pages/one/two/three.css"),
+    },
+    {
+      name: "Ignores non tjsx paths: scss",
+      input: normalize("app/users/pages/one/two/three.scss"),
+      expected: normalize("app/users/pages/one/two/three.scss"),
+    },
     {
       name: "handles leading /",
       input: normalize("/app/users/pages/one/two/three.tsx"),

packages/server/src/stages/pages/pages.test.ts

@@ -10,7 +10,10 @@ function getStreamWithInputCache(entries: string[]) {
 
 describe("createStagePages", () => {
   it("should throw an error when there are duplicates", (done) => {
-    const stream = getStreamWithInputCache([normalize("app/api/foo"), normalize("app/api/foo")])
+    const stream = getStreamWithInputCache([
+      normalize("app/api/foo.ts"),
+      normalize("app/api/foo.ts"),
+    ])
     stream.on("error", (err) => {
       expect(err.message).toContain("conflicting api routes:")
       done()
@@ -22,12 +25,12 @@ describe("createStagePages", () => {
 
   it("should throw an error when there are duplicate pages", (done) => {
     const stream = getStreamWithInputCache(
-      ["app/foo/pages/bar", "app/pages/bar", "pages/bar"].map(normalize),
+      ["app/foo/pages/bar.ts", "app/pages/bar.ts", "pages/bar.ts"].map(normalize),
     )
     stream.on("error", (err) => {
       expect(err.message).toContain("conflicting page routes:")
       expect(err.paths).toEqual([
-        ["app/foo/pages/bar", "app/pages/bar", "pages/bar"].map(normalize),
+        ["app/foo/pages/bar.ts", "app/pages/bar.ts", "pages/bar.ts"].map(normalize),
       ])
       done()
     })
@@ -37,10 +40,12 @@ describe("createStagePages", () => {
   })
 
   it("should throw an error when there are duplicate api routes from both in pages and out", (done) => {
-    const stream = getStreamWithInputCache(["app/pages/api/bar", "app/api/bar"].map(normalize))
+    const stream = getStreamWithInputCache(
+      ["app/pages/api/bar.ts", "app/api/bar.ts"].map(normalize),
+    )
     stream.on("error", (err: DuplicatePathError) => {
       expect(err.message).toContain("conflicting api routes:")
-      expect(err.paths).toEqual([["app/pages/api/bar", "app/api/bar"].map(normalize)])
+      expect(err.paths).toEqual([["app/pages/api/bar.ts", "app/api/bar.ts"].map(normalize)])
       done()
     })
 
@@ -49,7 +54,9 @@ describe("createStagePages", () => {
   })
 
   it("should not throw an error when there are nested api routes", (done) => {
-    const stream = getStreamWithInputCache(["app/pages/foo/api/bar", "app/api/bar"].map(normalize))
+    const stream = getStreamWithInputCache(
+      ["app/pages/foo/api/bar.ts", "app/api/bar.ts"].map(normalize),
+    )
     stream.on("error", () => {
       expect(true).toBe("This should not have been called")
       done()

packages/server/src/stages/relative/index.ts

@@ -30,10 +30,11 @@ const isJavaScriptFile = (filepath: string) => filepath.match(/\.(ts|tsx|js|jsx)
 
 const isInAppFolder = (s: string, cwd: string) => s.replace(cwd + path.sep, "").indexOf("app") === 0
 
-export const patternRelativeImport = /(from\s+(?:['"]))(\.[^'"]+)(['"])/g
+export const patternRelativeImportSingle = /(import\s(?:{[^}]*})?.*(?=(?:['"])(?:\.[^'"]+)(?:['"]))(?:['"]))(\.[^'"]+)(['"])/
+export const patternRelativeImportGlobal = new RegExp(patternRelativeImportSingle, "g")
 
 export function replaceRelativeImports(content: string, replacer: (s: string) => string) {
-  return content.replace(patternRelativeImport, (...args) => {
+  return content.replace(patternRelativeImportGlobal, (...args) => {
     const [, start, importPath, end] = args
     return [start, replacer(importPath), end].join("")
   })

packages/server/src/stages/relative/pattern-relative-import.test.ts

@@ -0,0 +1,25 @@
+import {patternRelativeImportSingle} from "."
+describe("patternRelativeImport", () => {
+  it("matches an import statement", () => {
+    const matches = `import from "./bar/baz"`.match(patternRelativeImportSingle)
+
+    expect(matches && matches[2]).toEqual("./bar/baz")
+  })
+
+  it("matches a multiline import statement", () => {
+    const input = `import {
+  foo, 
+  bar
+} from "./ding"`
+    const matches = input.match(patternRelativeImportSingle)
+
+    expect(matches && matches[2]).toEqual("./ding")
+  })
+
+  it("matches an import statement with no specifier", () => {
+    const input = `import from "./ding"`
+    const matches = input.match(patternRelativeImportSingle)
+
+    expect(matches && matches[2]).toEqual("./ding")
+  })
+})

packages/server/src/stages/relative/relative.test.ts

@@ -9,53 +9,55 @@ describe("relative", () => {
       {
         path: normalize("/projects/blitz/blitz/app/users/pages.ts"),
         contents: `import {getFoo} from 'app/foo/bar';
-  import from "app/thing/bar"
-  import from 'app/thing/bar'`,
+import from "app/thing/bar"
+import from 'app/thing/bar'
+import 'app/thing/bar'`,
       },
       {
         path: normalize("/projects/blitz/blitz/app/users/foo.jpeg"),
         contents: `import {getFoo} from 'app/foo/bar';
-  import from "../thing/bar"
-  import from '../thing/bar'`,
+import from "../thing/bar"
+import from '../thing/bar'`,
       },
       {
         path: normalize("/projects/blitz/blitz/app/users/bar.tsx"),
         contents: `import {getFoo} from 'app/foo/bar';
-  import from "app/thing/bar"
-  import from 'app/thing/bar'`,
+import from "app/thing/bar"
+import from 'app/thing/bar'`,
       },
       {
         path: normalize("/projects/blitz/blitz/app/users/baz.js"),
         contents: `import {getFoo} from 'app/foo/bar';
-  import from "app/thing/bar"
-  import from 'app/thing/bar'`,
+import from "app/thing/bar"
+import from 'app/thing/bar'`,
       },
     ]
 
     const files = [
       {
         path: normalize("/projects/blitz/blitz/app/users/pages.ts"),
         contents: `import {getFoo} from 'app/foo/bar';
-  import from "../thing/bar"
-  import from '../thing/bar'`,
+import from "../thing/bar"
+import from '../thing/bar'
+import '../thing/bar'`,
       },
       {
         path: normalize("/projects/blitz/blitz/app/users/foo.jpeg"),
         contents: `import {getFoo} from 'app/foo/bar';
-  import from "../thing/bar"
-  import from '../thing/bar'`,
+import from "../thing/bar"
+import from '../thing/bar'`,
       },
       {
         path: normalize("/projects/blitz/blitz/app/users/bar.tsx"),
         contents: `import {getFoo} from 'app/foo/bar';
-  import from "../thing/bar"
-  import from '../thing/bar'`,
+import from "../thing/bar"
+import from '../thing/bar'`,
       },
       {
         path: normalize("/projects/blitz/blitz/app/users/baz.js"),
         contents: `import {getFoo} from 'app/foo/bar';
-  import from "../thing/bar"
-  import from '../thing/bar'`,
+import from "../thing/bar"
+import from '../thing/bar'`,
       },
     ]
     const {stream} = createStageRelative(mockStageArgs({cwd: normalize("/projects/blitz/blitz")}))