diff --git a/dist/functions/bundler/all-pages-esbuild-context-bundler.d.ts b/dist/functions/bundler/all-pages-esbuild-context-bundler.d.ts index 2e40b68..0941c94 100644 --- a/dist/functions/bundler/all-pages-esbuild-context-bundler.d.ts +++ b/dist/functions/bundler/all-pages-esbuild-context-bundler.d.ts @@ -1,3 +1,4 @@ +import * as esbuild from "esbuild"; import type { BundlerCTXMap } from "../../types"; type Params = { post_build_fn?: (params: { @@ -17,4 +18,13 @@ type Params = { */ export declare function isHydrationEntryPage(local_path: string): boolean; export default function allPagesESBuildContextBundler(params?: Params): Promise; +/** + * CommonJS `require("react")` (e.g. `use-sync-external-store/shim`, pulled + * in by MUI) for a module that is external in ESM output becomes esbuild's + * `__require`, which throws "Dynamic require of "react" is not supported" in + * the browser. Give those requires a small bundled ESM module that imports + * the external instead; esbuild wires `require()` to it. + */ +export declare function externalRequireShim(specifier: string): string; +export declare function forceExternalReact(): esbuild.Plugin; export {}; diff --git a/dist/functions/bundler/all-pages-esbuild-context-bundler.js b/dist/functions/bundler/all-pages-esbuild-context-bundler.js index d2188a0..ecb5d7d 100644 --- a/dist/functions/bundler/all-pages-esbuild-context-bundler.js +++ b/dist/functions/bundler/all-pages-esbuild-context-bundler.js @@ -128,18 +128,44 @@ export default async function allPagesESBuildContextBundler(params) { ], (line) => log.error(line)); } } -function forceExternalReact() { +const ExternalRequireNamespace = "bunext-external-require"; +/** + * CommonJS `require("react")` (e.g. `use-sync-external-store/shim`, pulled + * in by MUI) for a module that is external in ESM output becomes esbuild's + * `__require`, which throws "Dynamic require of "react" is not supported" in + * the browser. Give those requires a small bundled ESM module that imports + * the external instead; esbuild wires `require()` to it. + */ +export function externalRequireShim(specifier) { + const spec = JSON.stringify(specifier); + // A namespace import never fails to link, even when the vendor module has + // no default export (jsx-runtime); fall back to the namespace for + // `require(...).default` (Babel interop). + return [ + `import * as ns from ${spec};`, + `export * from ${spec};`, + `export default ns.default ?? ns;`, + ].join("\n"); +} +export function forceExternalReact() { return { name: "force-external-react", setup(build) { build.onResolve({ filter: /^react(-dom)?(\/.*)?$/ }, (args) => { if (args.pluginData?.externalReact) return null; + if (args.kind === "require-call") { + return { path: args.path, namespace: ExternalRequireNamespace }; + } return { path: args.path, external: true, }; }); + build.onLoad({ filter: /.*/, namespace: ExternalRequireNamespace }, (args) => ({ + contents: externalRequireShim(args.path), + loader: "js", + })); }, }; } diff --git a/dist/functions/bundler/pages-ssr-context-bundler.js b/dist/functions/bundler/pages-ssr-context-bundler.js index ae82a85..c62336b 100644 --- a/dist/functions/bundler/pages-ssr-context-bundler.js +++ b/dist/functions/bundler/pages-ssr-context-bundler.js @@ -46,7 +46,7 @@ async function computeEntryToPage() { } catch (error) { } for (const page of pages) { - if (page.local_path.match(/\/pages\/api\//) || + if (page.url_path.startsWith("/api/") || page.local_path.match(/\.server\.tsx?$/)) { const ts = await Bun.file(page.local_path).text(); if (ts.match(/(export default)|(export \w+ handler)|(export \w+ server)/)) { diff --git a/dist/utils/grab-all-pages.d.ts b/dist/utils/grab-all-pages.d.ts index 82b09ec..399f4e8 100644 --- a/dist/utils/grab-all-pages.d.ts +++ b/dist/utils/grab-all-pages.d.ts @@ -5,4 +5,13 @@ type Params = { include_server?: boolean; }; export default function grabAllPages(params?: Params): PageFiles[]; +/** + * The URL path is the file's path relative to the pages root, so a nested + * directory that happens to be called `pages` (e.g. `src/pages/admin/pages/ + * index.tsx` -> `/admin/pages/index`) is kept, not treated as the root. + */ +export declare function grabPageFileObject({ file_path, pages_root, }: { + file_path: string; + pages_root: string; +}): PageFiles | undefined; export {}; diff --git a/dist/utils/grab-all-pages.js b/dist/utils/grab-all-pages.js index cfdbfe2..b4699c9 100644 --- a/dist/utils/grab-all-pages.js +++ b/dist/utils/grab-all-pages.js @@ -17,13 +17,9 @@ export default function grabAllPages(params) { } return pages; } -function grabPageDirRecursively({ page_dir, include_server, }) { +function grabPageDirRecursively({ page_dir, pages_root = page_dir, include_server, }) { const pages = readdirSync(page_dir); const pages_files = []; - const root_pages_file = grabPageFileObject({ file_path: `` }); - if (root_pages_file) { - pages_files.push(root_pages_file); - } for (let i = 0; i < pages.length; i++) { const page = pages[i]; const page_name = page.split("/").pop(); @@ -56,6 +52,7 @@ function grabPageDirRecursively({ page_dir, include_server, }) { continue; const new_page_files = grabPageDirRecursively({ page_dir: full_page_path, + pages_root, include_server, }); pages_files.push(...new_page_files); @@ -63,6 +60,7 @@ function grabPageDirRecursively({ page_dir, include_server, }) { else if (page.match(/\.(ts|js)x?$/)) { const pages_file = grabPageFileObject({ file_path: full_page_path, + pages_root, }); if (pages_file) { pages_files.push(pages_file); @@ -77,14 +75,22 @@ function grabPageDirRecursively({ page_dir, include_server, }) { return 0; }); } -function grabPageFileObject({ file_path, }) { - let url_path = file_path - .replace(/.*\/pages\//, "/") - ?.replace(/\.(ts|js)x?$/, ""); - let file_name = url_path.split("/").pop(); +/** + * The URL path is the file's path relative to the pages root, so a nested + * directory that happens to be called `pages` (e.g. `src/pages/admin/pages/ + * index.tsx` -> `/admin/pages/index`) is kept, not treated as the root. + */ +export function grabPageFileObject({ file_path, pages_root, }) { + const relative_path = path + .relative(pages_root, file_path) + .split(path.sep) + .join("/"); + if (!relative_path || relative_path.startsWith("../")) + return; + const url_path = "/" + relative_path.replace(/\.(ts|js)x?$/, ""); + const file_name = url_path.split("/").pop(); if (!file_name) return; - // const transformed_path = pagePathTransform({ page_path: file_path }); return { local_path: file_path, url_path, diff --git a/package.json b/package.json index ba9369a..94bb713 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@moduletrace/bunext", - "version": "1.4.0", + "version": "1.4.1", "main": "dist/index.js", "module": "index.ts", "dependencies": { diff --git a/src/__tests__/functions/bundler/force-external-react.test.ts b/src/__tests__/functions/bundler/force-external-react.test.ts new file mode 100644 index 0000000..b6ebbc0 --- /dev/null +++ b/src/__tests__/functions/bundler/force-external-react.test.ts @@ -0,0 +1,67 @@ +import { describe, it, expect, afterAll } from "bun:test"; +import * as esbuild from "esbuild"; +import fs from "fs"; +import path from "path"; +import { + externalRequireShim, + forceExternalReact, +} from "../../../functions/bundler/all-pages-esbuild-context-bundler"; + +// Under /test (gitignored) so the bundle can resolve the repo's react at runtime +const outDir = path.resolve(__dirname, "../../../../test/force-external-react"); + +afterAll(() => { + fs.rmSync(outDir, { recursive: true, force: true }); +}); + +async function bundle(contents: string) { + fs.mkdirSync(outDir, { recursive: true }); + const outfile = path.join(outDir, `out-${Date.now()}-${Math.random().toString(36).slice(2)}.mjs`); + await esbuild.build({ + stdin: { contents, resolveDir: outDir, loader: "js" }, + bundle: true, + format: "esm", + platform: "browser", + outfile, + plugins: [forceExternalReact()], + logLevel: "silent", + }); + return { outfile, code: fs.readFileSync(outfile, "utf-8") }; +} + +describe("forceExternalReact", () => { + it("keeps ESM imports of react external", async () => { + const { code } = await bundle(`import { useState } from "react"; export default useState;`); + expect(code).toContain(`from "react"`); + expect(code).not.toContain("Dynamic require"); + }); + + it("turns CommonJS require('react') into an import of the external", async () => { + // The shape of use-sync-external-store/shim and Babel-compiled packages + const { outfile, code } = await bundle(` + const React = require("react"); + const interop = (m) => (m && m.__esModule ? m : { default: m }); + const jsx = require("react/jsx-runtime"); + export const plain = typeof React.useState; + export const viaDefault = typeof interop(require("react")).default.createElement; + export const jsxFn = typeof jsx.jsx; + export const jsxDefault = typeof (jsx.default || jsx).jsx; + `); + + expect(code).not.toMatch(/__require\("react/); + expect(code).toContain(`from "react"`); + + const mod = await import(outfile); + expect(mod.plain).toBe("function"); + expect(mod.viaDefault).toBe("function"); + expect(mod.jsxFn).toBe("function"); + expect(mod.jsxDefault).toBe("function"); + }); + + it("shim re-exports everything and a default", () => { + const shim = externalRequireShim("react-dom"); + expect(shim).toContain(`import * as ns from "react-dom";`); + expect(shim).toContain(`export * from "react-dom";`); + expect(shim).toContain("export default ns.default ?? ns;"); + }); +}); diff --git a/src/__tests__/utils/grab-all-pages.test.ts b/src/__tests__/utils/grab-all-pages.test.ts new file mode 100644 index 0000000..275e85a --- /dev/null +++ b/src/__tests__/utils/grab-all-pages.test.ts @@ -0,0 +1,58 @@ +import { describe, it, expect, afterAll, beforeAll } from "bun:test"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import grabAllPages, { grabPageFileObject } from "../../utils/grab-all-pages"; + +// grabAllPages skips anything under __tests__, so fixtures live in the OS tmp dir +const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "bunext-pages-")); +const pagesDir = path.join(tmpDir, "src", "pages"); + +const files = [ + "index.tsx", + "about/index.tsx", + "admin/pages/index.tsx", + "admin/su/pages/[page_id]/edit/index.tsx", + "api/pages/list.ts", +]; + +beforeAll(() => { + for (const f of files) { + const full = path.join(pagesDir, f); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, "export default function P() { return null }\n"); + } + (global as any).BUNEXT_DIR_NAMES = { PAGES_DIR: pagesDir }; + (global as any).BUNEXT_CONSTANTS = { RouteIgnorePatterns: [/\/\(/] }; + (global as any).BUNEXT_CONFIG = {}; +}); + +afterAll(() => { + (global as any).BUNEXT_DIR_NAMES = undefined; + (global as any).BUNEXT_CONSTANTS = undefined; + (global as any).BUNEXT_CONFIG = undefined; + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +describe("grabAllPages", () => { + it("derives URL paths relative to src/pages, even under a nested `pages` dir", () => { + const urls = grabAllPages().map((p) => p.url_path).sort(); + expect(urls).toEqual([ + "/about/index", + "/admin/pages/index", + "/admin/su/pages/[page_id]/edit/index", + "/api/pages/list", + "/index", + ]); + }); + + it("splits API routes by URL path, not by any `pages/api` in the file path", () => { + expect(grabAllPages({ api_only: true }).map((p) => p.url_path)).toEqual(["/api/pages/list"]); + expect(grabAllPages({ exclude_api: true }).some((p) => p.url_path.startsWith("/api/"))).toBe(false); + }); + + it("ignores paths outside the pages root", () => { + expect(grabPageFileObject({ file_path: path.join(tmpDir, "other.tsx"), pages_root: pagesDir })).toBeUndefined(); + expect(grabPageFileObject({ file_path: pagesDir, pages_root: pagesDir })).toBeUndefined(); + }); +}); diff --git a/src/functions/bundler/all-pages-esbuild-context-bundler.ts b/src/functions/bundler/all-pages-esbuild-context-bundler.ts index bf545e8..0c66f52 100644 --- a/src/functions/bundler/all-pages-esbuild-context-bundler.ts +++ b/src/functions/bundler/all-pages-esbuild-context-bundler.ts @@ -180,17 +180,49 @@ export default async function allPagesESBuildContextBundler(params?: Params) { } } -function forceExternalReact(): esbuild.Plugin { +const ExternalRequireNamespace = "bunext-external-require"; + +/** + * CommonJS `require("react")` (e.g. `use-sync-external-store/shim`, pulled + * in by MUI) for a module that is external in ESM output becomes esbuild's + * `__require`, which throws "Dynamic require of "react" is not supported" in + * the browser. Give those requires a small bundled ESM module that imports + * the external instead; esbuild wires `require()` to it. + */ +export function externalRequireShim(specifier: string): string { + const spec = JSON.stringify(specifier); + // A namespace import never fails to link, even when the vendor module has + // no default export (jsx-runtime); fall back to the namespace for + // `require(...).default` (Babel interop). + return [ + `import * as ns from ${spec};`, + `export * from ${spec};`, + `export default ns.default ?? ns;`, + ].join("\n"); +} + +export function forceExternalReact(): esbuild.Plugin { return { name: "force-external-react", setup(build) { build.onResolve({ filter: /^react(-dom)?(\/.*)?$/ }, (args) => { if (args.pluginData?.externalReact) return null; + if (args.kind === "require-call") { + return { path: args.path, namespace: ExternalRequireNamespace }; + } return { path: args.path, external: true, }; }); + + build.onLoad( + { filter: /.*/, namespace: ExternalRequireNamespace }, + (args) => ({ + contents: externalRequireShim(args.path), + loader: "js", + }), + ); }, }; } diff --git a/src/functions/bundler/pages-ssr-context-bundler.ts b/src/functions/bundler/pages-ssr-context-bundler.ts index df60bc0..e06769b 100644 --- a/src/functions/bundler/pages-ssr-context-bundler.ts +++ b/src/functions/bundler/pages-ssr-context-bundler.ts @@ -68,7 +68,7 @@ async function computeEntryToPage(): Promise<{ for (const page of pages) { if ( - page.local_path.match(/\/pages\/api\//) || + page.url_path.startsWith("/api/") || page.local_path.match(/\.server\.tsx?$/) ) { const ts = await Bun.file(page.local_path).text(); diff --git a/src/utils/grab-all-pages.ts b/src/utils/grab-all-pages.ts index 01b7e80..61428b5 100644 --- a/src/utils/grab-all-pages.ts +++ b/src/utils/grab-all-pages.ts @@ -32,20 +32,17 @@ export default function grabAllPages(params?: Params) { function grabPageDirRecursively({ page_dir, + pages_root = page_dir, include_server, }: { page_dir: string; + /** `src/pages`: URL paths are relative to it, however deep we recurse */ + pages_root?: string; include_server?: boolean; }) { const pages = readdirSync(page_dir); const pages_files: PageFiles[] = []; - const root_pages_file = grabPageFileObject({ file_path: `` }); - - if (root_pages_file) { - pages_files.push(root_pages_file); - } - for (let i = 0; i < pages.length; i++) { const page = pages[i]; const page_name = page.split("/").pop(); @@ -88,6 +85,7 @@ function grabPageDirRecursively({ if (checkExcludedPatterns({ path: full_page_path })) continue; const new_page_files = grabPageDirRecursively({ page_dir: full_page_path, + pages_root, include_server, }); @@ -95,6 +93,7 @@ function grabPageDirRecursively({ } else if (page.match(/\.(ts|js)x?$/)) { const pages_file = grabPageFileObject({ file_path: full_page_path, + pages_root, }); if (pages_file) { @@ -110,20 +109,30 @@ function grabPageDirRecursively({ }); } -function grabPageFileObject({ +/** + * The URL path is the file's path relative to the pages root, so a nested + * directory that happens to be called `pages` (e.g. `src/pages/admin/pages/ + * index.tsx` -> `/admin/pages/index`) is kept, not treated as the root. + */ +export function grabPageFileObject({ file_path, + pages_root, }: { file_path: string; + pages_root: string; }): PageFiles | undefined { - let url_path = file_path - .replace(/.*\/pages\//, "/") - ?.replace(/\.(ts|js)x?$/, ""); + const relative_path = path + .relative(pages_root, file_path) + .split(path.sep) + .join("/"); - let file_name = url_path.split("/").pop(); + if (!relative_path || relative_path.startsWith("../")) return; + + const url_path = "/" + relative_path.replace(/\.(ts|js)x?$/, ""); + + const file_name = url_path.split("/").pop(); if (!file_name) return; - // const transformed_path = pagePathTransform({ page_path: file_path }); - return { local_path: file_path, url_path,