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

[SUGGESTION] Rename Settings class to a less generic name #173

Open
joaoa-casagrande opened this issue Dec 21, 2024 · 1 comment
Open

[SUGGESTION] Rename Settings class to a less generic name #173

joaoa-casagrande opened this issue Dec 21, 2024 · 1 comment
Labels
breaking-change Resolving this issue will introduce a breaking change. enhancement New feature or request

Comments

@joaoa-casagrande
Copy link

In this example:

settings: Settings = {
  columns: {
    id: {
      title: 'ID'
    },
    name: {
      title: 'Full Name'
    },
    username: {
      title: 'User Name'
    },
    email: {
      title: 'Email'
    }
  }
};

The type of the settings property is called Settings, and with today's IDEs (like WebStorm), automatic import is a very handy feature.
So when a class has a common name, like Config or Settings instead of something more specific like Angular2SmartConfig or Angular2SmartSettings, it' becomes a hassle to import bacuse the IDE may import the wrong.
So maybe changing it to something more specific to this package may be better in the future

@uap-universe
Copy link
Collaborator

uap-universe commented Dec 28, 2024

It is quite common in the typescript world to risk name clashes. For example RxJS is using the generic terms Observable, Observer, Subscriber, Subscription, etc. as if these weren't things that existed long before RxJS has seen light.

The next problem would be with the Row class, which we would need to rename to SmartTableRow.

Also, DataSource is something not really exclusive (and maybe not even expected) for a table library.

I don't want to say "no", right now, because it's a quite easy fix and - let's say - possible in a new major release which will contain a lot of breaking changes anyway, but to be honest I would not have too much hope that there will be a consistent solution for each and every class.

A temporary solution for your project could be to use the import ... as feature of typescript. And to make refactoring in the future less painful, here is the name we would be using in a possible 4.0.0 release: SmartTableSettings.

@uap-universe uap-universe added breaking-change Resolving this issue will introduce a breaking change. enhancement New feature or request labels Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Resolving this issue will introduce a breaking change. enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants