Skip to content

Commit

Permalink
Prevent require cycles (#29937)
Browse files Browse the repository at this point in the history
Prevent require cycles because the Metro bundler warns about them, even in libraries.

GitOrigin-RevId: cc0578cbb97b84fdbb886b6c9f81d8e57a541d7f
  • Loading branch information
thomasballinger authored and Convex, Inc. committed Sep 18, 2024
1 parent 4539827 commit b9ada8f
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 20 deletions.
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@
"test": "vitest --silent",
"test-not-silent": "vitest",
"new-test": "vitest",
"test-esm": "node ./scripts/test-esm.mjs && ./scripts/checkdeps.mjs",
"test-esm": "node ./scripts/test-esm.mjs && ./scripts/checkdeps.mjs && ./scripts/checkimports.mjs",
"pack-internal": "echo TODO maybe set an environment variable"
},
"keywords": [
Expand All @@ -164,8 +164,7 @@
"dependencies": {
"esbuild": "0.23.0",
"jwt-decode": "^4.0.0",
"prettier": "3.2.5",
"globals": "~15.9.0"
"prettier": "3.2.5"
},
"peerDependencies": {
"@auth0/auth0-react": "^2.0.1",
Expand Down Expand Up @@ -234,6 +233,7 @@
"eslint-plugin-require-extensions": "~0.1.3",
"fetch-retry": "~5.0.6",
"find-up": "^6.3.0",
"globals": "~15.9.0",
"happy-dom": "~14.12.3",
"inquirer": "^9.1.4",
"inquirer-search-list": "~1.2.6",
Expand All @@ -247,6 +247,7 @@
"react-dom": "^18.0.0",
"semver": "^7.6.0",
"shx": "~0.3.4",
"skott": "~0.35.3",
"strip-ansi": "^7.0.1",
"tsx": "~4.15.6",
"typedoc": "^0.24.6",
Expand Down
50 changes: 50 additions & 0 deletions scripts/checkimports.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/usr/bin/env node
import { fileURLToPath } from "url";
import { dirname } from "path";
import skott from "skott";

const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);
const __root = dirname(__dirname);

async function entrypointHasCycles(entrypoint) {
// Note that skott can do a lot of other things too!
const { useGraph } = await skott({
entrypoint: `./dist/esm/${entrypoint}/index.js`,
incremental: false,
cwd: __root,
includeBaseDir: true,
verbose: false,
});
const { findCircularDependencies } = useGraph();

const circular = findCircularDependencies();
if (circular.length) {
console.log("Found import cycles by traversing", entrypoint);
console.log(circular);
return false;
}
return true;
}

let allOk = true;
// These haven't been fixed yet so we don't fail if they have cycles.
for (const entrypoint of [
"bundler",
"nextjs",
"react",
"react-auth0",
"react-clerk",
"values",
// don't care about cycles in CLI
]) {
const ok = await entrypointHasCycles(entrypoint);
allOk &&= ok;
}

if (!(await entrypointHasCycles("server"))) {
process.exit(1);
} else {
console.log("No import cycles found in server.");
process.exit(0);
}
6 changes: 1 addition & 5 deletions src/server/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import { Expand, UnionToIntersection } from "../type_utils.js";
import { PaginationOptions, PaginationResult } from "./pagination.js";
import { getFunctionAddress } from "./impl/actions_impl.js";
import { functionName } from "./functionName.js";

/**
* The type of a Convex function.
Expand Down Expand Up @@ -62,11 +63,6 @@ export type FunctionReference<
_componentPath: ComponentPath;
};

/**
* A symbol for accessing the name of a {@link FunctionReference} at runtime.
*/
export const functionName = Symbol.for("functionName");

/**
* Get the name of a function from a {@link FunctionReference}.
*
Expand Down
11 changes: 1 addition & 10 deletions src/server/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,7 @@ import {
ComponentDefinitionAnalysis,
ComponentDefinitionType,
} from "./definition.js";

export const toReferencePath = Symbol.for("toReferencePath");

export function extractReferencePath(reference: any): string | null {
return reference[toReferencePath] ?? null;
}

export function isFunctionHandle(s: string): boolean {
return s.startsWith("function://");
}
import { toReferencePath } from "./paths.js";

/**
* @internal
Expand Down
9 changes: 9 additions & 0 deletions src/server/components/paths.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export const toReferencePath = Symbol.for("toReferencePath");

export function extractReferencePath(reference: any): string | null {
return reference[toReferencePath] ?? null;
}

export function isFunctionHandle(s: string): boolean {
return s.startsWith("function://");
}
4 changes: 4 additions & 0 deletions src/server/functionName.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/**
* A symbol for accessing the name of a {@link FunctionReference} at runtime.
*/
export const functionName = Symbol.for("functionName");
4 changes: 4 additions & 0 deletions src/server/functions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/**
* A symbol for accessing the name of a {@link FunctionReference} at runtime.
*/
export const functionName = Symbol.for("functionName");
5 changes: 3 additions & 2 deletions src/server/impl/actions_impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { convexToJson, jsonToConvex, Value } from "../../values/index.js";
import { version } from "../../index.js";
import { performAsyncSyscall } from "./syscall.js";
import { parseArgs } from "../../common/index.js";
import { functionName, FunctionReference } from "../../server/api.js";
import { extractReferencePath, isFunctionHandle } from "../components/index.js";
import { FunctionReference } from "../../server/api.js";
import { extractReferencePath, isFunctionHandle } from "../components/paths.js";
import { functionName } from "../functionName.js";

function syscallArgs(
requestId: string,
Expand Down

0 comments on commit b9ada8f

Please sign in to comment.