Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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: 2 additions & 0 deletions packages/cli/src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { existsSync } from 'node:fs'
import fsExtra from 'fs-extra'
import path from 'node:path'
import { fileURLToPath, pathToFileURL } from 'node:url'
import { withNodeModulesBins } from '../utils/binPaths'
import { getPreferredPackageManager } from '../utils/commands'
import { checkDeprecatedSecretFiles } from '../utils/deprecations'
import { getBasePath, withBasePath } from '../utils/directory'
Expand Down Expand Up @@ -94,6 +95,7 @@ export default class Build extends Command {
shell: true,
cwd: tmpDir,
stdio: 'inherit',
env: withNodeModulesBins(tmpDir),
})

if (scriptResult.status && scriptResult.status !== 0) {
Expand Down
6 changes: 4 additions & 2 deletions packages/cli/src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import dotenv from 'dotenv'
import { cpSync, existsSync, readFileSync } from 'node:fs'
import path from 'node:path'
import { fileURLToPath } from 'node:url'
import { withNodeModulesBins } from '../utils/binPaths'
import { getPreferredPackageManager } from '../utils/commands'
import { checkDeprecatedSecretFiles } from '../utils/deprecations'
import { getBasePath, withBasePath } from '../utils/directory'
Expand Down Expand Up @@ -68,6 +69,7 @@ async function storeDev(
errorMessage: `The "predev" step ("${packageManager} predev") failed inside the ".faststore" directory. See the error output below for details.`,
throws: 'error',
cwd: tmpDir,
env: withNodeModulesBins(tmpDir),
})

const { success } = copyGenerated(
Expand All @@ -89,10 +91,10 @@ async function storeDev(
cwd: tmpDir,
signal: devAbortController.signal,
stdio: ['inherit', 'pipe', 'inherit'],
env: {
env: withNodeModulesBins(tmpDir, {
...process.env,
...envVars,
},
}),
})

let nextStdout = ''
Expand Down
88 changes: 88 additions & 0 deletions packages/cli/src/utils/binPaths.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import fs from 'node:fs'
import os from 'node:os'
import path from 'node:path'
import { afterAll, beforeAll, describe, expect, it } from 'vitest'

import { withNodeModulesBins } from './binPaths'

// mimics a hoisted monorepo: bins live at the workspace root, the store has its
// own node_modules, and `.faststore` has none
let workspaceRoot: string
let storeDir: string
let tmpDir: string

beforeAll(() => {
workspaceRoot = fs.realpathSync(
fs.mkdtempSync(path.join(os.tmpdir(), 'bin-paths-'))
)
storeDir = path.join(workspaceRoot, 'stores', 'my-store')
tmpDir = path.join(storeDir, '.faststore')

fs.mkdirSync(path.join(workspaceRoot, 'node_modules', '.bin'), {
recursive: true,
})
fs.mkdirSync(path.join(storeDir, 'node_modules', '.bin'), { recursive: true })
fs.mkdirSync(tmpDir, { recursive: true })
})

afterAll(() => {
fs.rmSync(workspaceRoot, { recursive: true, force: true })
})

