-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Update CLI to show static warning for old architecture in run-windows and interactive prompt for init-windows #15038
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
base: main
Are you sure you want to change the base?
Conversation
… and interactive prompt for init-windows (#15029) * Initial plan * Initial plan: Update CLI prompt for architecture selections on old arch Co-authored-by: anupriya13 <[email protected]> * Implement interactive architecture prompt for old architecture templates Co-authored-by: anupriya13 <[email protected]> * Final implementation: Interactive architecture prompt with comprehensive testing Co-authored-by: anupriya13 <[email protected]> * Remove unintended changes from vnext/codegen directory Co-authored-by: anupriya13 <[email protected]> * Add prerelease change file for architecture prompt feature Co-authored-by: anupriya13 <[email protected]> * Replace interactive prompt with static warning for run-windows command Co-authored-by: anupriya13 <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: anupriya13 <[email protected]>
Co-Authored-By: Jon Thysell <[email protected]>
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.
Pull Request Overview
This PR adds improved CLI warnings and user prompts for the React Native Windows Old Architecture deprecation. The changes enhance the developer experience by providing more informative warnings and allowing users to interactively switch to the New Architecture during project initialization.
Key changes:
- Adds an interactive prompt in
init-windows
that allows users to switch from Old Architecture templates to New Architecture - Replaces the simple warning in
run-windows
with a more detailed, formatted deprecation notice - Includes comprehensive test coverage for the new prompt functionality
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
architecturePrompt.ts |
New utility module providing interactive prompts for architecture choice with timeout and retry logic |
architecturePrompt.test.ts |
Comprehensive test suite covering all prompt scenarios including user input, cancellation, and retry behavior |
runWindows.ts |
Updates old architecture warning with enhanced formatting and detailed messaging |
initWindows.ts |
Integrates architecture prompt to allow users to switch templates during initialization |
change/@react-native-windows-cli-df0889c3-e18b-4f66-82ef-3b55c9294c4b.json |
Change log entry for versioning |
packages/@react-native-windows/cli/src/commands/runWindows/runWindows.ts
Show resolved
Hide resolved
'This project is using the React Native (for Windows) Old Architecture, which will eventually be deprecated. See https://microsoft.github.io/react-native-windows/docs/new-architecture for details on switching to the New Architecture.', | ||
console.log( | ||
chalk.yellow( | ||
`⚠️ The 'old architecture project' is based on the React Native Old Architecture, which will eventually be deprecated in future releases.`, |
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.
`⚠️ The 'old architecture project' is based on the React Native Old Architecture, which will eventually be deprecated in future releases.`, | |
`⚠️ This project is using the React Native (for Windows) Old Architecture, which will eventually be deprecated.`, |
): Promise<ArchitecturePromptResult> { | ||
console.log( | ||
chalk.yellow( | ||
`⚠️ The '${templateName}' template is based on the React Native Old Architecture, which will eventually be deprecated in future releases.`, |
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.
`⚠️ The '${templateName}' template is based on the React Native Old Architecture, which will eventually be deprecated in future releases.`, | |
`⚠️ The '${templateName}' template is using the React Native (for Windows) Old Architecture, which will eventually be deprecated in future releases.`, |
console.log(); | ||
console.log( | ||
chalk.cyan( | ||
'💡 We recommend switching to the New Architecture to take advantage of improved performance, long-term support, and modern capabilities.', |
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.
'💡 We recommend switching to the New Architecture to take advantage of improved performance, long-term support, and modern capabilities.', | |
'💡 We recommend using a New Architecture template to take advantage of improved performance, long-term support, and modern capabilities.', |
}, | ||
); | ||
|
||
// Timeout fallback with clearTimeout support |
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.
This is an important decision. Do not have it timeout.
|
||
if (!response.choice) { | ||
// User cancelled or no input | ||
return {shouldContinueWithOldArch: true, userCancelled: true}; |
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.
What is userCanceled
? I don't see any option to cancel. The user must select yes or no - or they can Ctrl+C
to exit the command.
// Max retries reached | ||
console.log( | ||
chalk.yellow( | ||
`\nMax retries reached. Proceeding with Old Architecture by default. You can opt into the New Architecture later.`, |
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.
There's no need for max retry logic. The user either answers yes or no, and if they never do, there's no reason to just go forward.
spinner.info('Switching to New Architecture template (cpp-app)...'); | ||
this.options.template = 'cpp-app'; | ||
|
||
// Verify the new template exists |
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.
This isn't necessary in here since you've picked a hard-coded template.
// User chose to switch to New Architecture | ||
spinner.info('Switching to New Architecture template (cpp-app)...'); | ||
this.options.template = 'cpp-app'; | ||
|
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.
If you change the template, you need to think about how you update the telemetry. The telemetry has already recorded what the user initially selected and/or the default at the start of the command (in startTelemetrySession
).
We don't have any other examples of where we change the user's decision during a command. You will need to design/spec out how to capture this in the telemetry, so that you can differentiate what happened for the user. We already capture what the user initially selected, and what the default would have been. You have different options here.
- If you do nothing, you'll never know how many people changed their minds here.
- If you just overwrite what the user initially selected, you won't know that they initially chose and the prompt changed their mind
- Add a new field somewhere that captures that they changed their mind.
All of them have consequences to affecting the data that you need to make decisions. You'll have to look at the reporting and think it through, otherwise the data will be messed up and not useful.
" I'm not sure that this is strong enough. -- It sounds like paper will no longer work in RN 0.82. -- We should probably say that using the old architecture will only work through RNW 0.81, after which it will no longer be supported. Maybe something like: " |
Description
Type of Change
Why
What is the motivation for this change? Add a few sentences describing the context and overall goals of the pull request's commits.
Resolves #15027
What
What changes were made to the codebase to solve the bug, add the functionality, etc. that you specified above.
Adds a yes/no prompt in init-windows and if user wants new arch then will switch to that instead.
Screenshots
Add any relevant screen captures here from before or after your changes.
init-windows with New Arch
init-windows with Old Arch
Testing
If you added tests that prove your changes are effective or that your feature works, add a few sentences here detailing the added test scenarios.
Optional: Describe the tests that you ran locally to verify your changes.
Tested locally
init-windows.test.mp4
Changelog
Should this change be included in the release notes: yes
Add a brief summary of the change to use in the release notes for the next release.