Skip to content

Commit

Permalink
Fixed validation in the resolveErrors plugin (#597)
Browse files Browse the repository at this point in the history
* Fixed validation in the `resolveErrors` plugin

* Use import instead of require to load `stack-utils` (#596)

* remove redundant tsconfig options

* ok, ok, esModuleInterop is still needed by ts-jest
  • Loading branch information
Andarist authored Jul 5, 2024
1 parent 0f9764b commit 2ad92e5
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 20 deletions.
2 changes: 1 addition & 1 deletion packages/replayio/src/utils/async/logAsyncOperation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import {
} from "@replay-cli/shared/async/createDeferred";
import { statusFailed, statusPending, statusSuccess } from "@replay-cli/shared/theme";
import { dots } from "cli-spinners";
import { logUpdate } from "../../../../shared/src/logUpdate";
import { disableAnimatedLog } from "../../config";
import { logUpdate } from "@replay-cli/shared/logUpdate";

export type LogProgressOptions = { delayBeforeLoggingMs?: number };

Expand Down
4 changes: 2 additions & 2 deletions packages/replayio/src/utils/browser/launchBrowser.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { getReplayPath } from "@replay-cli/shared/getReplayPath";
import { logger } from "@replay-cli/shared/logger";
import { dim, stderrPrefix, stdoutPrefix } from "@replay-cli/shared/theme";
import { spawnProcess } from "@replay-cli/shared/spawnProcess";
import { dim } from "@replay-cli/shared/theme";
import { ensureDirSync, existsSync } from "fs-extra";
import { join } from "path";
import { spawnProcess } from "../../../../shared/src/spawnProcess";
import { runtimeMetadata, runtimePath } from "../installation/config";
import { prompt } from "../prompt/prompt";
import { getBrowserPath } from "./getBrowserPath";
Expand Down
4 changes: 1 addition & 3 deletions packages/shared/src/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ import LokiTransport from "winston-loki";
import { AuthInfo } from "./graphql/fetchAuthInfoFromGraphQL";
import { getDeviceId } from "./getDeviceId";
import { randomUUID } from "crypto";

// Does not work with import. Only works with require().
const StackUtils = require("stack-utils");
import StackUtils from "stack-utils";

const GRAFANA_USER = "909360";
const GRAFANA_PUBLIC_TOKEN =
Expand Down
4 changes: 4 additions & 0 deletions scripts/pkg-build/src/bin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ async function buildPkg(pkg: Package, packagesByName: Map<string, Package>) {
)
);

const bundledIds = new Set<string>();
const fsMap = new Map<string, string>();
const resolvedBundledIds = new Map<string, string>();

Expand All @@ -55,11 +56,14 @@ async function buildPkg(pkg: Package, packagesByName: Map<string, Package>) {
input,
plugins: [
resolveErrors({
bundledIds,
isExternal,
packagesByName,
pkg,
}),
json(),
bundledDependencies({
bundledIds,
fsMap,
isBundledDependency,
packagesByName,
Expand Down
4 changes: 4 additions & 0 deletions scripts/pkg-build/src/plugins/bundledDependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,14 @@ export function getPotentialBundledSourceId(
}

export function bundledDependencies({
bundledIds,
fsMap,
isBundledDependency,
packagesByName,
resolvedBundledIds,
rootDir,
}: {
bundledIds: Set<string>;
fsMap: Map<string, string>;
isBundledDependency: PackagePredicate;
packagesByName: Map<string, Package>;
Expand Down Expand Up @@ -171,6 +173,7 @@ export function bundledDependencies({
const relativeInSource = path.relative(path.dirname(importerSourceId), resolved.id);
const bundledLocalId = path.join(path.dirname(importer!), relativeInSource);

bundledIds.add(resolved.id);
resolvedBundledIds.set(bundledLocalId, resolved.id);

return bundledLocalId;
Expand Down Expand Up @@ -208,6 +211,7 @@ export function bundledDependencies({
""
)}`;

bundledIds.add(resolved.id);
resolvedBundledIds.set(bundledLocalId, resolved.id);

return bundledLocalId;
Expand Down
36 changes: 25 additions & 11 deletions scripts/pkg-build/src/plugins/resolveErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,34 @@ import normalizePath from "normalize-path";
import { Plugin } from "rollup";
import { PackagePredicate } from "../makePackagePredicate";

// bundled dependencies complicated this a lot and currently this might not serve its original purpose
// the rules inside this could use some improvement
// ideally the rules that check if something starts with / would be rewritten
function getPackageDir(id: string, packageDirs: Set<string>) {
let dirname = path.dirname(id);
while (dirname !== "/") {
if (packageDirs.has(dirname)) {
return dirname;
}
dirname = path.dirname(dirname);
}
return null;
}

export function resolveErrors({
bundledIds,
isExternal,
packagesByName,
pkg,
}: {
bundledIds: Set<string>;
isExternal: PackagePredicate;
packagesByName: Map<string, Package>;
pkg: Package;
}): Plugin {
const packageDirs = new Set([...packagesByName.values()].map(p => p.dir));
return {
name: "resolve-errors",
// based on https://github.com/preconstruct/preconstruct/blob/5113f84397990ff1381b644da9f6bb2410064cf8/packages/cli/src/rollup-plugins/resolve.ts
async resolveId(source, importer, options) {
if (
options.isEntry ||
source.startsWith("\0") ||
options.custom?.["bundled-dependencies"] ||
importer?.startsWith("/")
) {
if (options.isEntry || source.startsWith("\0") || options.custom?.["bundled-dependencies"]) {
return;
}
if (!source.startsWith(".") && !source.startsWith("/") && !isExternal(source)) {
Expand All @@ -49,13 +57,19 @@ export function resolveErrors({
);
}

if (resolved.id.startsWith("\0") || resolved.id.startsWith(pkg.dir)) {
if (resolved.id.startsWith("\0") || bundledIds.has(source)) {
return resolved;
}

const importerPkgDir = importer ? getPackageDir(importer, packageDirs) : pkg.dir;

if (importerPkgDir && resolved.id.startsWith(importerPkgDir)) {
return resolved;
}

throw new Error(
`all relative imports in a package should only import modules inside of their package directory but ${
importer ? `"${normalizePath(path.relative(pkg.relativeDir, importer))}"` : "a module"
importer ?? "a module"
} is importing "${source}"`
);
},
Expand Down
4 changes: 1 addition & 3 deletions scripts/tsconfig/base.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
"forceConsistentCasingInFileNames": true,
"useDefineForClassFields": true,
"rootDir": "${configDir}/src",
"outDir": "${configDir}/dist",
// TS reports TS5074 without this explicit setting
"tsBuildInfoFile": "${configDir}/tsconfig.tsbuildinfo"
"outDir": "${configDir}/dist"
}
}

0 comments on commit 2ad92e5

Please sign in to comment.