Skip to content

Commit

Permalink
fix(admin-vite-plugin): Normalize file paths and add tests (#9595)
Browse files Browse the repository at this point in the history
**What**
- #9338 had a regression which caused the import path in some virtual modules to be invalid on Windows.
- This PR fixes the issue so we now again create the correct import paths, and adds tests to prevent this from slipping in again.
  • Loading branch information
kasperkristensen authored Oct 15, 2024
1 parent 84fa6cc commit 813efea
Show file tree
Hide file tree
Showing 15 changed files with 1,069 additions and 14 deletions.
7 changes: 5 additions & 2 deletions packages/admin/admin-vite-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@
],
"scripts": {
"build": "tsup",
"watch": "tsup --watch"
"watch": "tsup --watch",
"test": "vitest --run",
"test:watch": "vitest"
},
"devDependencies": {
"@babel/types": "7.25.6",
"@types/node": "^20.10.4",
"tsup": "8.0.1",
"typescript": "5.3.3",
"vite": "^5.2.11"
"vite": "^5.2.11",
"vitest": "^2.1.3"
},
"peerDependencies": {
"vite": "^5.0.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
traverse,
} from "../babel"
import { logger } from "../logger"
import { crawl, getParserOptions } from "../utils"
import { crawl, getParserOptions, normalizePath } from "../utils"
import { getConfigArgument, getModel, validateLink } from "./helpers"

type CustomFieldDisplay = {
Expand Down Expand Up @@ -288,5 +288,6 @@ function generateCustomFieldConfigName(index: number): string {
}

function generateImport(file: string, index: number): string {
return `import ${generateCustomFieldConfigName(index)} from "${file}"`
const path = normalizePath(file)
return `import ${generateCustomFieldConfigName(index)} from "${path}"`
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
traverse,
} from "../babel"
import { logger } from "../logger"
import { crawl, getParserOptions } from "../utils"
import { crawl, getParserOptions, normalizePath } from "../utils"
import { getConfigArgument, getModel, validateLink } from "./helpers"

type CustomFieldConfigField = {
Expand Down Expand Up @@ -263,7 +263,8 @@ function generateCustomFieldConfigName(index: number): string {
}

function generateImport(file: string, index: number): string {
return `import ${generateCustomFieldConfigName(index)} from "${file}"`
const path = normalizePath(file)
return `import ${generateCustomFieldConfigName(index)} from "${path}"`
}

function getForms(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
traverse,
} from "../babel"
import { logger } from "../logger"
import { crawl, getParserOptions } from "../utils"
import { crawl, getParserOptions, normalizePath } from "../utils"
import { getConfigArgument, getModel } from "./helpers"

type ParsedCustomFieldLink = {
Expand Down Expand Up @@ -138,7 +138,8 @@ function generateCustomFieldConfigName(index: number): string {
}

function generateImport(file: string, index: number): string {
return `import ${generateCustomFieldConfigName(index)} from "${file}"`
const path = normalizePath(file)
return `import ${generateCustomFieldConfigName(index)} from "${path}"`
}

function getLink(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import { describe, expect, it, vi } from "vitest"

import fs from "fs/promises"
import * as utils from "../../utils"
import { generateMenuItems } from "../generate-menu-items"

vi.mock("../../utils", async () => {
const actual = await vi.importActual("../../utils")
return {
...actual,
crawl: vi.fn(),
}
})

vi.mock("fs/promises", () => ({
default: {
readFile: vi.fn(),
},
}))

const mockFileContents = [
`
import { defineRouteConfig } from "@medusajs/admin-sdk"
const Page = () => {
return <div>Page 1</div>
}
export const config = defineRouteConfig({
label: "Page 1",
icon: "icon1",
})
export default Page
`,
`
import { defineRouteConfig } from "@medusajs/admin-sdk"
const Page = () => {
return <div>Page 2</div>
}
export const config = defineRouteConfig({
label: "Page 2",
})
export default Page
`,
]

const expectedMenuItems = `
menuItems: [
{
label: RouteConfig0.label,
icon: RouteConfig0.icon,
path: "/one",
},
{
label: RouteConfig1.label,
icon: undefined,
path: "/two",
}
]
`

describe("generateMenuItems", () => {
it("should generate menu items", async () => {
const mockFiles = [
"Users/user/medusa/src/admin/routes/one/page.tsx",
"Users/user/medusa/src/admin/routes/two/page.tsx",
]
vi.mocked(utils.crawl).mockResolvedValue(mockFiles)

vi.mocked(fs.readFile).mockImplementation(async (file) =>
Promise.resolve(mockFileContents[mockFiles.indexOf(file as string)])
)

const result = await generateMenuItems(
new Set(["Users/user/medusa/src/admin"])
)

expect(result.imports).toEqual([
`import { config as RouteConfig0 } from "Users/user/medusa/src/admin/routes/one/page.tsx"`,
`import { config as RouteConfig1 } from "Users/user/medusa/src/admin/routes/two/page.tsx"`,
])
expect(utils.normalizeString(result.code)).toEqual(
utils.normalizeString(expectedMenuItems)
)
})

it("should handle windows paths", async () => {
// Setup mocks
const mockFiles = [
"C:\\medusa\\src\\admin\\routes\\one\\page.tsx",
"C:\\medusa\\src\\admin\\routes\\two\\page.tsx",
]
vi.mocked(utils.crawl).mockResolvedValue(mockFiles)

vi.mocked(fs.readFile).mockImplementation(async (file) =>
Promise.resolve(mockFileContents[mockFiles.indexOf(file as string)])
)

const result = await generateMenuItems(new Set(["C:\\medusa\\src\\admin"]))

expect(result.imports).toEqual([
`import { config as RouteConfig0 } from "C:/medusa/src/admin/routes/one/page.tsx"`,
`import { config as RouteConfig1 } from "C:/medusa/src/admin/routes/two/page.tsx"`,
])
expect(utils.normalizeString(result.code)).toEqual(
utils.normalizeString(expectedMenuItems)
)
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import { describe, expect, it, vi } from "vitest"

import { Stats } from "fs"
import fs from "fs/promises"
import * as utils from "../../utils"
import { generateRoutes } from "../generate-routes"

// Mock the dependencies
vi.mock("../../utils", async () => {
const actual = await vi.importActual("../../utils")
return {
...actual,
crawl: vi.fn(),
}
})

vi.mock("fs/promises", () => ({
default: {
readFile: vi.fn(),
stat: vi.fn(),
},
}))

const mockFileContents = [
`
import { defineRouteConfig } from "@medusajs/admin-sdk"
const Page = () => {
return <div>Page 1</div>
}
export const config = defineRouteConfig({
label: "Page 1",
icon: "icon1",
})
export default Page
`,
`
import { defineRouteConfig } from "@medusajs/admin-sdk"
const Page = () => {
return <div>Page 2</div>
}
export const config = defineRouteConfig({
label: "Page 2",
})
export default Page
`,
]

const expectedRoutesWithoutLoaders = `
routes: [
{
Component: RouteComponent0,
loader: undefined,
path: "/one",
},
{
Component: RouteComponent1,
loader: undefined,
path: "/two",
}
]
`

const expectedRoutesWithLoaders = `
routes: [
{
Component: RouteComponent0,
loader: RouteLoader0,
path: "/one",
},
{
Component: RouteComponent1,
loader: RouteLoader1,
path: "/two",
}
]
`

describe("generateRoutes", () => {
it("should generate routes", async () => {
const mockFiles = [
"Users/user/medusa/src/admin/routes/one/page.tsx",
"Users/user/medusa/src/admin/routes/two/page.tsx",
]
vi.mocked(utils.crawl).mockResolvedValue(mockFiles)

vi.mocked(fs.readFile).mockImplementation(async (file) =>
Promise.resolve(mockFileContents[mockFiles.indexOf(file as string)])
)

vi.mocked(fs.stat).mockRejectedValue(new Error("File not found"))

const result = await generateRoutes(
new Set(["Users/user/medusa/src/admin"])
)
expect(utils.normalizeString(result.code)).toEqual(
utils.normalizeString(expectedRoutesWithoutLoaders)
)
})
it("should generate routes with loaders", async () => {
const mockFiles = [
"Users/user/medusa/src/admin/routes/one/page.tsx",
"Users/user/medusa/src/admin/routes/two/page.tsx",
]
vi.mocked(utils.crawl).mockResolvedValue(mockFiles)

vi.mocked(fs.readFile).mockImplementation(async (file) =>
Promise.resolve(mockFileContents[mockFiles.indexOf(file as string)])
)

vi.mocked(fs.stat).mockResolvedValue({} as Stats) // We just want to mock that the check passes

const result = await generateRoutes(
new Set(["Users/user/medusa/src/admin"])
)
expect(utils.normalizeString(result.code)).toEqual(
utils.normalizeString(expectedRoutesWithLoaders)
)
})
it("should handle windows paths", async () => {
const mockFiles = [
"C:\\medusa\\src\\admin\\routes\\one\\page.tsx",
"C:\\medusa\\src\\admin\\routes\\two\\page.tsx",
]
vi.mocked(utils.crawl).mockResolvedValue(mockFiles)

vi.mocked(fs.readFile).mockImplementation(async (file) =>
Promise.resolve(mockFileContents[mockFiles.indexOf(file as string)])
)

vi.mocked(fs.stat).mockRejectedValue(new Error("File not found"))

const result = await generateRoutes(new Set(["C:\\medusa\\src\\admin"]))

expect(utils.normalizeString(result.code)).toEqual(
utils.normalizeString(expectedRoutesWithoutLoaders)
)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import fs from "fs/promises"
import { outdent } from "outdent"
import { isIdentifier, isObjectProperty, parse, traverse } from "../babel"
import { logger } from "../logger"
import { crawl, getConfigObjectProperties, getParserOptions } from "../utils"
import {
crawl,
getConfigObjectProperties,
getParserOptions,
normalizePath,
} from "../utils"
import { getRoute } from "./helpers"

type MenuItem = {
Expand Down Expand Up @@ -90,7 +95,8 @@ async function parseFile(
}

function generateImport(file: string, index: number): string {
return `import { config as ${generateRouteConfigName(index)} } from "${file}"`
const path = normalizePath(file)
return `import { config as ${generateRouteConfigName(index)} } from "${path}"`
}

function generateMenuItem(
Expand Down
10 changes: 9 additions & 1 deletion packages/admin/admin-vite-plugin/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
} from "./babel"

export function normalizePath(file: string) {
return path.normalize(file).split(path.sep).join("/")
return path.normalize(file.replace(/\\/g, "/"))
}

/**
Expand Down Expand Up @@ -145,3 +145,11 @@ export function isFileInAdminSubdirectory(
const normalizedPath = normalizePath(file)
return normalizedPath.includes(`/src/admin/${subdirectory}/`)
}

/**
* Test util to normalize strings, so they can be compared without taking
* whitespace into account.
*/
export function normalizeString(str: string): string {
return str.replace(/\s+/g, " ").trim()
}
Loading

0 comments on commit 813efea

Please sign in to comment.