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

Missing standalone components declared with button[custom-button] are not reported by the compiler #46351

Open
Lonli-Lokli opened this issue Jun 13, 2022 · 16 comments
Labels
area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime compiler: template type-checking core: directive matching cross-cutting: standalone Issues related to the NgModule-less world P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Milestone

Comments

@Lonli-Lokli
Copy link

Which @angular/* package(s) are the source of the bug?

core

Is this a regression?

No

Description

When standalone component is created with @Component({ selector: 'button[cv-button]' it will be not reported during compile time

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/angular-standalone-uzrsmn?file=src%2Fmain.ts

Please provide the exception or error you saw

No message is displayed in Console about missing info, uncomment 59 line to make component work

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 14.0.0
Node: 14.18.2
Package Manager: yarn 3.2.0
OS: win32 x64

Angular: 14.0.0
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router, service-worker

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1400.0
@angular-devkit/build-angular   14.0.0
@angular-devkit/core            14.0.0
@angular-devkit/schematics      14.0.0
@schematics/angular             14.0.0
rxjs                            7.5.5
typescript                      4.7.3

Anything else?

No response

@pkozlowski-opensource
Copy link
Member

Yes, this is the "blind spot" in our template syntax. Essentially we can't know, from the syntax, if <button cv-button> means:

  • button element with the cv-button directive;
  • button element with the cv-button attribute (ex. added for the styling purposes).

There is a longer discussion on the topic in #3425

@pkozlowski-opensource pkozlowski-opensource added area: core Issues related to the framework runtime core: directive matching cross-cutting: standalone Issues related to the NgModule-less world labels Jun 13, 2022
@ngbot ngbot bot added this to the needsTriage milestone Jun 13, 2022
@Lonli-Lokli
Copy link
Author

Lonli-Lokli commented Jun 13, 2022

@pkozlowski-opensource Thanks for clarification.

I am observing similar behaviour with sugar-style directives (*ngIf for example), is it the same reason?

Example - https://stackblitz.com/edit/angular-standalone-4tmeb9?file=src%2Fmain.ts,src%2Fapp%2Fimage.component.ts
Uncomment line 72 to make it work

EDIT:
Also with ()-output directives
Example https://stackblitz.com/edit/angular-standalone-mdcjmm?file=src%2Fmain.ts,src%2Fapp%2Fimage.component.ts
Uncomment linee 96 to make it work

@pkozlowski-opensource
Copy link
Member

Yes, directives matching on even handlers (ex. (foo)="doSth()") will suffer from the same problem as again - we can't say, from the syntax itself, if (foo) is meant to be an event or a directive. In short - there are situations were we are not sure if a user meant "a directive" or just an attribute / event handler.

The *ngIf situation is a bit more complex but sometimes it is equivalent to the attribute case (if you write <div *ngIf> it will de-sugar to <ng-template ngIf><div></div></ng-template>. We are, most probably, going to improve the situation here with some errors specific to the control-flow directives.

cc @AndrewKushnir

@Lonli-Lokli
Copy link
Author

Lonli-Lokli commented Jun 13, 2022

If it's not possible for compiler, maybe it make sense to pass this to developer?
Report such things always as warnings, but allow developer's customization as it's done with eslint/stylelint

preudo code from future .angularlint

"@angular-lint/directives": [
          "error",
          {
            "reportPattern": "cv-**"
          }
        ],

@alxhub
Copy link
Member

alxhub commented Jun 14, 2022

#46146 will be in 14.1 which adds this warning for ngIf & friends.

@jnizet
Copy link
Contributor

jnizet commented Jul 1, 2022

I was asked to copy and paste here the description of a duplicate issue I opened (#46661), so here it goes:

If a directive has an attribute selector and the attribute is used without square brackets, such as routerLink="/users", and we forget to import the directive, or the module defining the directive, then the compiler doesn't complain at all, and we end up with an unnoticed bug.

When using NgModules, such a bug happens rarely, because you rarely forget to import the router module (for example) in your main or feature module. But when using standalone components, it's much more frequent, because the import needs to be repeated on every component that needs the directive, and of course we tend to only import what's absolutely needed.

After migrating two relatively small applications to standalone components, I've introduced such a bug, especially with routerLink, at least twice.

I understand why the compiler doesn't complain, but this, IMHO, is a source of hard to detect bugs: I doubt most people have integration tests checking that each and every link in their components works as expected. And even at runtime, the bug goes unnoticed until you click on the link and realize it doesn't work.
Proposed solution

It would be nice to be able to activate some compiler option that would, for example, check that any non-standard HTML attribute used in a component (such as routerLink) matches with a known directive selector for that component.

Or to be able to provide a list of selectors or attributes that must be checked.

@joergbaier
Copy link

joergbaier commented Aug 31, 2022

+💯 huge bug source... I'd love to see a strict build option that complains about any "unknown attribute". This way we are sure that imports are not missing.

Rather than maintaining a list of all known html attributes (if that is the main concern/difficulty)... at least give us a "allowedAttributes: []" where we can specify which attributes are safe to ignore (do not require an import).

@joergbaier
Copy link

In short - there are situations were we are not sure if a user meant "a directive" or just an attribute / event handler.

To me this a pretty big design flaw 🤔

@JoostK JoostK added compiler: template type-checking area: compiler Issues related to `ngc`, Angular's template compiler labels Sep 7, 2022
@alxhub alxhub added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Nov 16, 2022
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 16, 2022
@jpduckwo
Copy link

I don't know if this is the same - I think might be another use case. We have lots of custom form control components to use inside mat-form-field. The compiler also doesn't detect these as being missed if they are in the template but not imported in the standalone component. Should I raise a new issue for this or is the underlying problem the same?

Here app-rich-text-form-control isn't imported but the compiler doesn't pick it up. Fails at runtime

  standalone: true,
  imports: [CommonModule, ...MATERIAL_COMMON],
  template: `
    <mat-form-field appearance="fill">
      <app-rich-text-form-control></app-rich-text-form-control>
    </mat-form-field>
    ...

@kyjus25
Copy link

kyjus25 commented Jun 19, 2023

+1 for major pain point with standalone components

@kyjus25
Copy link

kyjus25 commented Jun 21, 2023

Just wanted to mention for everyone here, Webstorm (specifically the EAP version is what I tried) DOES pick up these warnings in the editor. Might save you a headache or two after you convert everything to standalone to load up the 30 day Webstorm trial and comb through your HTML files

Screenshot 2023-06-21 at 11 39 53 AM

@softgandhi
Copy link

I am also feeling sad when migrated but missing the tool support.

thanks @kyjus25 for alternatives.
looking for the official angular plugin to help sort out missing directives and components.

@PowerKiKi
Copy link
Contributor

My suggestion would be to solve this with a new extended diagnosis. The new diagnosis would warns for unknown tags and unknown attributes. Each project can gradually configure the diagnosis with a list of allowed unknown tags and allowed unknown attributes. In addition users can opt-in for this rule to break the build by configuring the diagnostic as an error, to be really strict.

Of course this is not an ideal solution, but it should be relatively simple to implement. And it would be better than runtime errors, and so much better than runtime lack of functionality (as seen with routerLink).

@kyjus25
Copy link

kyjus25 commented Oct 19, 2023

@PowerKiKi I'm fine with not ideal as long as it provides some peace of mind. Nothing scarier than deploying code that may or may not be silently broken. To me this seems like top priority

@yhaiovyi-tripactions
Copy link

Since when though did it work for non-standalone components?

@Zarlex
Copy link

Zarlex commented Feb 22, 2024

Is there any progress on this? I tried using the angular extended diagnosis and hoped the error missingControlFlowDirective would be thrown at compile time by adding this to my tsconfig.json for cases were the import of a structural directive was forgotten.

"angularCompilerOptions": {
        ...
        "extendedDiagnostics": {
            "checks": {
                "missingControlFlowDirective": "error"
            },
            "defaultCategory": "error"
        }
    }

According to the docs this should report missing imports of structural directives as errors

This diagnostics ensures that a standalone component which uses known control flow directives (such as *ngIf, *ngFor, *ngSwitch) in a template, also imports those directives either individually or by importing the CommonModule. Alternatively, use Angular's built-in control flow.

However, this only works partially.

It works for structural directives that are applied on html elements or ng-container. But if the structural directive is applied on a custom component <app-my-component *ngIf="isAuthenticated"></app-my-component> it does not report an error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime compiler: template type-checking core: directive matching cross-cutting: standalone Issues related to the NgModule-less world P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests