refactor: minor code improvements (google#998)

continues google#988 google#984

Author: antongolub

src/core.ts

@@ -100,7 +100,7 @@ export interface Options {
 }
 
 // prettier-ignore
-export const defaults: Options = getZxDefaults({
+export const defaults: Options = resolveDefaults({
   [CWD]:          process.cwd(),
   [SYNC]:         false,
   verbose:        false,
@@ -122,38 +122,6 @@ export const defaults: Options = getZxDefaults({
   timeoutSignal:  SIGTERM,
 })
 
-export function getZxDefaults(
-  defs: Options,
-  prefix: string = 'ZX_',
-  env = process.env
-) {
-  const types: Record<PropertyKey, Array<'string' | 'boolean'>> = {
-    preferLocal: ['string', 'boolean'],
-    detached: ['boolean'],
-    verbose: ['boolean'],
-    quiet: ['boolean'],
-    timeout: ['string'],
-    timeoutSignal: ['string'],
-    prefix: ['string'],
-    postfix: ['string'],
-  }
-
-  const o = Object.entries(env).reduce<Record<string, string | boolean>>(
-    (m, [k, v]) => {
-      if (v && k.startsWith(prefix)) {
-        const _k = snakeToCamel(k.slice(prefix.length))
-        const _v = { true: true, false: false }[v.toLowerCase()] ?? v
-        if (_k in types && types[_k].some((type) => type === typeof _v)) {
-          m[_k] = _v
-        }
-      }
-      return m
-    },
-    {}
-  )
-  return Object.assign(defs, o)
-}
-
 // prettier-ignore
 export interface Shell<
   S = false,
@@ -587,34 +555,26 @@ export class ProcessPromise extends Promise<ProcessOutput> {
 
   // Async iterator API
   async *[Symbol.asyncIterator]() {
-    const _store = this._zurk!.store.stdout
-    let _stream
-
-    if (_store.length) {
-      _stream = VoidStream.from(_store)
-    } else {
-      _stream = this.stdout[Symbol.asyncIterator]
-        ? this.stdout
-        : VoidStream.from(this.stdout)
+    let last: string | undefined
+    const getLines = (chunk: Buffer | string) => {
+      const lines = ((last || '') + chunk.toString()).split('\n')
+      last = lines.pop()
+      return lines
     }
 
-    let buffer = ''
-
-    for await (const chunk of _stream) {
-      const chunkStr = chunk.toString()
-      buffer += chunkStr
-
-      let lines = buffer.split('\n')
-      buffer = lines.pop() || ''
-
-      for (const line of lines) {
-        yield line
-      }
+    for (const chunk of this._zurk!.store.stdout) {
+      const lines = getLines(chunk)
+      for (const line of lines) yield line
     }
 
-    if (buffer.length > 0) {
-      yield buffer
+    for await (const chunk of this.stdout[Symbol.asyncIterator]
+      ? this.stdout
+      : VoidStream.from(this.stdout)) {
+      const lines = getLines(chunk)
+      for (const line of lines) yield line
     }
+
+    if (last) yield last
   }
 
   // Stream-like API
@@ -968,3 +928,30 @@ const promisifyStream = <S extends Writable>(
         : promisifyStream(piped as Writable, from)
     },
   })
+
+export function resolveDefaults(
+  defs: Options,
+  prefix: string = 'ZX_',
+  env = process.env
+) {
+  const allowed = new Set([
+    'cwd',
+    'preferLocal',
+    'detached',
+    'verbose',
+    'quiet',
+    'timeout',
+    'timeoutSignal',
+    'prefix',
+    'postfix',
+  ])
+
+  return Object.entries(env).reduce<Options>((m, [k, v]) => {
+    if (v && k.startsWith(prefix)) {
+      const _k = snakeToCamel(k.slice(prefix.length))
+      const _v = { true: true, false: false }[v.toLowerCase()] ?? v
+      if (allowed.has(_k)) (m as any)[_k] = _v
+    }
+    return m
+  }, defs)
+}

test/core.test.js

@@ -19,44 +19,31 @@ import { basename } from 'node:path'
 import { WriteStream } from 'node:fs'
 import { Readable, Transform, Writable } from 'node:stream'
 import { Socket } from 'node:net'
-import { ProcessPromise, ProcessOutput, getZxDefaults } from '../build/index.js'
+import {
+  ProcessPromise,
+  ProcessOutput,
+  resolveDefaults,
+} from '../build/index.js'
 import '../build/globals.js'
 
 describe('core', () => {
-  describe('getZxDefaults', () => {
-    test('verbose rewrite', async () => {
-      const defaults = getZxDefaults({ verbose: false }, 'ZX_', {
+  describe('resolveDefaults()', () => {
+    test('overrides known (allowed) opts', async () => {
+      const defaults = resolveDefaults({ verbose: false }, 'ZX_', {
         ZX_VERBOSE: 'true',
+        ZX_PREFER_LOCAL: '/foo/bar/',
       })
       assert.equal(defaults.verbose, true)
+      assert.equal(defaults.preferLocal, '/foo/bar/')
     })
 
-    test('verbose ignore', async () => {
-      const defaults = getZxDefaults({ verbose: false }, 'ZX_', {
-        ZX_VERBOSE: 'true123',
-      })
-      assert.equal(defaults.verbose, false)
-    })
-
-    test('input ignored', async () => {
-      const defaults = getZxDefaults({}, 'ZX_', {
+    test('ignores unknown', async () => {
+      const defaults = resolveDefaults({}, 'ZX_', {
         ZX_INPUT: 'input',
+        ZX_FOO: 'test',
       })
       assert.equal(defaults.input, undefined)
-    })
-
-    test('preferLocal rewrite boolean', async () => {
-      const defaults = getZxDefaults({ preferLocal: false }, 'ZX_', {
-        ZX_PREFER_LOCAL: 'true',
-      })
-      assert.equal(defaults.preferLocal, true)
-    })
-
-    test('preferLocal rewrite string', async () => {
-      const defaults = getZxDefaults({ preferLocal: false }, 'ZX_', {
-        ZX_PREFER_LOCAL: 'true123',
-      })
-      assert.equal(defaults.preferLocal, 'true123')
+      assert.equal(defaults.foo, undefined)
     })
   })
 
@@ -759,7 +746,6 @@ describe('core', () => {
     describe('[Symbol.asyncIterator]', () => {
       it('should iterate over lines from stdout', async () => {
         const process = $`echo "Line1\nLine2\nLine3"`
-
         const lines = []
         for await (const line of process) {
           lines.push(line)
@@ -773,7 +759,6 @@ describe('core', () => {
 
       it('should handle partial lines correctly', async () => {
         const process = $`node -e "process.stdout.write('PartialLine1\\nLine2\\nPartial'); setTimeout(() => process.stdout.write('Line3\\n'), 100)"`
-
         const lines = []
         for await (const line of process) {
           lines.push(line)
@@ -795,7 +780,6 @@ describe('core', () => {
 
       it('should handle empty stdout', async () => {
         const process = $`echo -n ""`
-
         const lines = []
         for await (const line of process) {
           lines.push(line)
@@ -806,7 +790,6 @@ describe('core', () => {
 
       it('should handle single line without trailing newline', async () => {
         const process = $`echo -n "SingleLine"`
-
         const lines = []
         for await (const line of process) {
           lines.push(line)
@@ -821,27 +804,17 @@ describe('core', () => {
       })
 
       it('should yield all buffered and new chunks when iterated after a delay', async () => {
-        const process = $`sleep 0.1; echo Chunk1; sleep 0.2; echo Chunk2;`
-
-        const collectedChunks = []
-
-        await new Promise((resolve) => setTimeout(resolve, 400))
+        const process = $`sleep 0.1; echo Chunk1; sleep 0.1; echo Chunk2; sleep 0.2; echo Chunk3; sleep 0.1; echo Chunk4;`
+        const chunks = []
 
+        await new Promise((resolve) => setTimeout(resolve, 250))
         for await (const chunk of process) {
-          collectedChunks.push(chunk)
+          chunks.push(chunk)
         }
 
-        assert.equal(collectedChunks.length, 2, 'Should have received 2 chunks')
-        assert.equal(
-          collectedChunks[0],
-          'Chunk1',
-          'First chunk should be "Chunk1"'
-        )
-        assert.equal(
-          collectedChunks[1],
-          'Chunk2',
-          'Second chunk should be "Chunk2"'
-        )
+        assert.equal(chunks.length, 4, 'Should get all chunks')
+        assert.equal(chunks[0], 'Chunk1', 'First chunk should be "Chunk1"')
+        assert.equal(chunks[3], 'Chunk4', 'Second chunk should be "Chunk4"')
       })
     })