Table component injection tokens #7754
Conversation
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
packages/core/src/renderer/components/item-object-list/content.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
packages/core/src/renderer/components/item-object-list/content.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
| export const tableFeature = getFeature({ | ||
| id: "core-table-feature", | ||
|
|
||
| register: (di) => { | ||
| di.register(tableComponentInjectable); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Can you remind me (mostly for learning purpose) why do we need to make it as a feature?
(instead of simple some injection token re-use from some package, e.g. import {cystomTableToken} from "@k8slens/item-list")?
There was a problem hiding this comment.
From my understanding, it seems to be an architectural decision. If we utilize an injection token in the core and there are two "users" of this token registered, an error will occur. Only one injection token consumer is allowed. However, by incorporating a feature within the open-lens package, we can easily separate features to determine their visibility in all lens variations. Additionally, the feature is beneficial when dealing with multiple injection tokens that need to be registered.
There was a problem hiding this comment.
@jansav @Iku-turso A good chance to clarify the question and correct me.
There was a problem hiding this comment.
Additionally, the feature is beneficial when dealing with multiple injection tokens that need to be registered.
Can't get it.. why not di.injectMany(customTableToken)? And decide which one to use..
This is simple & perfect for plain component injections/replacement).
There was a problem hiding this comment.
Feature is a group of injectables which are registered and deregistered at once. The idea is that everything related to the feature can be in separate npm package called @k8slens/some-feature which then exports someFeature. Now that open-lens (or any of the applications) registers someFeature with registerFeature, the feature is added.
--
This case seems to have been built in a way that we expect a single implementation of tableComponentInjectionToken to be around. This makes sense, because there should be only one of them. However, the important part here is that implementation tableComponentInjectable should be under open-lens and not under core. Therefore it's only registered in the open-lens and not in any other application which is built on top of core package. In this way di.inject(tableComponentInjectionToken) will be enough. Feature is not needed here because the "open-lens" implementation of table can be moved under open-lens and we're fine with stuff under there being registered individually.
packages/core/src/renderer/components/kube-object-list-layout/kube-object-list-layout.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
| @@ -0,0 +1,15 @@ | |||
| /** | |||
There was a problem hiding this comment.
Move this under open-lens package and import Table from @k8slens/core. Add export if it's not exported yet.
No description provided.