Skip to content

Commit f31b6de

Browse files
authored
fix: address CLI regressions from dependency slimming (#1534)
The post-merge review of #1532 found regressions in the published CLI: - declare engines >=22 (fs.globSync raised the runtime floor silently; Node 20 users crashed at runtime with no install-time warning) - finish the glob removal: project-files.ts still imported it at runtime and only worked through transitive hoisting. getPublishableFiles now uses fs.globSync; the ignore-package filter stays the authoritative .dclignore gate, directories are stat-filtered (the old nodir), and dot-dirs/node_modules are pruned during the walk. Old and new produce identical file lists on a fixture covering the edge cases. Root scripts/tests with the same phantom import migrated the same way. - restore the stable 8000-based preview port search (tryListen(0) handed out a random ephemeral port on every run) - decide colors by FORCE_COLOR/NO_COLOR again: styleText's default validation consults stdout while all logger output goes to stderr - abort pending prompts on SIGINT: on Node <24 readline pauses stdin instead of rejecting, so Ctrl+C hung forever and onCancel never ran - pass the Windows 'start' command line verbatim: libuv's CRT-style quoting mangled the '""' title arg and the browser never opened - apply the protobufjs override in sdk-commands' own manifest: the root override never reached its standalone lockfile, which still shipped the vulnerable 7.2.4 that #1532 set out to remove (consumer installs still need a @dcl/protocol bump; see PR)
1 parent d3168c9 commit f31b6de

12 files changed

Lines changed: 234 additions & 422 deletions

File tree

packages/@dcl/sdk-commands/package-lock.json

Lines changed: 163 additions & 377 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@dcl/sdk-commands/package.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@
4242
"@types/qrcode": "^1.5.6",
4343
"@types/ws": "^8.5.4"
4444
},
45+
"engines": {
46+
"node": ">=22.0.0"
47+
},
48+
"overrides": {
49+
"protobufjs": "^7.6.3"
50+
},
4551
"files": [
4652
"dist",
4753
".dclrc",

packages/@dcl/sdk-commands/src/components/log.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,9 @@ export const writeToStderr = (...parameters: readonly unknown[]) => {
1919
// @see https://no-color.org
2020
// @see https://www.npmjs.com/package/chalk
2121
const useColor = process.env.FORCE_COLOR !== '0' && !process.env.NO_COLOR
22-
const paint =
23-
(format: Parameters<typeof styleText>[0]) =>
24-
(text: string | number) =>
25-
useColor ? styleText(format, String(text)) : String(text)
22+
// skip styleText's stream validation: it checks stdout, but the logger writes to stderr
23+
const paint = (format: Parameters<typeof styleText>[0]) => (text: string | number) =>
24+
useColor ? styleText(format, String(text), { validateStream: false }) : String(text)
2625

2726
export const colors = {
2827
bgBlack: paint('bgBlack'),

packages/@dcl/sdk-commands/src/logic/get-free-port.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import * as net from 'net'
22

3+
// search upward from 8000 (portfinder's contract) so the preview URL stays stable across runs
4+
const BASE_PORT = 8000
5+
const HIGHEST_PORT = 65535
6+
37
function tryListen(port: number): Promise<number> {
48
return new Promise((resolve, reject) => {
59
const server = net.createServer()
@@ -16,11 +20,14 @@ export async function getPort(port: number, failoverPort = 2044) {
1620
const resolvedPort = port && Number.isInteger(port) ? +port : 0
1721

1822
if (!resolvedPort) {
19-
try {
20-
return await tryListen(0)
21-
} catch (e) {
22-
return failoverPort
23+
for (let candidate = BASE_PORT; candidate <= HIGHEST_PORT; candidate++) {
24+
try {
25+
return await tryListen(candidate)
26+
} catch {
27+
// busy, try the next one
28+
}
2329
}
30+
return failoverPort
2431
}
2532

2633
return resolvedPort

packages/@dcl/sdk-commands/src/logic/open.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { spawn } from 'child_process'
1+
import { spawn, SpawnOptions } from 'child_process'
22
import { release } from 'os'
33

44
function isWsl(): boolean {
@@ -10,14 +10,25 @@ export default async function open(target: string): Promise<void> {
1010
if (protocol !== 'http:' && protocol !== 'https:') {
1111
throw new Error(`Refusing to open non-http(s) URL: ${target}`)
1212
}
13-
const [command, args] =
14-
process.platform === 'darwin'
15-
? ['open', [target]]
16-
: process.platform === 'win32' || isWsl()
17-
? // `start` treats the first quoted arg as a window title; `&` must be escaped for cmd
18-
[isWsl() ? 'cmd.exe' : 'cmd', ['/c', 'start', '""', target.replace(/&/g, '^&')]]
19-
: ['xdg-open', [target]]
20-
const child = spawn(command, args, { stdio: 'ignore', detached: true })
13+
const spawnOptions: SpawnOptions = { stdio: 'ignore', detached: true }
14+
let command: string
15+
let args: string[]
16+
if (process.platform === 'darwin') {
17+
command = 'open'
18+
args = [target]
19+
} else if (process.platform === 'win32') {
20+
// libuv's CRT-style quoting mangles the '""' title arg, so hand cmd the exact line
21+
command = 'cmd'
22+
args = ['/c', `start "" "${target}"`]
23+
spawnOptions.windowsVerbatimArguments = true
24+
} else if (isWsl()) {
25+
command = 'cmd.exe'
26+
args = ['/c', 'start', '""', target.replace(/&/g, '^&')]
27+
} else {
28+
command = 'xdg-open'
29+
args = [target]
30+
}
31+
const child = spawn(command, args, spawnOptions)
2132
// opening the browser is best-effort: without this handler a missing
2233
// xdg-open/open/cmd (headless, containers) crashes the CLI
2334
child.on('error', () => {})

packages/@dcl/sdk-commands/src/logic/project-files.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { ContentMapping } from '@dcl/schemas/dist/misc/content-mapping'
22
import { CliComponents } from '../components'
33
import { getDCLIgnorePatterns } from './dcl-ignore'
4-
import { sync as globSync } from 'glob'
4+
import { globSync, statSync, Dirent } from 'fs'
55
import ignore from 'ignore'
66
import i18next from 'i18next'
77
import os from 'os'
@@ -27,13 +27,15 @@ export async function getPublishableFiles(
2727
const ig = ignore().add(ignorePatterns)
2828
const allFiles = globSync('**/*', {
2929
cwd: projectRoot,
30-
absolute: false,
31-
dot: false,
32-
ignore: ignorePatterns,
33-
nodir: true
30+
// prune walking into trees ig.filter below always excludes anyway (perf only;
31+
// exclude receives a string on some Node versions and a Dirent on others)
32+
exclude: (entry: string | Dirent) => {
33+
const name = typeof entry === 'string' ? path.basename(entry) : entry.name
34+
return name.startsWith('.') || name === 'node_modules'
35+
}
3436
})
3537

36-
return ig.filter(allFiles)
38+
return ig.filter(allFiles).filter((file) => statSync(resolve(projectRoot, file)).isFile())
3739
}
3840

3941
/**

packages/@dcl/sdk-commands/src/logic/prompts.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,13 @@ export default async function prompts(
2222
options: Options = {}
2323
): Promise<Record<string, string | number | boolean>> {
2424
const rl = readline.createInterface({ input: process.stdin, output: process.stdout })
25+
// on Node <24 Ctrl+C only pauses stdin instead of rejecting rl.question; abort so onCancel runs
26+
const canceled = new AbortController()
27+
rl.on('SIGINT', () => canceled.abort())
2528
try {
2629
for (;;) {
2730
const suffix = question.type === 'confirm' ? (question.initial ? ' [Y/n] ' : ' [y/N] ') : ' '
28-
const answer = await rl.question(`${question.message}${suffix}`)
31+
const answer = await rl.question(`${question.message}${suffix}`, { signal: canceled.signal })
2932
if (question.type === 'confirm') {
3033
const value = answer === '' ? !!question.initial : /^y(es)?$/i.test(answer)
3134
return { [question.name]: value }

scripts/helpers.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import { exec } from 'child_process'
2-
import { sync as globSync } from 'glob'
32
import { resolve, relative } from 'path'
43
import { existsSync, readFileSync, writeFileSync, lstatSync, removeSync, copySync } from 'fs-extra'
5-
import { rmSync } from 'fs'
4+
import { rmSync, globSync } from 'fs'
65

76
/**
87
* @returns the resolved absolute path
@@ -51,10 +50,12 @@ export function itDeletesFolder(folder: string, cwd: string) {
5150
}
5251
export function itDeletesGlob(pattern: string, cwd: string) {
5352
it(`deletes ${pattern} in ${cwd}`, () => {
54-
globSync(pattern, { absolute: true, cwd }).forEach((file) => {
55-
console.log(`> deleting ${file}`)
56-
rmSync(file, { recursive: true, force: true })
57-
})
53+
globSync(pattern, { cwd })
54+
.map((file) => resolve(cwd, file))
55+
.forEach((file) => {
56+
console.log(`> deleting ${file}`)
57+
rmSync(file, { recursive: true, force: true })
58+
})
5859
})
5960
}
6061

test/ecs/add-entity-from-composite.spec.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
1-
import { readFileSync } from 'fs'
2-
import { glob } from 'glob'
1+
import { readFileSync, globSync } from 'fs'
32
import path from 'path'
43
import { Composite, Engine, Entity, IEngine } from '../../packages/@dcl/ecs/src'
54
import { components } from '../../packages/@dcl/ecs/src'
65

76
const COMPOSITE_BASE_PATH = 'test/ecs/composites'
87

98
function getJsonCompositeFrom(globPath: string, cwd: string): Composite.Resource[] {
10-
const compositeFileContent = glob.sync(globPath, { cwd }).map((item) => ({
9+
const compositeFileContent = globSync(globPath, { cwd }).map((item) => ({
1110
src: item,
1211
content: readFileSync(path.resolve(cwd, item)).toString()
1312
}))

test/ecs/composite.spec.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { existsSync, readFileSync, writeFileSync } from 'fs'
2-
import { glob } from 'glob'
1+
import { existsSync, readFileSync, writeFileSync, globSync } from 'fs'
32
import path from 'path'
43
import {
54
Composite,
@@ -23,7 +22,7 @@ const nonBinaryCompositeJsonPath = 'non-binary.composite'
2322
// ########
2423

2524
function getJsonCompositeFrom(globPath: string, cwd: string): Composite.Resource[] {
26-
const compositeFileContent = glob.sync(globPath, { cwd }).map((item) => ({
25+
const compositeFileContent = globSync(globPath, { cwd }).map((item) => ({
2726
src: item,
2827
content: readFileSync(path.resolve(cwd, item)).toString()
2928
}))
@@ -34,7 +33,7 @@ function getJsonCompositeFrom(globPath: string, cwd: string): Composite.Resource
3433
}
3534

3635
function getBinaryCompositeFrom(globPath: string, cwd: string) {
37-
const compositeFileContent = glob.sync(globPath, { cwd }).map((item) => ({
36+
const compositeFileContent = globSync(globPath, { cwd }).map((item) => ({
3837
src: item,
3938
content: readFileSync(path.resolve(cwd, item))
4039
}))
@@ -149,7 +148,7 @@ describe('composite instantiation system', () => {
149148
// actually instances nested child composites. Leaf composites (whose
150149
// definition carries no `composite::root` component) emit none.
151150
const hasNestedComposites = composite.composite.components.some(
152-
component => component.name === CompositeRootComponent.componentName
151+
(component) => component.name === CompositeRootComponent.componentName
153152
)
154153
if (hasNestedComposites) {
155154
expect(composites.length).toBeGreaterThan(0)

0 commit comments

Comments
 (0)