-
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?
Changes from all commits
dfeffaf
8b30d54
076d5dd
eb5e4f5
3b82e7f
29b3359
d7187e4
40e03fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "prerelease", | ||
"comment": "Update CLI to show static warning for old architecture in run-windows and interactive prompt for init-windows (#15029)", | ||
"packageName": "@react-native-windows/cli", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ import { | |
} from '../../utils/telemetryHelpers'; | ||
import {copyAndReplaceWithChangedCallback} from '../../generator-common'; | ||
import * as nameHelpers from '../../utils/nameHelpers'; | ||
import {promptForArchitectureChoice} from '../../utils/architecturePrompt'; | ||
import type {InitOptions} from './initWindowsOptions'; | ||
import {initOptions} from './initWindowsOptions'; | ||
|
||
|
@@ -167,9 +168,26 @@ export class InitWindows { | |
} | ||
|
||
if (this.options.template.startsWith('old')) { | ||
spinner.warn( | ||
`The legacy '${this.options.template}' template targets the React Native 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.`, | ||
const promptResult = await promptForArchitectureChoice( | ||
this.options.template, | ||
); | ||
|
||
if ( | ||
!promptResult.shouldContinueWithOldArch && | ||
!promptResult.userCancelled | ||
) { | ||
// User chose to switch to New Architecture | ||
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 commentThe 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. |
||
if (!this.templates.has(this.options.template.replace(/[\\]/g, '/'))) { | ||
throw new CodedError( | ||
'InvalidTemplateName', | ||
`Unable to find New Architecture template '${this.options.template}'.`, | ||
); | ||
} | ||
} | ||
} | ||
|
||
const templateConfig = this.templates.get(this.options.template)!; | ||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -208,9 +208,24 @@ async function runWindowsInternal( | |||||||||
|
||||||||||
// Warn about old architecture projects | ||||||||||
if (config.project.windows?.rnwConfig?.projectArch === 'old') { | ||||||||||
newWarn( | ||||||||||
'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 commentThe reason will be displayed to describe this comment to others. Learn more. The message refers to 'old architecture project' but should be more specific. Consider using 'This project' or 'Your project' for clarity, similar to the original warning message.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback
anupriya13 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
), | ||||||||||
); | ||||||||||
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.', | ||||||||||
), | ||||||||||
); | ||||||||||
console.log(); | ||||||||||
console.log( | ||||||||||
chalk.blue( | ||||||||||
'🔗 Learn more: https://microsoft.github.io/react-native-windows/docs/new-architecture', | ||||||||||
), | ||||||||||
); | ||||||||||
console.log(); | ||||||||||
} | ||||||||||
|
||||||||||
// Get the solution file | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
/** | ||
* Copyright (c) Microsoft Corporation. All rights reserved. | ||
* Licensed under the MIT License. | ||
* @format | ||
*/ | ||
|
||
import {promptForArchitectureChoice} from '../utils/architecturePrompt'; | ||
|
||
// Mock prompts module | ||
jest.mock('prompts', () => { | ||
return jest.fn(); | ||
}); | ||
|
||
import prompts from 'prompts'; | ||
const mockPrompts = prompts as jest.MockedFunction<typeof prompts>; | ||
|
||
describe('architecturePrompt', () => { | ||
beforeEach(() => { | ||
jest.useFakeTimers(); // ensure timers are controlled | ||
jest.clearAllMocks(); | ||
// Mock console methods to prevent output during tests | ||
jest.spyOn(console, 'log').mockImplementation(() => {}); | ||
}); | ||
|
||
afterEach(() => { | ||
// flush any pending timers from the timeout in promptForArchitectureChoice | ||
jest.runOnlyPendingTimers(); | ||
jest.useRealTimers(); | ||
jest.restoreAllMocks(); | ||
}); | ||
|
||
test('returns true when user chooses Y', async () => { | ||
mockPrompts.mockResolvedValue({choice: 'y'}); | ||
|
||
const result = await promptForArchitectureChoice('old/uwp-cpp-app'); | ||
|
||
expect(result.shouldContinueWithOldArch).toBe(true); | ||
expect(result.userCancelled).toBe(false); | ||
}); | ||
|
||
test('returns true when user chooses Y (uppercase)', async () => { | ||
mockPrompts.mockResolvedValue({choice: 'Y'}); | ||
|
||
const result = await promptForArchitectureChoice('old/uwp-cpp-app'); | ||
|
||
expect(result.shouldContinueWithOldArch).toBe(true); | ||
expect(result.userCancelled).toBe(false); | ||
}); | ||
|
||
test('returns false when user chooses N', async () => { | ||
mockPrompts.mockResolvedValue({choice: 'n'}); | ||
|
||
const result = await promptForArchitectureChoice('old/uwp-cpp-app'); | ||
|
||
expect(result.shouldContinueWithOldArch).toBe(false); | ||
expect(result.userCancelled).toBe(false); | ||
}); | ||
|
||
test('returns false when user chooses N (uppercase)', async () => { | ||
mockPrompts.mockResolvedValue({choice: 'N'}); | ||
|
||
const result = await promptForArchitectureChoice('old/uwp-cpp-app'); | ||
|
||
expect(result.shouldContinueWithOldArch).toBe(false); | ||
expect(result.userCancelled).toBe(false); | ||
}); | ||
|
||
test('returns true with userCancelled when user cancels', async () => { | ||
mockPrompts.mockResolvedValue({}); | ||
|
||
const result = await promptForArchitectureChoice('old/uwp-cpp-app'); | ||
|
||
expect(result.shouldContinueWithOldArch).toBe(true); | ||
expect(result.userCancelled).toBe(true); | ||
}); | ||
|
||
test('returns true with userCancelled when prompts throws cancellation error', async () => { | ||
mockPrompts.mockRejectedValue(new Error('User cancelled')); | ||
|
||
const result = await promptForArchitectureChoice('old/uwp-cpp-app'); | ||
|
||
expect(result.shouldContinueWithOldArch).toBe(true); | ||
expect(result.userCancelled).toBe(true); | ||
}); | ||
|
||
test('handles max retries for invalid input', async () => { | ||
// First two calls return invalid responses, third call succeeds | ||
mockPrompts | ||
.mockRejectedValueOnce(new Error('Invalid input')) | ||
.mockRejectedValueOnce(new Error('Invalid input')) | ||
.mockResolvedValueOnce({choice: 'y'}); | ||
|
||
const result = await promptForArchitectureChoice('old/uwp-cpp-app', 3); | ||
|
||
expect(result.shouldContinueWithOldArch).toBe(true); | ||
expect(result.userCancelled).toBe(false); | ||
expect(mockPrompts).toHaveBeenCalledTimes(3); | ||
}); | ||
|
||
test('returns true after max retries exceeded', async () => { | ||
// All calls return invalid responses | ||
mockPrompts.mockRejectedValue(new Error('Invalid input')); | ||
|
||
const result = await promptForArchitectureChoice('old/uwp-cpp-app', 2); | ||
|
||
expect(result.shouldContinueWithOldArch).toBe(true); | ||
expect(result.userCancelled).toBe(false); | ||
expect(mockPrompts).toHaveBeenCalledTimes(2); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,142 @@ | ||||||
/** | ||||||
* Copyright (c) Microsoft Corporation. | ||||||
* Licensed under the MIT License. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The copyright notice is inconsistent with other test files. It should be 'Copyright (c) Microsoft Corporation. All rights reserved.' to match the pattern used in the test file.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
* @format | ||||||
*/ | ||||||
|
||||||
import prompts from 'prompts'; | ||||||
import chalk from 'chalk'; | ||||||
|
||||||
export interface ArchitecturePromptResult { | ||||||
shouldContinueWithOldArch: boolean; | ||||||
userCancelled: boolean; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Shows an interactive prompt asking the user whether they want to continue with Old Architecture | ||||||
* or switch to New Architecture. | ||||||
* @param templateName The name of the old architecture template being used | ||||||
* @param maxRetries Maximum number of retries for invalid input (default: 3) | ||||||
* @returns Promise with the user's choice | ||||||
*/ | ||||||
export async function promptForArchitectureChoice( | ||||||
templateName: string, | ||||||
maxRetries: number = 3, | ||||||
): 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 commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
), | ||||||
); | ||||||
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 commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
), | ||||||
); | ||||||
console.log(); | ||||||
console.log( | ||||||
chalk.blue( | ||||||
'🔗 Learn more: https://microsoft.github.io/react-native-windows/docs/new-architecture', | ||||||
), | ||||||
); | ||||||
console.log(); | ||||||
|
||||||
let attempts = 0; | ||||||
|
||||||
while (attempts < maxRetries) { | ||||||
try { | ||||||
let timeoutId: NodeJS.Timeout | undefined; | ||||||
anupriya13 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
// Wrap prompts in a cancelable promise | ||||||
const userInputPromise = new Promise<{choice?: string}>( | ||||||
(resolve, reject) => { | ||||||
prompts( | ||||||
{ | ||||||
type: 'text', | ||||||
name: 'choice', | ||||||
message: | ||||||
'Would you like to continue using the Old Architecture? (Y/N)', | ||||||
validate: (value: string) => { | ||||||
const normalized = value.trim().toLowerCase(); | ||||||
return normalized === 'y' || normalized === 'n' | ||||||
? true | ||||||
: "Invalid input. Please enter 'Y' for Yes or 'N' for No."; | ||||||
}, | ||||||
}, | ||||||
{ | ||||||
onCancel: () => reject(new Error('User cancelled')), | ||||||
}, | ||||||
) | ||||||
.then(resolve) | ||||||
.catch(reject); | ||||||
}, | ||||||
); | ||||||
anupriya13 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
// Timeout fallback with clearTimeout support | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an important decision. Do not have it timeout. |
||||||
const timeoutPromise = new Promise<{choice?: string}>(resolve => { | ||||||
timeoutId = setTimeout(() => { | ||||||
console.log( | ||||||
chalk.yellow( | ||||||
'\n⏳ No input received in 3 seconds. Proceeding with Old Architecture by default.', | ||||||
), | ||||||
); | ||||||
resolve({choice: 'y'}); | ||||||
anupriya13 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
}, 3000); | ||||||
}); | ||||||
|
||||||
const response = await Promise.race([userInputPromise, timeoutPromise]); | ||||||
|
||||||
// ensures timeout callback never runs after user input | ||||||
if (timeoutId) clearTimeout(timeoutId); | ||||||
|
||||||
if (!response.choice) { | ||||||
// User cancelled or no input | ||||||
anupriya13 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return {shouldContinueWithOldArch: true, userCancelled: true}; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is |
||||||
} | ||||||
|
||||||
const normalizedChoice = response.choice.trim().toLowerCase(); | ||||||
|
||||||
if (normalizedChoice === 'y') { | ||||||
console.log( | ||||||
chalk.yellow( | ||||||
'Proceeding with Old Architecture. You can migrate later using our migration guide: https://microsoft.github.io/react-native-windows/docs/new-architecture', | ||||||
), | ||||||
); | ||||||
return {shouldContinueWithOldArch: true, userCancelled: false}; | ||||||
} else if (normalizedChoice === 'n') { | ||||||
console.log( | ||||||
chalk.green( | ||||||
'Great choice! Setting up the project with New Architecture support.', | ||||||
), | ||||||
); | ||||||
return {shouldContinueWithOldArch: false, userCancelled: false}; | ||||||
} | ||||||
} catch (error) { | ||||||
if ((error as Error).message === 'User cancelled') { | ||||||
console.log( | ||||||
chalk.yellow( | ||||||
'\nNo input received. Proceeding with Old Architecture by default. You can opt into the New Architecture later.', | ||||||
), | ||||||
); | ||||||
return {shouldContinueWithOldArch: true, userCancelled: true}; | ||||||
} | ||||||
|
||||||
// retry on invalid input | ||||||
attempts++; | ||||||
if (attempts < maxRetries) { | ||||||
console.log( | ||||||
chalk.red("Invalid input. Please enter 'Y' for Yes or 'N' for No."), | ||||||
); | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
// 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 commentThe 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. |
||||||
), | ||||||
); | ||||||
return {shouldContinueWithOldArch: true, userCancelled: false}; | ||||||
} |
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.
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.