Skip to content
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

Add Karma configuration from Angular plugin configuration #885

Merged
merged 1 commit into from
Dec 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/knip/fixtures/plugins/angular/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@
},
"devDependencies": {
"@angular/cli": "*",
"karma": "*",
"karma-chrome-launcher": "*",
"karma-coverage": "*",
"karma-jasmine": "*",
"karma-jasmine-html-reporter": "*",
"jasmine-core": "*",
"typescript": "*"
}
}
Empty file.
3 changes: 2 additions & 1 deletion packages/knip/fixtures/plugins/angular2/angular.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@
}
],
"styles": ["src/styles.css"],
"scripts": []
"scripts": [],
"karmaConfig": "another-karma.conf.js"
}
}
}
Expand Down
Empty file.
Empty file.
3 changes: 2 additions & 1 deletion packages/knip/fixtures/plugins/angular2/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"@angular/cli": "*",
"@angular/ssr": "*",
"@angular-builders/custom-esbuild": "*",
"@angular-devkit/build-angular": "*"
"@angular-devkit/build-angular": "*",
"karma": "*"
}
}
31 changes: 30 additions & 1 deletion packages/knip/src/plugins/angular/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import type { IsPluginEnabled, Plugin, ResolveConfig } from '../../types/config.
import { type Input, toConfig, toDependency, toEntry, toProductionEntry } from '../../util/input.js';
import { join } from '../../util/path.js';
import { hasDependency } from '../../util/plugin.js';
import type { AngularCLIWorkspaceConfiguration, WebpackBrowserSchemaForBuildFacade } from './types.js';
import * as karma from '../karma/helpers.js';
import type { AngularCLIWorkspaceConfiguration, KarmaTarget, WebpackBrowserSchemaForBuildFacade } from './types.js';

// https://angular.io/guide/workspace-config

Expand Down Expand Up @@ -61,6 +62,34 @@ const resolveConfig: ResolveConfig<AngularCLIWorkspaceConfiguration> = async (co
}
}
}
if (target.builder === '@angular-devkit/build-angular:karma' && opts) {
const karmaBuilderOptions = opts as KarmaTarget;
// https://github.com/angular/angular-cli/blob/19.0.6/packages/angular_devkit/build_angular/src/builders/karma/schema.json#L143
const testFilePatterns = karmaBuilderOptions.include ?? ['**/*.spec.ts'];
for (const testFilePattern of testFilePatterns) {
inputs.add(toEntry(testFilePattern));
}
// https://github.com/angular/angular-cli/blob/19.0.6/packages/angular_devkit/build_angular/src/builders/karma/schema.json#L146
const excludedTestFilePatterns = karmaBuilderOptions.exclude ?? [];
for (const excludedTestFilePattern of excludedTestFilePatterns) {
inputs.add(toEntry(`!${excludedTestFilePattern}`));
}
const karmaConfig = karmaBuilderOptions.karmaConfig;
if (!karmaConfig) {
// Hardcoded default Karma config from Angular builder
// https://github.com/angular/angular-cli/blob/19.0.6/packages/angular_devkit/build_angular/src/builders/karma/index.ts#L115
karma
.inputsFromPlugins(
['karma-jasmine', 'karma-chrome-launcher', 'karma-jasmine-html-reporter', 'karma-coverage'],
options.manifest.devDependencies
)
.forEach(inputs.add, inputs);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't like the .forEach(inputs.add, inputs). But doesn't work without the latest inputs argument to specify this. And a for loop (as forEach is forbidden by Biome linter) looks too much 🤷 happy to change if someone else is unhappy too

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this is fine really

karma.inputsFromFrameworks(['jasmine']).forEach(inputs.add, inputs);
}
if (karmaConfig && !karma.configFiles.includes(karmaConfig)) {
inputs.add(toConfig('karma', karmaConfig, options.configFilePath));
}
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/knip/src/plugins/angular/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2533,7 +2533,7 @@ interface ExtractI18NTarget1 {
/**
* Karma target options for Build Facade.
*/
interface KarmaTarget {
export interface KarmaTarget {
/**
* The name of the main entry-point file.
*/
Expand Down
51 changes: 51 additions & 0 deletions packages/knip/src/plugins/karma/helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { type Input, toDeferResolveEntry, toDevDependency } from '../../util/input.js';
import { isInternal } from '../../util/path.js';
import type { Config, ConfigOptions } from './types.js';

//👇 All but CoffeeScript ones. Low usage nowadays compared to the effort to implement support for those files
export const configFiles = ['karma.conf.js', 'karma.conf.ts', '.config/karma.conf.js', '.config/karma.conf.ts'];

export const inputsFromFrameworks = (frameworks: readonly string[]): readonly Input[] =>
frameworks.map(framework => {
return toDevDependency(framework === 'jasmine' ? 'jasmine-core' : framework);
});

export const inputsFromPlugins = (
plugins: ConfigOptions['plugins'],
devDependencies: Record<string, string> | undefined
): readonly Input[] => {
if (!plugins) {
const karmaPluginDevDeps = Object.keys(devDependencies ?? {}).filter(name => name.startsWith('karma-'));
return karmaPluginDevDeps.map(karmaPluginDevDep => toDevDependency(karmaPluginDevDep));
}
return plugins
.map(plugin => {
if (typeof plugin !== 'string') return;
return isInternal(plugin) ? toDeferResolveEntry(plugin) : toDevDependency(plugin);
})
.filter(input => !!input);
};

export type ConfigFile = (config: Config) => void;
export const loadConfig = (configFile: ConfigFile): ConfigOptions | undefined => {
if (typeof configFile !== 'function') return;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this in order to allow empty configuration files for test fixtures

const inMemoryConfig = new InMemoryConfig();
configFile(inMemoryConfig);
return inMemoryConfig.config ?? {};
};

/**
* Dummy configuration class with no default config options
* Relevant config options' defaults are empty, so that's good enough
* Real class: https://github.com/karma-runner/karma/blob/v6.4.4/lib/config.js#L275
*/
class InMemoryConfig implements Config {
config?: ConfigOptions;
/**
* Real method merges configurations with Lodash's `mergeWith`
* https://github.com/karma-runner/karma/blob/v6.4.4/lib/config.js#L343
*/
set(config: ConfigOptions) {
this.config = config;
}
}
59 changes: 8 additions & 51 deletions packages/knip/src/plugins/karma/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type { IsPluginEnabled, Plugin, ResolveConfig, ResolveEntryPaths } from '../../types/config.js';
import { type Input, toDeferResolveEntry, toDevDependency, toEntry } from '../../util/input.js';
import { isInternal, join } from '../../util/path.js';
import { type Input, toEntry } from '../../util/input.js';
import { join } from '../../util/path.js';
import { hasDependency } from '../../util/plugin.js';
import type { Config, ConfigOptions } from './types.js';
import { type ConfigFile, configFiles, inputsFromFrameworks, inputsFromPlugins, loadConfig } from './helpers.js';

// https://karma-runner.github.io/latest/config/configuration-file.html

Expand All @@ -12,50 +12,29 @@ const enablers = ['karma'];

const isEnabled: IsPluginEnabled = ({ dependencies }) => hasDependency(dependencies, enablers);

//👇 All but CoffeeScript ones. Low usage nowadays compared to the effort to implement support for those files
const config = ['karma.conf.js', 'karma.conf.ts', '.config/karma.conf.js', '.config/karma.conf.ts'];
const config = configFiles;

const entry: string[] = [];

type ConfigFile = (config: Config) => void;

const resolveConfig: ResolveConfig<ConfigFile> = async (localConfig, options) => {
const inputs = new Set<Input>();

const config = loadConfig(localConfig);
if (!config) return [];

if (config.frameworks) {
for (const framework of config.frameworks) {
inputs.add(toDevDependency(devDepForFramework(framework)));
}
}
if (config.plugins) {
for (const plugin of config.plugins) {
if (typeof plugin !== 'string') continue;
if (isInternal(plugin)) {
inputs.add(toDeferResolveEntry(plugin));
} else {
inputs.add(toDevDependency(plugin));
}
}
} else {
const karmaPluginDevDeps = Object.keys(options.manifest.devDependencies ?? {}).filter(name =>
name.startsWith('karma-')
);
for (const karmaPluginDevDep of karmaPluginDevDeps) {
inputs.add(toDevDependency(karmaPluginDevDep));
}
inputsFromFrameworks(config.frameworks).forEach(inputs.add, inputs);
}
inputsFromPlugins(config.plugins, options.manifest.devDependencies).forEach(inputs.add, inputs);

return Array.from(inputs);
};

const devDepForFramework = (framework: string): string => (framework === 'jasmine' ? 'jasmine-core' : framework);

const resolveEntryPaths: ResolveEntryPaths<ConfigFile> = (localConfig, options) => {
const inputs = new Set<Input>();

const config = loadConfig(localConfig);
if (!config) return [];

const basePath = config.basePath ?? '';
if (config.files) {
Expand All @@ -73,28 +52,6 @@ const resolveEntryPaths: ResolveEntryPaths<ConfigFile> = (localConfig, options)
return Array.from(inputs);
};

const loadConfig = (configFile: ConfigFile): ConfigOptions => {
const inMemoryConfig = new InMemoryConfig();
configFile(inMemoryConfig);
return inMemoryConfig.config ?? {};
};

/**
* Dummy configuration class with no default config options
* Relevant config options' defaults are empty, so that's good enough
* Real class: https://github.com/karma-runner/karma/blob/v6.4.4/lib/config.js#L275
*/
class InMemoryConfig implements Config {
config?: ConfigOptions;
/**
* Real method merges configurations with Lodash's `mergeWith`
* https://github.com/karma-runner/karma/blob/v6.4.4/lib/config.js#L343
*/
set(config: ConfigOptions) {
this.config = config;
}
}

export default {
title,
enablers,
Expand Down
6 changes: 4 additions & 2 deletions packages/knip/test/plugins/angular.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ test('Find dependencies with the Angular plugin', async () => {

assert(issues.unlisted['angular.json']['@angular-devkit/build-angular']);
assert(issues.unresolved['tsconfig.spec.json']['jasmine']);
assert(issues.devDependencies['package.json']['karma']);

assert.deepEqual(counters, {
...baseCounters,
devDependencies: 1,
unlisted: 1,
unresolved: 1,
processed: 1,
total: 1,
processed: 2,
total: 2,
});
});
9 changes: 6 additions & 3 deletions packages/knip/test/plugins/angular2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@ import baseCounters from '../helpers/baseCounters.js';
const cwd = resolve('fixtures/plugins/angular2');

test('Find dependencies with the Angular plugin (2)', async () => {
const { counters } = await main({
const { issues, counters } = await main({
...baseArguments,
cwd,
});

assert(issues.devDependencies['package.json']['karma']);

assert.deepEqual(counters, {
...baseCounters,
processed: 5,
total: 5,
devDependencies: 1,
processed: 7,
total: 7,
});
});
Loading