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

Conversation

davidlj95
Copy link
Contributor

Angular allows running tests using Karma via the @angular-devkit/build_angular:karma builder. However, configuration in Angular CLI workspace (aka angular.json) using that builder to run tests using Karma isn't taken into account as Karma configuration. But this is something that comes built-in when creating a fresh Angular project. This PR aims to take into account that builder configuration for knip.

To do so:

  • Karma plugin has been refactored: so that common functionality shared by both plugins lives in helpers.ts. In the same fashion that XO uses some ESLint helpers. The code to load a configuration file has been moved there too to keep clean the index.ts file.
  • Includes and excludes specified as builder options are used as included/excluded non-production entry points. If unspecified, Angular CLI defaults are used. Which actually come from the JSON Schema. Will try and see if can the script can be updated to obtain those values too.
  • Karma config file specified as karmaConfig in builder options is used to:
    • Load default Karma config if not specified
    • Load an alternative Karma config file if specified, but not one that Karma plugin would take by default.

Copy link

pkg-pr-new bot commented Dec 20, 2024

Open in Stackblitz

npm i https://pkg.pr.new/knip@885

commit: ba45938


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

['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

@davidlj95 davidlj95 force-pushed the angular-plugin-karma-config branch from 6161030 to ba45938 Compare December 20, 2024 13:21
@webpro webpro merged commit e1ba447 into webpro-nl:main Dec 22, 2024
23 checks passed
@webpro
Copy link
Collaborator

webpro commented Dec 22, 2024

Great, plugins use each other all over the place and it's chaotic and beautiful :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants