Skip to content
Open
Show file tree
Hide file tree
Changes from all 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: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"@octokit/rest": "22.0.0",
"@shopify/eslint-plugin-cli": "file:packages/eslint-plugin-cli",
"@shopify/generate-docs": "1.2.0",
"@types/node": "18.19.70",
"@types/node": "22.19.17",
"@typescript-eslint/parser": "8.56.1",
"@vitest/coverage-istanbul": "^3.1.4",
"bugsnag-build-reporter": "^2.0.0",
Expand Down
1 change: 0 additions & 1 deletion packages/cli-kit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@
"color-json": "3.0.5",
"conf": "11.0.2",
"deepmerge": "4.3.1",
"dotenv": "16.4.7",
"env-paths": "3.0.0",
"execa": "7.2.0",
"fast-glob": "3.3.3",
Expand Down
11 changes: 7 additions & 4 deletions packages/cli-kit/src/public/node/dot-env.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {outputDebug, outputContent, outputToken} from './output.js'
import {AbortError} from './error.js'
import {fileExists, readFile, writeFile} from './fs.js'
import {parse} from 'dotenv'
import {parseEnv} from 'node:util'

/**
* This interface represents a .env file.
Expand All @@ -28,10 +28,13 @@ export async function readAndParseDotEnv(path: string): Promise<DotEnvFile> {
throw new AbortError(`The environment file at ${path} does not exist.`)
}
const content = await readFile(path)
return {
path,
variables: parse(content),
// parseEnv types values as `string | undefined`; drop any undefined values so the
// result truly matches Record<string, string> rather than casting the type away.
const variables: Record<string, string> = {}
for (const [key, value] of Object.entries(parseEnv(content))) {
if (value !== undefined) variables[key] = value
}
return {path, variables}
}

/**
Expand Down
3 changes: 1 addition & 2 deletions packages/e2e/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
"@iarna/toml": "2.2.5",
"@playwright/test": "^1.50.0",
"@shopify/toml-patch": "0.3.0",
"@types/node": "18.19.70",
"dotenv": "16.4.7",
"@types/node": "22.19.17",
"execa": "^7.2.0",
"node-pty": "^1.0.0",
"strip-ansi": "^7.1.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/e2e/playwright.config.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/* eslint-disable line-comment-position */
import {TEST_TIMEOUT} from './setup/constants.js'
import {config} from 'dotenv'
import {loadEnvFile} from './setup/env.js'
import {defineConfig} from '@playwright/test'

config()
loadEnvFile()

const isCI = Boolean(process.env.CI)

Expand Down
4 changes: 2 additions & 2 deletions packages/e2e/scripts/cleanup-apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@
* E2E_ORG_ID — Organization ID to scan for apps
*/

import {config} from 'dotenv'
import * as path from 'path'
import {fileURLToPath} from 'url'
import {chromium} from '@playwright/test'
import {loadEnvFile} from '../setup/env.js'
import {BROWSER_TIMEOUT} from '../setup/constants.js'
import {navigateToDashboard, refreshIfPageError, trackMainFrameStatus} from '../setup/browser.js'
import {deleteAppFromDevDashboard} from '../setup/app.js'
Expand All @@ -34,7 +34,7 @@ import type {Page} from '@playwright/test'
// Load .env from packages/e2e/ (not cwd) only if not already configured
const __dirname = path.dirname(fileURLToPath(import.meta.url))
if (!process.env.E2E_ACCOUNT_EMAIL || !process.env.E2E_ACCOUNT_PASSWORD || !process.env.E2E_ORG_ID) {
config({path: path.resolve(__dirname, '../.env')})
loadEnvFile(path.resolve(__dirname, '../.env'))
}

// ---------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions packages/e2e/scripts/cleanup-stores.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
* E2E_ORG_ID — Organization ID to scan for stores
*/

import {config} from 'dotenv'
import * as path from 'path'
import {fileURLToPath} from 'url'
import {chromium} from '@playwright/test'
import {loadEnvFile} from '../setup/env.js'
import {BROWSER_TIMEOUT} from '../setup/constants.js'
import {deleteStore, dismissDevConsole, isStoreAppsEmpty} from '../setup/store.js'
import {refreshIfPageError, trackMainFrameStatus} from '../setup/browser.js'
Expand All @@ -32,7 +32,7 @@ import type {Page} from '@playwright/test'
// Load .env from packages/e2e/ (not cwd) only if not already configured
const __dirname = path.dirname(fileURLToPath(import.meta.url))
if (!process.env.E2E_ACCOUNT_EMAIL || !process.env.E2E_ACCOUNT_PASSWORD || !process.env.E2E_ORG_ID) {
config({path: path.resolve(__dirname, '../.env')})
loadEnvFile(path.resolve(__dirname, '../.env'))
}

// ---------------------------------------------------------------------------
Expand Down
17 changes: 17 additions & 0 deletions packages/e2e/setup/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {test as base} from '@playwright/test'
import * as path from 'path'
import * as fs from 'fs'
import {fileURLToPath} from 'url'
import {parseEnv} from 'node:util'

const __filename = fileURLToPath(import.meta.url)
const __dirname = path.dirname(__filename)
Expand Down Expand Up @@ -76,6 +77,22 @@ export const executables = {
createApp: path.resolve(__dirname, '../../../packages/create-app/bin/run.js'),
}

/**
* Loads environment variables from a `.env` file into `process.env`, mirroring
* dotenv's `config()`: a missing file is silently ignored and values already
* present in `process.env` are never overwritten.
*
* @param envPath - Path to the `.env` file. Defaults to `packages/e2e/.env`.
*/
export function loadEnvFile(envPath: string = path.resolve(__dirname, '../.env')): void {
if (!fs.existsSync(envPath)) return
for (const [key, value] of Object.entries(parseEnv(fs.readFileSync(envPath, 'utf8')))) {
if (value !== undefined && process.env[key] === undefined) {
process.env[key] = value
}
}
}

/**
* Creates an isolated temporary directory with XDG subdirectories and .npmrc.
* Returns the temp directory path and the env vars to pass to child processes.
Expand Down
28 changes: 27 additions & 1 deletion packages/eslint-plugin-cli/rules/no-inline-graphql.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
const path = require('path')
const fs = require('fs')
const crypto = require('crypto')
const {execFileSync} = require('child_process')
const debugEnabled = process.env.DEBUG && process.env.DEBUG.includes('eslint-plugin-cli')

/**
Expand Down Expand Up @@ -31,9 +32,34 @@ function hashFileSync(filePath, algorithm = 'sha256') {
return hash.digest('hex')
}

// The `knownFailures` keys are repo-root-relative, so we need the repo root to
// build the lookup key. Ask git rather than deriving it from this rule's
// __dirname: the package manager can place the plugin at different depths in
// node_modules (e.g. pnpm `file:` injection vs `link:` symlink, depending on
// peer resolution), so a fixed `..` offset from a shifting __dirname silently
// breaks the lookup and flags every grandfathered file. Memoized since the root
// is constant for a lint run.
let repoRoot
function findRepoRoot(filePath) {
if (repoRoot === undefined) {
try {
repoRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], {
cwd: path.dirname(filePath),
encoding: 'utf8',
}).trim()
} catch {
// git missing or not a repo (this plugin is published, so it can run
// outside our tree). Fall back to cwd so linting keeps working; the
// known-failures lookup just won't match, which only matters in this repo.
repoRoot = process.cwd()
}
}
return repoRoot
}
Comment thread
amcaplan marked this conversation as resolved.

function checkKnownFailuresIfShouldFail(context) {
const filePath = context.filename || context.getFilename()
const relativePath = path.relative(path.resolve(__dirname, '../../../../../../..'), filePath)
const relativePath = path.relative(findRepoRoot(filePath), filePath).split(path.sep).join('/')
const fileHash = hashFileSync(filePath)
const shouldFail = !knownFailures[relativePath] || knownFailures[relativePath] !== fileHash

Expand Down
2 changes: 1 addition & 1 deletion packages/theme/src/cli/utilities/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('errors', () => {

beforeEach(() => {
// Use a more reliable mock that throws an error
vi.spyOn(process, 'exit').mockImplementation((code?: number): never => {
vi.spyOn(process, 'exit').mockImplementation((code?: string | number | null): never => {
throw new Error(`Process exit with code ${code}`)
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ describe('pollRemoteJsonChanges', async () => {
vi.mocked(fetchChecksums).mockResolvedValue(updatedRemoteChecksums)

// Mock process.exit with a function that throws instead of actually exiting
vi.spyOn(process, 'exit').mockImplementation((code?: number): never => {
vi.spyOn(process, 'exit').mockImplementation((code?: string | number | null): never => {
throw new Error(`Process exit with code ${code}`)
})

Expand Down
Loading
Loading