search

NG-ZORRO / ng-zorro-antd

Angular UI Component Library based on Ant Design
https://ng.ant.design
MIT License
8.85k stars 3.9k forks source link

Tips for modifying standalone components #8229

Open HyperLife1119 opened 9 months ago

HyperLife1119 commented 9 months ago

Here are some tips for standalone component modifications:

Welcome to add more tips :)

HyperLife1119 commented 9 months ago

cc @ParsaArvanehPA @evgeniyefimov

ParsaArvanehPA commented 9 months ago

Thanks for the tips :)

I follow this procedure to ensure that no unnecessary module is imported: • I remove all imports in .module file • I add the standalone files in the import section of .module file • I then alt+enter on the missing classes and add them to the import section of each standalone component or directive. • Build again to make sure that everything is alright.

This way there is even no need to add CommonModule, only the required classes will be imported.

ParsaArvanehPA commented 9 months ago

This is the final component from ng-zorro that has been made standalone PR #8276 . I think all of them are now standalone, but I will double check later to make sure nothing is left out. @HyperLife1119

HyperLife1119 commented 9 months ago

Thank you for your amazing work! @ParsaArvanehPA

HyperLife1119 commented 9 months ago

There are currently the following components that require BidiModule, just check that the template contains the [dir] binding to confirm this.

We need to make sure that these components import BidiModule correctly. @ParsaArvanehPA

ParsaArvanehPA commented 9 months ago

There are currently the following components that require BidiModule, just check that the template contains the [dir] binding to confirm this.

  • cascader
  • date-picker
  • slider
  • tree-select
  • upload

We need to make sure that these components import BidiModule correctly. @ParsaArvanehPA

The Pr for these components have already been merged to master, so I created a new PR #8279 for cascader and slider and upload.

You can see the changes made to tree-select here.

You can see the changes made to date-picker here.

ParsaArvanehPA commented 9 months ago

Just out of curiosity, why is importing Bidi module needed? If it was required, shouldn't it be giving error or warning during build? @HyperLife1119

HyperLife1119 commented 9 months ago

BidiModule contains a directive named Dir, and its selector is [dir]. It looks like an ordinary DOM property binding, so it will not cause a compilation error if the module is not imported. It is a bit like a trap.

https://github.com/angular/components/blob/main/src/cdk/bidi/dir.ts

I've rechecked the logic of these components with respect to direction, and I've found that they really don't seem to need the [dir] directive, they just rebind the value from the Directionality service to the element's dir attribute and that's it, there's no parsing of the dir value involved. I guess we really don't need BidiModule, you're right!

Sorry, let's delete it again :D

@ParsaArvanehPA

ParsaArvanehPA commented 9 months ago

BidiModule contains a directive named Dir, and its selector is [dir]. It looks like an ordinary DOM property binding, so it will not cause a compilation error if the module is not imported. It is a bit like a trap.

https://github.com/angular/components/blob/main/src/cdk/bidi/dir.ts

I've rechecked the logic of these components with respect to direction, and I've found that they really don't seem to need the [dir] directive, they just rebind the value from the Directionality service to the element's dir attribute and that's it, there's no parsing of the dir value involved. I guess we really don't need BidiModule, you're right!

Sorry, let's delete it again :D

@ParsaArvanehPA

Oh that's a very interesting fact to know 👍 Yeah sure I will make the requested changes as soon as I can :)

evgeniyefimov commented 9 months ago

There are also directives that implicitly augments other components: NzRowDirective augments nz-form-item NzColDirective augments nz-form-control and nz-form-label NzTransitionPatchDirective augments [nz-button], nz-button-group, [nz-icon], [nz-menu-item], [nz-submenu], nz-select-top-control, nz-select-placeholder, nz-input-group

There are maybe more of them, I didn't check the whole repo. Components that are implicitly augmented by these directives need to have them in upper scope. Currently the directives are imported in components' modules, this makes modules mandatory for such components. The simplest solution is to utilize host directives. WDYT?

HyperLife1119 commented 9 months ago

Yes, they definitely need to be migrated to host directives. Do you want to be in charge of this? @evgeniyefimov

evgeniyefimov commented 9 months ago

@HyperLife1119 I tried to figure out NzTransitionPatchDirective usage. Found that it is imported only into NzButtonModule scope, but at the same time this directive has additionally [nz-icon], [nz-menu-item], [nz-submenu], nz-select-top-control, nz-select-placeholder, nz-input-group selectors. I didn't get if it is legacy selectors for these components or not.