Skip to content

feat(module:anchor): support standalone component #8185

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

Conversation

evgeniyefimov
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Transfromed Anchor module to standalone simillar to #8037

Seems BidiModule is unused. Didn't find any usage of this module in the anchor's module. Removed.

Imported only required directives from @angular/common instead of whole CommonModule.

Copy link

codecov bot commented Nov 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (43b34ae) 91.70% compared to head (6756ea8) 91.70%.
Report is 32 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8185   +/-   ##
=======================================
  Coverage   91.70%   91.70%           
=======================================
  Files         519      518    -1     
  Lines       17885    17887    +2     
  Branches     2747     2837   +90     
=======================================
+ Hits        16401    16404    +3     
+ Misses       1182     1181    -1     
  Partials      302      302           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

HyperLife1119
HyperLife1119 previously approved these changes Nov 26, 2023
Copy link
Collaborator

@HyperLife1119 HyperLife1119 left a comment

Choose a reason for hiding this comment

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

I don't think anchor should be a standalone component, because it usually requires the combination of multiple components (nz-anchor and nz-link) to be used. Anchor cannot be used alone.

All functions that require the combination of multiple components to function properly should not be standalone components. Instead, these components should be bundled into one feature through NgModule.

Based on this point of view, it is not suitable to be converted into a standalone component.

WDYT?

@evgeniyefimov
Copy link
Contributor Author

I don't think anchor should be a standalone component, because it usually requires the combination of multiple components (nz-anchor and nz-link) to be used. Anchor cannot be used alone.

All functions that require the combination of multiple components to function properly should not be standalone components. Instead, these components should be bundled into one feature through NgModule.

Based on this point of view, it is not suitable to be converted into a standalone component.

WDYT?

Module is still there, so nz-anchor and nz-link bundled together when module is imported. Also docs mention module import.

From user perspective nothing will be changed.

From developers point of view it will be easier to understand what dependencies are required in components and import only the necessary dependencies, thereby reducing the size of the bundle.

@HyperLife1119
Copy link
Collaborator

I have changed my mind and you are right. We should not restrict whether users should import ng modules or standalone components, this should be left to the user's choice. 👍

Copy link
Collaborator

@HyperLife1119 HyperLife1119 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -26,6 +27,8 @@ import { NzAnchorComponent } from './anchor.component';
selector: 'nz-link',
exportAs: 'nzLink',
preserveWhitespaces: false,
standalone: true,
imports: [NgIf, PlatformModule],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I discovered that PlatformModule is actually an empty module, let's delete it safely :)

@HyperLife1119 HyperLife1119 merged commit 03cda21 into NG-ZORRO:master Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants