From 17b5c07bedd450904a7539f32246b34fd730c212 Mon Sep 17 00:00:00 2001 From: Josh GM Walker <56300765+Josh-Walker-GM@users.noreply.github.com> Date: Sun, 22 Oct 2023 17:10:13 +0100 Subject: [PATCH 1/3] error on duplicate page imports --- ...babel-plugin-redwood-routes-auto-loader.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/babel-config/src/plugins/babel-plugin-redwood-routes-auto-loader.ts b/packages/babel-config/src/plugins/babel-plugin-redwood-routes-auto-loader.ts index 03ce0e6d3460..7af440d0b4b3 100644 --- a/packages/babel-config/src/plugins/babel-plugin-redwood-routes-auto-loader.ts +++ b/packages/babel-config/src/plugins/babel-plugin-redwood-routes-auto-loader.ts @@ -43,6 +43,26 @@ export default function ( // @NOTE: This var gets mutated inside the visitors let pages = processPagesDir().map(withRelativeImports) + // Currently processPagesDir() can return duplicate entries when there are multiple files + // ending in Page in the individual page directories. This will cause an error upstream. + // Here we check for duplicates and throw a more helpful error message. + const duplicatePageImports = new Set() + const sortedPageImports = pages.map((page) => page.importName).sort() + for (let i = 0; i < sortedPageImports.length - 1; i++) { + if (sortedPageImports[i + 1] === sortedPageImports[i]) { + duplicatePageImports.add(sortedPageImports[i]) + } + } + if (duplicatePageImports.size > 0) { + throw new Error( + `Unable to find only a single file ending in 'Page.{js,jsx,ts,tsx}' in the follow page directories: ${Array.from( + duplicatePageImports + ) + .map((name) => `'${name}'`) + .join(', ')}` + ) + } + return { name: 'babel-plugin-redwood-routes-auto-loader', visitor: { From d6653d5daed0c5b875b896480bb6a9ae87197ff2 Mon Sep 17 00:00:00 2001 From: Josh GM Walker <56300765+Josh-Walker-GM@users.noreply.github.com> Date: Sun, 22 Oct 2023 17:11:06 +0100 Subject: [PATCH 2/3] add test for duplicate page import error message --- .../route-auto-loader/failure/redwood.toml | 10 ++++++ .../failure/web/src/Routes.tsx | 11 +++++++ .../web/src/pages/HomePage/HomePage.tsx | 9 +++++ .../web/src/pages/HomePage/useHomePage.tsx | 5 +++ ...-plugin-redwood-routes-auto-loader.test.ts | 33 ++++++++++++++++--- 5 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/redwood.toml create mode 100644 packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/Routes.tsx create mode 100644 packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/pages/HomePage/HomePage.tsx create mode 100644 packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/pages/HomePage/useHomePage.tsx diff --git a/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/redwood.toml b/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/redwood.toml new file mode 100644 index 000000000000..286d9315b1ba --- /dev/null +++ b/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/redwood.toml @@ -0,0 +1,10 @@ +[web] + port = 8910 + apiProxyPath = "/api/functions" + +[api] + port = 8911 + [api.paths] + functions = './api/src/functions' + graphql = './api/src/graphql' + generated = './api/generated' diff --git a/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/Routes.tsx b/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/Routes.tsx new file mode 100644 index 000000000000..6e95d14b53c8 --- /dev/null +++ b/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/Routes.tsx @@ -0,0 +1,11 @@ +import { Router, Route } from '@redwoodjs/router' + +const Routes = () => { + return ( + + + + ) +} + +export default Routes diff --git a/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/pages/HomePage/HomePage.tsx b/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/pages/HomePage/HomePage.tsx new file mode 100644 index 000000000000..c184a81e8e9b --- /dev/null +++ b/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/pages/HomePage/HomePage.tsx @@ -0,0 +1,9 @@ +const HomePage = () => { + return ( +
+

HomePage

+
+ ) +} + +export default HomePage diff --git a/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/pages/HomePage/useHomePage.tsx b/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/pages/HomePage/useHomePage.tsx new file mode 100644 index 000000000000..e93ada786af0 --- /dev/null +++ b/packages/babel-config/src/plugins/__tests__/__fixtures__/route-auto-loader/failure/web/src/pages/HomePage/useHomePage.tsx @@ -0,0 +1,5 @@ +const useHomePage = () => { + return 'useHomePage' +} + +export default useHomePage diff --git a/packages/babel-config/src/plugins/__tests__/babel-plugin-redwood-routes-auto-loader.test.ts b/packages/babel-config/src/plugins/__tests__/babel-plugin-redwood-routes-auto-loader.test.ts index 9c3e63601dc2..9c27d7c93800 100644 --- a/packages/babel-config/src/plugins/__tests__/babel-plugin-redwood-routes-auto-loader.test.ts +++ b/packages/babel-config/src/plugins/__tests__/babel-plugin-redwood-routes-auto-loader.test.ts @@ -7,11 +7,6 @@ import { getPaths } from '@redwoodjs/project-config' import babelRoutesAutoLoader from '../babel-plugin-redwood-routes-auto-loader' -const FIXTURE_PATH = path.resolve( - __dirname, - '../../../../../__fixtures__/example-todo-main/' -) - const transform = (filename: string) => { const code = fs.readFileSync(filename, 'utf-8') return babel.transform(code, { @@ -21,7 +16,35 @@ const transform = (filename: string) => { }) } +describe('mulitiple files ending in Page.{js,jsx,ts,tsx}', () => { + const FAILURE_FIXTURE_PATH = path.resolve( + __dirname, + './__fixtures__/route-auto-loader/failure' + ) + + beforeAll(() => { + process.env.RWJS_CWD = FAILURE_FIXTURE_PATH + }) + + afterAll(() => { + delete process.env.RWJS_CWD + }) + + test('Fails with appropriate message', () => { + expect(() => { + transform(getPaths().web.routes) + }).toThrowError( + "Unable to find only a single file ending in 'Page.{js,jsx,ts,tsx}' in the follow page directories: 'HomePage" + ) + }) +}) + describe('page auto loader correctly imports pages', () => { + const FIXTURE_PATH = path.resolve( + __dirname, + '../../../../../__fixtures__/example-todo-main/' + ) + let result: babel.BabelFileResult | null beforeAll(() => { From ce11673213a6b1578e3291182b463e1e4824af08 Mon Sep 17 00:00:00 2001 From: Josh GM Walker <56300765+Josh-Walker-GM@users.noreply.github.com> Date: Mon, 23 Oct 2023 21:25:32 +0100 Subject: [PATCH 3/3] respond to feedback, renaming --- .../babel-plugin-redwood-routes-auto-loader.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/babel-config/src/plugins/babel-plugin-redwood-routes-auto-loader.ts b/packages/babel-config/src/plugins/babel-plugin-redwood-routes-auto-loader.ts index 7af440d0b4b3..7457db2babcc 100644 --- a/packages/babel-config/src/plugins/babel-plugin-redwood-routes-auto-loader.ts +++ b/packages/babel-config/src/plugins/babel-plugin-redwood-routes-auto-loader.ts @@ -46,17 +46,17 @@ export default function ( // Currently processPagesDir() can return duplicate entries when there are multiple files // ending in Page in the individual page directories. This will cause an error upstream. // Here we check for duplicates and throw a more helpful error message. - const duplicatePageImports = new Set() - const sortedPageImports = pages.map((page) => page.importName).sort() - for (let i = 0; i < sortedPageImports.length - 1; i++) { - if (sortedPageImports[i + 1] === sortedPageImports[i]) { - duplicatePageImports.add(sortedPageImports[i]) + const duplicatePageImportNames = new Set() + const sortedPageImportNames = pages.map((page) => page.importName).sort() + for (let i = 0; i < sortedPageImportNames.length - 1; i++) { + if (sortedPageImportNames[i + 1] === sortedPageImportNames[i]) { + duplicatePageImportNames.add(sortedPageImportNames[i]) } } - if (duplicatePageImports.size > 0) { + if (duplicatePageImportNames.size > 0) { throw new Error( `Unable to find only a single file ending in 'Page.{js,jsx,ts,tsx}' in the follow page directories: ${Array.from( - duplicatePageImports + duplicatePageImportNames ) .map((name) => `'${name}'`) .join(', ')}`