Skip to content

Allow other angular builders instead of default #717

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

Closed
muuvmuuv opened this issue Jul 8, 2024 · 7 comments
Closed

Allow other angular builders instead of default #717

muuvmuuv opened this issue Jul 8, 2024 · 7 comments
Labels
feature request Feature request

Comments

@muuvmuuv
Copy link

muuvmuuv commented Jul 8, 2024

Hey, I saw that Angular is supported built-in, but we use a different builder (to customize esbuild), would love to see an option (maybe dot-path) to customize Angular main file.
We use "@angular-builders/custom-esbuild:application".
Also since Angular latest release "main" is deprecated in favor of "browser" with application builder. So any configuration would make this more future proof.

@muuvmuuv muuvmuuv added the feature request Feature request label Jul 8, 2024
@webpro
Copy link
Member

webpro commented Jul 8, 2024

Can you give more details, like an actual example of relevant configurationm, and what you'd expect from Knip here? And what's the output you're seeing now?

@muuvmuuv
Copy link
Author

muuvmuuv commented Jul 8, 2024

Here it says https://knip.dev/explanations/plugins#angular:

This will result in src/main.ts being added as an entry file (and @angular-devkit/build-angular as a referenced dependency).

Which wont work with other builders. Setting it manually works partly (angular deps are marked as missing peers and others that are used in ts files):

{
	"$schema": "https://unpkg.com/knip@5/schema.json",
	"entry": ["src/main.ts"],
	"project": ["src/*.{js,ts}"]
}

A Angular built-in plugin should maybe look for "defaultProject" in angular.json (or pick the first app that appears) and pick the first builder option as "build" then looks for "main" or "browser" to get the entry file. And uses "sourceRoot" as "project" instead of "**/*.{js,ts}".

Here a part of our angular.json:

{
	"version": 1,
	"$schema": "./node_modules/@angular/cli/lib/config/schema.json",
	"projects": {
		"app": {
			"root": "./",
			"sourceRoot": "src",
			"projectType": "application",
			"prefix": "app",
			"architect": {
				"build": {
					"builder": "@angular-builders/custom-esbuild:application",
					"options": {
						"browser": "./src/main.ts",
						"index": "./src/index.html",
						"outputPath": "./www",
						"tsConfig": "./tsconfig.app.json",
						"inlineStyleLanguage": "scss",
						"crossOrigin": "anonymous",

Also CLI devDeps are marked as "unused" but I guess I have not yet found the relevant option to exclude them.

@webpro
Copy link
Member

webpro commented Jul 8, 2024

Which wont work with other builders.

The Angular plugin is quite generic and I think Knip detects @angular-builders/custom-esbuild as a dependency.

A Angular built-in plugin should maybe look for "defaultProject" in angular.json (or pick the first app that appears) and pick the first builder option as "build"

This should already happen, it iterates over all projects → architect targets.

The Angular plugin implementation is not that much code even: https://github.com/webpro-nl/knip/blob/main/packages/knip/src/plugins/angular/index.ts#L26-L38

then looks for "main" or "browser" to get the entry file.

That src/main.ts is a default entry point. Additional to main, Knip could add the browser field.

And uses "sourceRoot" as "project" instead of "**/*.{js,ts}".

That's a good RFC. For now you can use a manual project: "src/**/*.{js,ts}"

Would still be good to better understand what's not working well for you. A reproduction or incorrect output would be helpful.

@muuvmuuv
Copy link
Author

muuvmuuv commented Jul 8, 2024

Oh yes, I see. Great so far.

It works now, it was just hard to get it working with Angular. Without specifying project it also tried my ios- and android-build dirs to scan which produced a lot of errors. After adjusting the config it works. Just wanted to point out that it does not out of the box with current Angular plugin.

@webpro
Copy link
Member

webpro commented Jul 8, 2024

So your point is mostly that the sourceRoot could be respected, right? The challenge here is that Knip plugins do not have the ability to override the global project config.

Using project: ["src/**/*.ts"] over the default probably should be promoted better I guess :)

@muuvmuuv
Copy link
Author

muuvmuuv commented Jul 8, 2024

Yes mostly, and that it would require zero config to use with Angular.

@webpro
Copy link
Member

webpro commented Jul 9, 2024

Thanks!

@webpro webpro closed this as completed Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request
Projects
None yet
Development

No branches or pull requests

2 participants