Skip to content

chore: drop command-line-args/usage and @swc/helpers from runtime deps #441

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.

Sign up for GitHub

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

Open
maorleger wants to merge 2 commits into
base: main
Choose a base branch
from
+367 −166
Open

chore: drop command-line-args/usage and @swc/helpers from runtime deps #441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4fc1621
chore: drop command-line-args/usage and @swc/helpers from runtime deps
maorleger Jun 4, 2026
7ca2aca
added missing comma
maorleger Jun 6, 2026
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
2 changes: 1 addition & 1 deletion bin/arrow2csv.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ const env = { ...process.env, TS_NODE_TRANSPILE_ONLY: `true` };

require('child_process').spawn(`node`, [
`--import`,
`@swc-node/register/esm-register`
`@swc-node/register/esm-register`,
arrow2csv, ...process.argv.slice(2)
], { cwd: here, env, stdio: `inherit` });
13 changes: 7 additions & 6 deletions bin/integration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import * as fs from 'node:fs';
import * as Path from 'node:path';
import { glob } from 'glob';
import { zip } from 'ix/iterable/zip';
import commandLineArgs from 'command-line-args';
import { parseCliArgs, formatUsage } from '../src/bin/cli.ts';
import { parseArrowJSON } from '../src/util/json.ts';

import {
Expand All @@ -35,7 +35,7 @@ import {
} from '../index.ts';

const { createElementComparator } = util;
const argv = commandLineArgs(cliOpts(), { partial: true });
const { values: argv, positionals } = parseCliArgs(cliOpts(), process.argv.slice(2));

const exists = async (p: string) => {
try {
Expand All @@ -47,9 +47,9 @@ const exists = async (p: string) => {

if (!argv.mode) { return print_usage(); }

const mode = argv.mode.toUpperCase();
let jsonPaths = [...(argv.json || [])];
let arrowPaths = [...(argv.arrow || [])];
const mode = (argv.mode as string).toUpperCase();
let jsonPaths = [...(argv.json as string[] | undefined ?? [])];
let arrowPaths = [...(argv.arrow as string[] | undefined ?? [])];

if (mode === 'VALIDATE' && jsonPaths.length === 0) {
[jsonPaths, arrowPaths] = await loadLocalJSONAndArrowPathsForDebugging(jsonPaths, arrowPaths);
Expand Down Expand Up @@ -104,7 +104,7 @@ function cliOpts() {
}

function print_usage() {
console.log(require('command-line-usage')([
console.log(formatUsage([
{
header: 'integration',
content: 'Script for running Arrow integration tests'
Expand All @@ -121,6 +121,7 @@ function print_usage() {
...cliOpts(),
{
name: 'help',
type: Boolean,
description: 'Print this usage guide.'
}
]
Expand Down
11 changes: 6 additions & 5 deletions bin/json-to-arrow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@

import * as fs from 'node:fs';
import * as Path from 'node:path';
import commandLineArgs from 'command-line-args';
import { parseCliArgs, formatUsage } from '../src/bin/cli.ts';
import { finished as eos } from 'node:stream/promises';
import { parseArrowJSON } from '../src/util/json.ts';
import { RecordBatchReader, RecordBatchFileWriter, RecordBatchStreamWriter } from '../index.ts';

const argv = commandLineArgs(cliOpts(), { partial: true });
const jsonPaths = [...(argv.json || [])];
const arrowPaths = [...(argv.arrow || [])];
const { values: argv } = parseCliArgs(cliOpts(), process.argv.slice(2));
const jsonPaths = [...(argv.json as string[] | undefined ?? [])];
const arrowPaths = [...(argv.arrow as string[] | undefined ?? [])];

(async () => {

Expand Down Expand Up @@ -81,7 +81,7 @@ function cliOpts() {
}

function print_usage() {
console.log(require('command-line-usage')([
console.log(formatUsage([
{
header: 'json-to-arrow',
content: 'Script for converting a JSON Arrow file to a binary Arrow file'
Expand All @@ -98,6 +98,7 @@ function print_usage() {
...cliOpts(),
{
name: 'help',
type: Boolean,
description: 'Print this usage guide.'
}
]
Expand Down
54 changes: 43 additions & 11 deletions gulp/argv.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,50 @@
// specific language governing permissions and limitations
// under the License.

import args from 'command-line-args';
export const argv = args([
{ name: `all`, type: Boolean },
{ name: 'verbose', alias: `v`, type: Boolean },
{ name: `target`, type: String, defaultValue: `` },
{ name: `module`, type: String, defaultValue: `` },
{ name: `coverage`, type: Boolean, defaultValue: false },
{ name: `tests`, type: String, multiple: true, defaultValue: [`test/unit/`] },
{ name: `targets`, alias: `t`, type: String, multiple: true, defaultValue: [] },
{ name: `modules`, alias: `m`, type: String, multiple: true, defaultValue: [] },
], { partial: true });
import { parseArgs } from 'node:util';

const options = {
all: { type: 'boolean' },
verbose: { type: 'boolean', short: 'v' },
target: { type: 'string', default: '' },
module: { type: 'string', default: '' },
coverage: { type: 'boolean', default: false },
tests: { type: 'string', multiple: true, default: ['test/unit/'] },
targets: { type: 'string', short: 't', multiple: true, default: [] },
modules: { type: 'string', short: 'm', multiple: true, default: [] },
};

const { values, tokens } = parseArgs({
options,
args: process.argv.slice(2),
strict: false,
allowPositionals: true,
tokens: true,
});

// Rebuild the `_unknown` array that command-line-args' `{ partial: true }` mode
// used to produce. gulp/test-task.js forwards _unknown to Jest, so any flag or
// positional we don't recognise here (e.g. `--testNamePattern foo`, `--bail`)
// needs to round-trip through with its original raw form preserved. parseArgs
// only flags positionals as such; unknown options land in `values` and need to
// be reconstructed from the token stream.
const knownNames = new Set(Object.keys(options));
const unknown = [];
for (const tok of tokens) {
if (tok.kind === 'positional') {
unknown.push(tok.value);
} else if (tok.kind === 'option' && !knownNames.has(tok.name)) {
unknown.push(tok.rawName);
// `--foo=bar` parses to a single token with `inlineValue: true`; the
// separate-arg form `--foo bar` puts `bar` in a following positional
// token, which we'll pick up on the next loop iteration.
if (tok.inlineValue) unknown.push(tok.value);
delete values[tok.name];
}
Comment on lines +39 to +57

@trxcllnt trxcllnt Jun 8, 2026

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.

This seems relevant

@maorleger maorleger Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, it's one of those things that's technically accurate but (IIUC) largely meaningless in practice.

Here's my understanding, correct me if I'm wrong:

gulp/argv.js is dev-only, and _unknown only has one reader (test-task.js) - all that reader does is forward it to jest. Jest treats --foo=bar and --foo bar identically (I used --testNamePattern argument to test this), so the two forms give exactly the same outcome.

So I think a change here is not necessary, but I can change the code comment a bit to something like:

- // needs to round-trip through with its original raw form preserved.
+ // needs to round-trip through in a form Jest will accept

Which would more accurately describe the contract

If you feel strongly that I should fix this or if I misunderstood something, let me know, but it doesn't seem necessary. I'll defer to y'all as owners 🙇

}
values._unknown = unknown;

export const argv = values;
export const { targets, modules } = argv;

if (argv.target === `src`) {
Expand Down
Loading
Loading