Skip to content

fix(common): remove unused parameters from the ngClass constructor#53831

Closed
josephperrott wants to merge 1 commit intoangular:mainfrom
josephperrott:remove-class-constructor-params
Closed

fix(common): remove unused parameters from the ngClass constructor#53831
josephperrott wants to merge 1 commit intoangular:mainfrom
josephperrott:remove-class-constructor-params

Conversation

@josephperrott
Copy link
Copy Markdown
Member

Remove unused parameters which were only being kept because of a downstream usage in flex layout which is deprecated and end of life.

Remove unused parameters which were only being kept because of a downstream usage in flex layout which is deprecated and end of life
@josephperrott josephperrott added action: merge The PR is ready for merge by the caretaker area: common Issues related to APIs in the @angular/common package target: major This PR is targeted for the next major release labels Jan 8, 2024
@ngbot ngbot bot modified the milestone: Backlog Jan 8, 2024
@pullapprove pullapprove bot requested a review from jessicajaniuk January 8, 2024 16:42
Copy link
Copy Markdown
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

LGTM

reviewed-for: fw-common, public-api

Copy link
Copy Markdown
Member

@pkozlowski-opensource pkozlowski-opensource left a comment

Choose a reason for hiding this comment

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

Yay, thnx!

@pullapprove pullapprove bot requested a review from alxhub January 8, 2024 16:47
@atscott atscott added target: patch This PR is targeted for the next patch release and removed breaking changes target: major This PR is targeted for the next major release labels Jan 8, 2024
@atscott
Copy link
Copy Markdown
Contributor

atscott commented Jan 8, 2024

Removing breaking change label and targeting for patch. We don't consider constructors of injectables part of the public API and do not allow extending classes unless explicitly noted in the documentation: https://github.com/angular/angular/blob/main/docs/PUBLIC_API.md#supported-public-api-surface-of-angular

Copy link
Copy Markdown
Contributor

@atscott atscott left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

@atscott
Copy link
Copy Markdown
Contributor

atscott commented Jan 8, 2024

This PR was merged into the repository by commit 1be6b0a.

atscott pushed a commit that referenced this pull request Jan 8, 2024
…53831)

Remove unused parameters which were only being kept because of a downstream usage in flex layout which is deprecated and end of life

PR Close #53831
@atscott atscott closed this in 1be6b0a Jan 8, 2024
@skovalyov
Copy link
Copy Markdown

What is the best solution for those who haven't yet completed migration from Angular Flex Layout? Is there any reliable Angular Flex Layout fork, which is maintained by the community?

@alfaproject
Copy link
Copy Markdown
Contributor

It's also awkward because updating to latest patch version broke our build, lol. Yes, we use Angular 17, Angular Material 16, and Angular Flex Layout 15. Not even sure what to say at this point, it's not like migrating custom Angular Material based components is a walk in the park and the new Angular 17 features are nice. Getting rid of Angular Material entirely isn't also a walk in the park at this point, so here we are, great.

@skovalyov
Copy link
Copy Markdown

@alfaproject I think I have a relatively easy solution to the Flex Layout problem. I haven't yet implemented it, but I'm pretty sure it should work. FlexLayoutModule declared at https://github.com/angular/flex-layout/blob/master/projects/libs/flex-layout/module.ts re-export 3 other modules, namely FlexModule, ExtendedModule and GridModule. The compatibility problems comes from ExtendedModule, where the override for ngClass directive is provided. The solution can be to create your own module instead of FlexLayoutModule, which would re-export only those parts of functionality you actually need. The only reason to have ngClass implementation from Angular Flex Layout is support for responsive breakpoints. Refactoring those might be much smaller effort for most projects comparing to full migration from Angular Flex Layout to, let's say, Tailwind.

@alfaproject
Copy link
Copy Markdown
Contributor

Yeah, thank you for the tips. We did a patch instead until we have enough bandwidth to get rid of flex layout or reimplement/copy what we need.

@DuncanFaulkner
Copy link
Copy Markdown
Contributor

@josephperrott @atscott @skovalyov @alfaproject @pkozlowski-opensource
Hi, I have ported @angular/flex-layout here https://github.com/ngbracket/ngx-layout

I fully understand that you want to remove any code from the Angular source that references a Library that's gone EOL, but I would like to try and maintain the library for as long as possible (I know this is in decline) but there are still a lot of projects using it.

I would like to try and understand what these two parameters were used for, in the hope that I can find another solution that doesn't require these parameters.

skovalyov added a commit to skovalyov/ngx-layout that referenced this pull request Jan 15, 2024
In angular/angular#53831 `NgClass` directive constructor signature was updated, which is a breaking change for Angular Flex Layout. This change updates the signature by removing `iterableDiffers` and `keyValueDiffers`, which are not used anymore.
DuncanFaulkner pushed a commit to ngbracket/ngx-layout that referenced this pull request Jan 15, 2024
In angular/angular#53831 `NgClass` directive constructor signature was updated, which is a breaking change for Angular Flex Layout. This change updates the signature by removing `iterableDiffers` and `keyValueDiffers`, which are not used anymore.
DuncanFaulkner added a commit to ngbracket/ngx-layout that referenced this pull request Jan 16, 2024
* Merge 17 branch into main (#46)

* feat(project): upgrade to Angular 17

* fix(update): issues with linting - still using tslint needs upgrading to eslint

* fix(update): fix yarn install to latest node

* fix(update): fix local test browser node version

* chore(changelog): update changelog

* Update README.md (#47)

* fix(update): update orbs in CI

* Update Angular version and fix NgClass directive (#50)

In angular/angular#53831 `NgClass` directive constructor signature was updated, which is a breaking change for Angular Flex Layout. This change updates the signature by removing `iterableDiffers` and `keyValueDiffers`, which are not used anymore.

* fix(package): update package json to match yarn lock for browser-sync-client

* fix(package): remove package lock file

* fix(etag): removed etag

* fix(yarn): removed mitt.1.2

* fix(yarn): lock issues failing CI

* fix(typescript): version of typescript was too high

* chore:(build): create release 17.0.1

---------

Co-authored-by: Sergey Kovalyov <skovalyov@gmail.com>
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…ngular#53831)

Remove unused parameters which were only being kept because of a downstream usage in flex layout which is deprecated and end of life

PR Close angular#53831
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
…ngular#53831)

Remove unused parameters which were only being kept because of a downstream usage in flex layout which is deprecated and end of life

PR Close angular#53831
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
…ngular#53831)

Remove unused parameters which were only being kept because of a downstream usage in flex layout which is deprecated and end of life

PR Close angular#53831
@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 12, 2024
@josephperrott josephperrott deleted the remove-class-constructor-params branch November 12, 2024 20:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: common Issues related to APIs in the @angular/common package target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants