Skip to content

fix: shell-args-parsing-bug#10637

Open
IzaakGough wants to merge 3 commits into
mainfrom
@invertase/fix-shell-arg-parsing-bug
Open

fix: shell-args-parsing-bug#10637
IzaakGough wants to merge 3 commits into
mainfrom
@invertase/fix-shell-arg-parsing-bug

Conversation

@IzaakGough

@IzaakGough IzaakGough commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #6446

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request extracts the shell script argument normalization logic into a reusable helper function normalizeShellScriptArgs in standalone/runtime.js, updates the shell script execution in standalone/runtime.js and standalone/firepit.js to use it, and adds corresponding unit tests. The feedback suggests refactoring normalizeShellScriptArgs to handle nullish arguments defensively and to return early when -c is not found, reducing unnecessary nesting in accordance with the repository style guide.

Comment thread standalone/runtime.js Outdated
Comment on lines +60 to +73
exports.normalizeShellScriptArgs = function(args) {
args = [...args];

const index = args.indexOf("-c");
if (index !== -1) {
args.splice(index, 1);

if (args[index] === "--") {
args.splice(index, 1);
}
}

return args;
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To adhere to the repository style guide regarding reducing nesting and handling edge cases early, we can return early if index === -1. This avoids wrapping the rest of the logic in an outer if block. Additionally, we should defensively handle cases where args might be nullish.

Suggested change
exports.normalizeShellScriptArgs = function(args) {
args = [...args];
const index = args.indexOf("-c");
if (index !== -1) {
args.splice(index, 1);
if (args[index] === "--") {
args.splice(index, 1);
}
}
return args;
};
exports.normalizeShellScriptArgs = function(args) {
args = [...(args || [])];
const index = args.indexOf("-c");
if (index === -1) {
return args;
}
args.splice(index, 1);
if (args[index] === "--") {
args.splice(index, 1);
}
return args;
};
References
  1. Reduce nesting as much as possible. Code should avoid unnecessarily deep nesting or long periods of nesting. Handle edge cases early and exit or fold them into the general case. (link)

@IzaakGough IzaakGough marked this pull request as ready for review June 15, 2026 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Next.js cloud function builds failing on first firebase deploy

2 participants