describe('withNodeModulesBins', () => {
it('prepends the bin directories of every ancestor, nearest first', () => {
const { PATH } = withNodeModulesBins(tmpDir, { PATH: '/usr/bin' })
const entries = PATH?.split(path.delimiter) ?? []

expect(entries.slice(0, 2)).toEqual([
path.join(storeDir, 'node_modules', '.bin'),
path.join(workspaceRoot, 'node_modules', '.bin'),
])
expect(entries.at(-1)).toBe('/usr/bin')
})

it('skips ancestors without a node_modules/.bin', () => {
const { PATH } = withNodeModulesBins(tmpDir, { PATH: '/usr/bin' })

expect(PATH).not.toContain(
path.join(workspaceRoot, 'stores', 'node_modules', '.bin')
)
})

it('keeps the other environment variables untouched', () => {
const env = withNodeModulesBins(tmpDir, {
PATH: '/usr/bin',
VTEX_ACCOUNT: 'storeframework',
})

expect(env.VTEX_ACCOUNT).toBe('storeframework')
})

it('reuses the existing PATH key casing, as used on Windows', () => {
const env = withNodeModulesBins(tmpDir, { Path: 'C:\\Windows\\system32' })

Check notice on line 62 in packages/cli/src/utils/binPaths.test.ts

View check run for this annotation

Sonar - Workflows / SonarQube Code Analysis

packages/cli/src/utils/binPaths.test.ts#L62

`String.raw` should be used to avoid escaping `\`.

expect(env.PATH).toBeUndefined()
expect(env.Path).toContain(path.join(storeDir, 'node_modules', '.bin'))
expect(env.Path).toContain('C:\\Windows\\system32')

Check notice on line 66 in packages/cli/src/utils/binPaths.test.ts

View check run for this annotation

Sonar - Workflows / SonarQube Code Analysis

packages/cli/src/utils/binPaths.test.ts#L66

`String.raw` should be used to avoid escaping `\`.
})

it('does not duplicate a bin directory already present in PATH', () => {
const storeBinDir = path.join(storeDir, 'node_modules', '.bin')
const { PATH } = withNodeModulesBins(tmpDir, {
PATH: [storeBinDir, '/usr/bin'].join(path.delimiter),
})
const entries = PATH?.split(path.delimiter) ?? []

expect(entries.filter((entry) => entry === storeBinDir)).toHaveLength(1)
expect(entries[0]).toBe(path.join(workspaceRoot, 'node_modules', '.bin'))
})

it('does not drop the bin directories when PATH is empty', () => {
const { PATH } = withNodeModulesBins(tmpDir, {})

expect(PATH?.split(path.delimiter).slice(0, 2)).toEqual([
path.join(storeDir, 'node_modules', '.bin'),
path.join(workspaceRoot, 'node_modules', '.bin'),
])
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
})
64 changes: 64 additions & 0 deletions packages/cli/src/utils/binPaths.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import fsExtra from 'fs-extra'
import path from 'node:path'

const { existsSync } = fsExtra

function collectBinDirs(fromDir: string) {
const binDirs: string[] = []
let currentDir = path.resolve(fromDir)

while (true) {
const binDir = path.join(currentDir, 'node_modules', '.bin')

if (existsSync(binDir)) {
binDirs.push(binDir)
}

const parentDir = path.dirname(currentDir)

if (parentDir === currentDir) {
return binDirs
}

currentDir = parentDir
}
}

// Environment variables are case-insensitive on Windows, where the key is
// usually `Path`. Reusing the existing key avoids handing the child process
// both `Path` and `PATH`.
function getPathKey(env: NodeJS.ProcessEnv) {
return Object.keys(env).find((key) => key.toUpperCase() === 'PATH') ?? 'PATH'
}

/**
* Prepends the `node_modules/.bin` of `fromDir` and of every ancestor
* directory to `PATH`.
*
* The generated `.faststore` package has no `node_modules` of its own, so the
* binaries its scripts call (`na`, `next`) live higher up the tree: in the
* store root or, on hoisted monorepos, the workspace root.
*/
export function withNodeModulesBins(
fromDir: string,
env: NodeJS.ProcessEnv = process.env
): NodeJS.ProcessEnv {
const binDirs = collectBinDirs(fromDir)

if (binDirs.length === 0) {
return { ...env }
}

const pathKey = getPathKey(env)
const currentEntries = env[pathKey]?.split(path.delimiter) ?? []
// Package managers already put some of these directories in PATH when they
// run a script, so skip the ones that are there.
const missingBinDirs = binDirs.filter(
(binDir) => !currentEntries.includes(binDir)
)

return {
...env,
[pathKey]: [...missingBinDirs, ...currentEntries].join(path.delimiter),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
}
3 changes: 3 additions & 0 deletions packages/cli/src/utils/runCommandSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,14 @@ export const runCommandSync = ({
errorMessage,
throws,
cwd,
env,
interactive = false,
}: {
cmd: string
errorMessage: string
throws: 'warning' | 'error'
cwd?: string
env?: NodeJS.ProcessEnv
interactive?: boolean
}) => {
try {
Expand All @@ -70,6 +72,7 @@ export const runCommandSync = ({
const res = execSync(interactive ? cmd : `${cmd} 2>&1`, {
stdio: interactive ? 'inherit' : 'pipe',
cwd,
env,
})
logger.log(`[STATUS] ${res?.toString() ?? 'Unknown'}`)
logger.log(`[FINISHED] ${cmd}`)
Expand Down
Loading