Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: better error message when providing a command builder to an expr #240

Merged
merged 1 commit into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion mod.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2108,12 +2108,24 @@ Deno.test("should support empty quoted string", async () => {
assertEquals(output, " test ");
});

Deno.test("esnure deprecated PathRef export still works", () => {
Deno.test("ensure deprecated PathRef export still works", () => {
const path = new PathRef("hello");
assert(path instanceof Path);
assert(path instanceof PathRef);
});

Deno.test("nice error message when not awaiting a CommandBuilder", async () => {
await assertRejects(
async () => {
const cmd = $`echo 1`;
return await $`echo ${cmd}`;
},
Error,
"Providing a command builder is not yet supported (https://github.com/dsherret/dax/issues/239). " +
"Await the command builder's text before using it in an expression (ex. await $`cmd`.text()).",
);
});

function ensurePromiseNotResolved(promise: Promise<unknown>) {
return new Promise<void>((resolve, reject) => {
promise.then(() => reject(new Error("Promise was resolved")));
Expand Down
10 changes: 5 additions & 5 deletions mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {
CommandBuilder,
escapeArg,
getRegisteredCommandNamesSymbol,
setCommandTextAndFdsSymbol,
setCommandTextStateSymbol,
template,
templateRaw,
} from "./src/command.ts";
Expand Down Expand Up @@ -632,8 +632,8 @@ function build$FromState<TExtras extends ExtrasObject = {}>(state: $State<TExtra
};
const result = Object.assign(
(strings: TemplateStringsArray, ...exprs: any[]) => {
const { text, streams } = template(strings, exprs);
return state.commandBuilder.getValue()[setCommandTextAndFdsSymbol](text, streams);
const textState = template(strings, exprs);
return state.commandBuilder.getValue()[setCommandTextStateSymbol](textState);
},
helperObject,
logDepthObj,
Expand Down Expand Up @@ -764,8 +764,8 @@ function build$FromState<TExtras extends ExtrasObject = {}>(state: $State<TExtra
return state.requestBuilder.url(url);
},
raw(strings: TemplateStringsArray, ...exprs: any[]) {
const { text, streams } = templateRaw(strings, exprs);
return state.commandBuilder.getValue()[setCommandTextAndFdsSymbol](text, streams);
const textState = templateRaw(strings, exprs);
return state.commandBuilder.getValue()[setCommandTextStateSymbol](textState);
},
withRetries<TReturn>(opts: RetryOptions<TReturn>): Promise<TReturn> {
return withRetries(result, state.errorLogger.getValue(), opts);
Expand Down
18 changes: 10 additions & 8 deletions src/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const builtInCommands = {
/** @internal */
export const getRegisteredCommandNamesSymbol: unique symbol = Symbol();
/** @internal */
export const setCommandTextAndFdsSymbol: unique symbol = Symbol();
export const setCommandTextStateSymbol: unique symbol = Symbol();

/**
* Underlying builder API for executing commands.
Expand Down Expand Up @@ -583,12 +583,9 @@ export class CommandBuilder implements PromiseLike<CommandResult> {
}

/** @internal */
[setCommandTextAndFdsSymbol](text: string, fds: StreamFds | undefined) {
[setCommandTextStateSymbol](textState: CommandBuilderStateCommand) {
return this.#newWithState((state) => {
state.command = {
text,
fds,
};
state.command = textState;
});
}
}
Expand Down Expand Up @@ -1240,7 +1237,7 @@ function templateInner(
strings: TemplateStringsArray,
exprs: any[],
escape: ((arg: string) => string) | undefined,
) {
): CommandBuilderStateCommand {
let nextStreamFd = 3;
let text = "";
let streams: StreamFds | undefined;
Expand Down Expand Up @@ -1380,7 +1377,7 @@ function templateInner(
}
return {
text,
streams,
fds: streams,
};

function handleReadableStream(createStream: () => ReadableStream) {
Expand Down Expand Up @@ -1440,6 +1437,11 @@ function templateLiteralExprToString(expr: any, escape: ((arg: string) => string
} else if (expr instanceof CommandResult) {
// remove last newline
result = expr.stdout.replace(/\r?\n$/, "");
} else if (expr instanceof CommandBuilder) {
throw new Error(
"Providing a command builder is not yet supported (https://github.com/dsherret/dax/issues/239). " +
"Await the command builder's text before using it in an expression (ex. await $`cmd`.text()).",
);
} else if (typeof expr === "object" && expr.toString === Object.prototype.toString) {
throw new Error("Provided object does not override `toString()`.");
} else {
Expand Down
Loading