-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
CLI: Add skip onboarding, recommended/minimal config #30930
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings | Greptile
}); | ||
|
||
describe('promptInstallType', () => { | ||
const settings = new Settings(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Settings instance should be initialized in beforeEach to ensure clean state between tests
it('uses specific prompt options for React Native projects', async () => { | ||
prompts.inject(['recommended']); | ||
const result = await promptInstallType({ | ||
settings, | ||
projectType: ProjectType.REACT_NATIVE, | ||
}); | ||
|
||
expect(result).toBe('recommended'); | ||
expect(prompts).not.toHaveBeenCalled(); | ||
expect(vi.mocked(telemetry).mock.calls[0][1]).toMatchInlineSnapshot(` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Test description suggests prompting but expects prompts not to be called - either the test name or implementation needs adjustment
it('prompts user when interactive and yes option is not set', async () => { | ||
prompts.inject(['recommended']); | ||
const result = await promptInstallType({ settings }); | ||
expect(result).toBe('recommended'); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Missing telemetry assertion in this test case while other similar tests verify it
it('prompts user when interactive and yes option is not set', async () => { | |
prompts.inject(['recommended']); | |
const result = await promptInstallType({ settings }); | |
expect(result).toBe('recommended'); | |
}); | |
it('prompts user when interactive and yes option is not set', async () => { | |
prompts.inject(['recommended']); | |
const result = await promptInstallType({ settings }); | |
expect(result).toBe('recommended'); | |
expect(vi.mocked(telemetry).mock.calls[0][1]).toMatchInlineSnapshot({ | |
installType: 'recommended', | |
step: 'install-type' | |
}); | |
}); |
View your CI Pipeline Execution ↗ for commit 9aa8a85.
☁️ Nx Cloud last updated this comment at |
514886d
to
fbf5d2a
Compare
|
||
import { SavingGlobalSettingsFileError } from '../server-errors'; | ||
|
||
const DEFAULT_SETTINGS_PATH = join(homedir(), '.storybook', 'settings.json'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I foresee issues if we ever use findUp
for .storybook
in different parts of the CLI and end up finding this package as if it is a config dir. We could be aware of this and try to always limit findUp
to only go up until the gitRoot
CLI: Add skip onboarding, recommended/minimal config (cherry picked from commit dde78a0)
Closes N/A
What I did
Adds the following prompt to the CLI:
If the user wants help with onboarding, we'll install the "recommended" setup of dev/docs/test (skipping addon-test if unavailable).
If the user skips onboarding, we won't ask again, but we will prompt for what type of project they want to install:
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Run
STORYBOOK_TELEMETRY_DEBUG=1 $PROJECT_ROOT/code/lib/create-storybook/bin/index.cjs
in an empty directory.Documentation
🦋 Canary release
This pull request has been released as version
0.0.0-pr-30930-sha-9aa8a851
. Try it out in a new sandbox by runningnpx [email protected] sandbox
or in an existing project withnpx [email protected] upgrade
.More information
0.0.0-pr-30930-sha-9aa8a851
shilman/cli-new-users
9aa8a851
1746107414
)To request a new release of this pull request, mention the
@storybookjs/core
team.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=30930
Greptile Summary
This PR introduces a new onboarding experience in Storybook's CLI, allowing users to choose between recommended and minimal configurations while tracking preferences through a new Settings system.
code/core/src/cli/globalSettings.ts
for persistent user preferences with JSON storage and dot notation supportcode/lib/create-storybook/src/initiate.ts
for first-time users to opt into onboarding or choose configuration typesettingsCreatedAt
field in telemetry metadata to track new user decisionsnotify-telemetry.ts
for improved clarity