-
Notifications
You must be signed in to change notification settings - Fork 11.9k
fix(@schematics/angular): add browsers option during vitest browser provider ng-add #32865
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ import { | |
| } from '../utility/dependency'; | ||
| import { JSONFile } from '../utility/json-file'; | ||
| import { latestVersions } from '../utility/latest-versions'; | ||
| import { getWorkspace } from '../utility/workspace'; | ||
| import { getWorkspace, updateWorkspace } from '../utility/workspace'; | ||
| import { Builders } from '../utility/workspace-models'; | ||
| import { Schema as VitestBrowserOptions } from './schema'; | ||
|
|
||
|
|
@@ -89,8 +89,33 @@ export default function (options: VitestBrowserOptions): Rule { | |
| } | ||
| }; | ||
|
|
||
| // Determine the default browser based on the provider package | ||
| let defaultBrowser: string; | ||
| if (packageName === '@vitest/browser-webdriverio') { | ||
| defaultBrowser = 'chrome'; | ||
| } else { | ||
| // Playwright and preview both use 'chromium' as the default | ||
| defaultBrowser = 'chromium'; | ||
| } | ||
|
|
||
| // Update angular.json to add the browsers option to the test target | ||
| const updateAngularJsonRule = updateWorkspace((workspace) => { | ||
| const project = workspace.projects.get(options.project); | ||
| if (project) { | ||
| const testTarget = project.targets.get('test'); | ||
| if (testTarget) { | ||
| testTarget.options ??= {}; | ||
| const existingBrowsers = testTarget.options['browsers'] as string[] | undefined; | ||
| if (!existingBrowsers?.length) { | ||
| testTarget.options['browsers'] = [defaultBrowser]; | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+103
to
+113
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 nested if checks for project and testTarget are redundant because their existence is already validated at the start of the schematic (lines 31-43), where an exception is thrown if they are not found. You can simplify this code to improve readability, as explicit checks are redundant when upstream processes guarantee existence. const project = workspace.projects.get(options.project);
const testTarget = project?.targets.get('test');
if (testTarget) {
testTarget.options ??= {};
const existingBrowsers = testTarget.options['browsers'] as string[] | undefined;
if (!existingBrowsers?.length) {
testTarget.options['browsers'] = [defaultBrowser];
}
}References
|
||
| }); | ||
|
|
||
| return chain([ | ||
| updateTsConfigRule, | ||
| updateAngularJsonRule, | ||
| ...dependencies.map((name) => | ||
| addDependency(name, latestVersions[name], { | ||
| type: DependencyType.Dev, | ||
|
|
@@ -101,8 +126,7 @@ export default function (options: VitestBrowserOptions): Rule { | |
| (_, context) => { | ||
| context.logger.info( | ||
| 'Vitest browser testing support has been added. ' + | ||
| "To run tests in a browser, add a 'browsers' field to the 'test' target in 'angular.json', " + | ||
| "or use the '--browsers' command line option.", | ||
| `The test target has been configured with '${defaultBrowser}' as the default browser.`, | ||
| ); | ||
| }, | ||
| ]); | ||
|
|
||
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.
For better code clarity and to prevent accidental reassignment, you can declare defaultBrowser as a const and initialize it using a ternary operator.