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

feat(COMPONENT): Make DataSource<T> an interface #21887

Closed
s4m0r4m4 opened this issue Feb 12, 2021 · 4 comments · Fixed by #21921
Closed

feat(COMPONENT): Make DataSource<T> an interface #21887

s4m0r4m4 opened this issue Feb 12, 2021 · 4 comments · Fixed by #21921
Labels
area: cdk/table docs This issue is related to documentation P4 A relatively minor issue that is not relevant to core functions

Comments

@s4m0r4m4
Copy link
Contributor

s4m0r4m4 commented Feb 12, 2021

Feature Description

Instead of an entirely abstract class (see https://github.com/angular/components/blob/master/src/cdk/collections/data-source.ts), why not make it an interface?

Use Case

This would enable users such as myself to create custom data sources that extend other classes. With DataSource being an abstract class, it means I have to extend that to make a custom DataSource and therefore cannot extend any other class with my custom DataSource. My use case is that i have a logging base class, and want to extend it in a new class to also act as a custom Data Source.

@s4m0r4m4 s4m0r4m4 added feature This issue represents a new feature or feature request rather than a bug or bug fix needs triage This issue needs to be triaged by the team labels Feb 12, 2021
@crisbeto
Copy link
Member

Changing it at this point would be breaking. For what it's worth, TypeScript allows you to implement classes as well. E.g. you should be able to do MyDataSource implements DataSource.

@s4m0r4m4
Copy link
Contributor Author

Thanks for the lightning-fast response! Yeah I get that it's a breaking change, maybe not worth it at this point in time. Perhaps just for consideration if you guys ever revamp Table Datasources.

Thanks for the tip about implementing the DataSource instead of extending it, that worked just fine for me. That tidbit could be helpful for addition to the docs, what do you think? I'd be happy to submit a quick PR for the docs update.

@crisbeto
Copy link
Member

Sure, feel free to send out a PR and I can review it.

@crisbeto crisbeto added area: cdk/table docs This issue is related to documentation P4 A relatively minor issue that is not relevant to core functions and removed feature This issue represents a new feature or feature request rather than a bug or bug fix needs triage This issue needs to be triaged by the team labels Feb 15, 2021
s4m0r4m4 added a commit to s4m0r4m4/components that referenced this issue Feb 16, 2021
docs(table): additional details on table data sources

Provides additional documentation and clarification on extending the base DataSource class with custom data sources either by extending or implementing.

Fixes angular#21887
mmalerba pushed a commit that referenced this issue Feb 16, 2021
Provides additional documentation and clarification on extending the base DataSource class with custom data sources either by extending or implementing.

Fixes #21887

* docs(material/table) removing backticks around "extending"
mmalerba pushed a commit that referenced this issue Feb 16, 2021
Provides additional documentation and clarification on extending the base DataSource class with custom data sources either by extending or implementing.

Fixes #21887

* docs(material/table) removing backticks around "extending"

(cherry picked from commit 5c57c86)
@angular-automatic-lock-bot
Copy link

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 Mar 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: cdk/table docs This issue is related to documentation P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants