Copilot commented on code in PR #441:
URL: https://github.com/apache/arrow-js/pull/441#discussion_r3367853604
##########
package.json:
##########
@@ -46,12 +46,7 @@
"jest.config.js"
],
"dependencies": {
- "@swc/helpers": "^0.5.11",
- "@types/command-line-args": "^5.2.3",
- "@types/command-line-usage": "^5.0.4",
"@types/node": "^25.2.0",
- "command-line-args": "^6.0.1",
- "command-line-usage": "^7.0.1",
"flatbuffers": "^25.1.24",
"json-with-bigint": "^3.5.3",
"tslib": "^2.6.2"
Review Comment:
`@swc/helpers` was moved from `dependencies` to `devDependencies`. If any
published JS output imports helpers at runtime (common with SWC output),
downstream consumers will fail with a missing module. If helpers are still
required at runtime, move `@swc/helpers` back to `dependencies`; otherwise
ensure the build fully inlines helpers (or otherwise removes runtime imports)
before publishing.
##########
gulp/argv.js:
##########
@@ -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];
+ }
Review Comment:
The comment says unknown args should round-trip with their original raw form
preserved, but for inline-value options (`--foo=bar`) the reconstruction
becomes two argv entries (`--foo`, `bar`) rather than the original single
token. If the intention is true raw preservation, consider reconstructing a
single string when `tok.inlineValue` is true (e.g. `--foo=bar`) instead of
pushing `tok.rawName` and `tok.value` separately.
##########
package.json:
##########
@@ -63,6 +58,7 @@
"@rollup/stream": "3.0.1",
"@swc-node/register": "1.11.1",
"@swc/core": "1.15.32",
+ "@swc/helpers": "^0.5.11",
"@types/benchmark": "2.1.5",
Review Comment:
`@swc/helpers` was moved from `dependencies` to `devDependencies`. If any
published JS output imports helpers at runtime (common with SWC output),
downstream consumers will fail with a missing module. If helpers are still
required at runtime, move `@swc/helpers` back to `dependencies`; otherwise
ensure the build fully inlines helpers (or otherwise removes runtime imports)
before publishing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
