Skip to content

Commit

Permalink
fix(exec): only pass script args to script (#11957)
Browse files Browse the repository at this point in the history
  • Loading branch information
Tobbe authored Feb 27, 2025
1 parent fbaaa2d commit 11c88f0
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 0 deletions.
9 changes: 9 additions & 0 deletions .changesets/11957.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
- fix(exec): only pass script args to script (#11957) by @Tobbe

Remove all Redwood arguments, keeping only the script arguments.

This is potentially breaking for you.
The most likely scenario is that you're directly accessing the `args._` array
in your script with specific indicies.

See the PR description for more details.
56 changes: 56 additions & 0 deletions packages/cli/src/commands/__tests__/exec.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,27 @@
import path from 'node:path'

import { fs as memfs, vol } from 'memfs'
import { vi, afterEach, beforeEach, describe, it, expect } from 'vitest'

import { runScriptFunction } from '../../lib/exec'
import '../../lib/mockTelemetry'
import { handler } from '../execHandler'

vi.mock('@redwoodjs/babel-config', () => ({
getWebSideDefaultBabelConfig: () => ({
presets: [],
plugins: [],
}),
registerApiSideBabelHook: () => {},
}))

vi.mock('@redwoodjs/project-config', () => ({
getPaths: () => ({
api: { base: '', src: '' },
web: { base: '', src: '' },
scripts: path.join('redwood-app', 'scripts'),
}),
getConfig: () => ({}),
resolveFile: (path: string) => path,
}))

Expand All @@ -26,6 +39,13 @@ vi.mock('@redwoodjs/internal/dist/files', () => ({
},
}))

vi.mock('../../lib/exec', () => ({
runScriptFunction: vi.fn(),
}))

vi.mock('fs', () => ({ ...memfs, default: { ...memfs } }))
vi.mock('node:fs', () => ({ ...memfs, default: { ...memfs } }))

beforeEach(() => {
vi.spyOn(console, 'log').mockImplementation(() => {})
})
Expand All @@ -34,6 +54,42 @@ afterEach(() => {
vi.mocked(console).log.mockRestore()
})

describe('yarn rw exec', () => {
it('passes args on to the script', async () => {
vol.fromJSON({
'redwood.toml': '# redwood.toml',
[path.join('redwood-app', 'scripts', 'normalScript.ts')]: '// script',
})

// Running:
// `yarn rw exec normalScript positional1 --no-prisma positional2 --arg1=foo --arg2 bar`
const args = {
_: ['exec', 'positional1', 'positional2'],
prisma: false,
arg1: 'foo',
arg2: 'bar',
list: false,
l: false,
silent: false,
s: false,
$0: 'rw',
name: 'normalScript',
}
await handler(args)
expect(runScriptFunction).toHaveBeenCalledWith({
args: {
args: {
_: ['positional1', 'positional2'],
arg1: 'foo',
arg2: 'bar',
},
},
functionName: 'default',
path: path.join('redwood-app', 'scripts', 'normalScript.ts'),
})
})
})

describe('yarn rw exec --list', () => {
it('includes nested scripts', async () => {
await handler({ list: true })
Expand Down
28 changes: 28 additions & 0 deletions packages/cli/src/commands/execHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,34 @@ export const handler = async (args) => {
return
}

// The command the user is running is something like this:
//
// yarn rw exec scriptName arg1 arg2 --positional1=foo --positional2=bar
//
// Further up in the command chain we've parsed this with yargs. We asked
// yargs to parse the command `exec [name]`. So it plucked `scriptName` from
// the command and placed that in a named variable called `name`.
// And even further up the chain yargs has already eaten the `yarn` part and
// assigned 'rw' to `$0`
// So what yargs has left in args._ is ['exec', 'arg1', 'arg2'] (and it has
// also assigned 'foo' to `args.positional1` and 'bar' to `args.positional2`).
// 'exec', 'arg1' and 'arg2' are in `args._` because those are positional
// arguments we haven't given a name.
// `'exec'` is of no interest to the user, as its not meant to be an argument
// to their script. And so we remove it from the array.
scriptArgs._ = scriptArgs._.slice(1)

// 'rw' is not meant for the script's args, so delete that
delete scriptArgs.$0

// Other arguments that yargs adds are `prisma`, `list`, `l`, `silent` and
// `s`.
// We eat `prisma` and `list` above. So that leaves us with `l`, `s` and
// `silent` that we need to delete as well
delete scriptArgs.l
delete scriptArgs.s
delete scriptArgs.silent

const {
overrides: _overrides,
plugins: webPlugins,
Expand Down

0 comments on commit 11c88f0

Please sign in to comment.