Skip to content

CardView - create .d.ts#29455

Merged
mpreyskurantov merged 68 commits into25_1from
grids/cardview/dts_1_of_3
May 8, 2025
Merged

CardView - create .d.ts#29455
mpreyskurantov merged 68 commits into25_1from
grids/cardview/dts_1_of_3

Conversation

@pomahtri
Copy link
Copy Markdown
Contributor

No description provided.

@pomahtri pomahtri added the 25_1 label Mar 31, 2025
@pomahtri pomahtri requested a review from a team March 31, 2025 07:23
@pomahtri pomahtri self-assigned this Mar 31, 2025
@pomahtri pomahtri requested a review from a team March 31, 2025 07:23
Base automatically changed from grids/cardview/ssr to grids/cardview/main April 3, 2025 18:44
@pomahtri pomahtri changed the title CardView - create .d.ts (1st part) CardView - create .d.ts Apr 21, 2025
@pomahtri pomahtri force-pushed the grids/cardview/dts_1_of_3 branch 4 times, most recently from d2b541e to 364b395 Compare April 22, 2025 09:10
@pomahtri pomahtri force-pushed the grids/cardview/main branch from 541cfd5 to 43037ec Compare April 22, 2025 12:17
@pomahtri pomahtri requested review from a team as code owners April 22, 2025 12:17
@pomahtri pomahtri force-pushed the grids/cardview/dts_1_of_3 branch from d7db79e to 86284ed Compare April 22, 2025 12:27
@pomahtri pomahtri removed request for a team April 22, 2025 12:28
Base automatically changed from grids/cardview/main to 25_1 April 22, 2025 13:16
@pomahtri pomahtri force-pushed the grids/cardview/dts_1_of_3 branch 2 times, most recently from 27bf1f6 to 400c6fa Compare April 22, 2025 15:42
* @public
* @namespace DevExpress.ui.dxCardView
*/
export type CardHeaderToolbarItem = dxToolbarItem & {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
export type CardHeaderToolbarItem = dxToolbarItem & {
export type CardHeaderItem = dxToolbarItem & {

Since it is only used in CardHeader, the Toolbar part is unnecessary

Copy link
Copy Markdown
Contributor

@wdevfx wdevfx May 7, 2025

Choose a reason for hiding this comment

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

Fixed

But one thought worried me a bit
If we would like to have some items in the Card in future -> the CardHeaderItem name will be already reserved

};

/** @public */
export type CardHeaderPredefinedToolbarItem = 'selectionCheckBox' | 'updateButton' | 'deleteButton';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
export type CardHeaderPredefinedToolbarItem = 'selectionCheckBox' | 'updateButton' | 'deleteButton';
export type CardHeaderPredefinedItem = 'selectionCheckBox' | 'updateButton' | 'deleteButton';

Since it is only used in CardHeader, the Toolbar part is unnecessary

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixed

Same thoughts like about the CardHeaderToolbarItem fix
But the chance here is extremely low,I guess

/**
* @docid
*/
type WithFieldValueInfo = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question
Just curious, why WithFieldValueInfo, not FieldValueInfo?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Because this type is used to add sth to existing types, I guess :)

E.g:

export type FieldValuePreparedEvent = EventInfo<dxCardView> & WithFieldValueInfo;

An event with field value info

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It sounds like a name of a HOC

* @public
* @namespace DevExpress.ui.dxCardView
*/
export type FieldInfoType = { // TODO: rename to FieldInfo
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
export type FieldInfoType = { // TODO: rename to FieldInfo
export type FieldInfo = {

I see the TODO, just to have a conversation entry for ir

Copy link
Copy Markdown
Contributor

@wdevfx wdevfx May 7, 2025

Choose a reason for hiding this comment

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

Fixed

But @pomahtri warned that the FieldInfo causes some error (I don't have more details)
So, if we will face complex issues with this name -> let's rollback my fix to the FieldInfoType with TODO

Copy link
Copy Markdown
Contributor

@alexslavr alexslavr May 8, 2025

Choose a reason for hiding this comment

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

Hi, @IlyaKhD, @wdevfx. FieldInfo causes a conflict with FieldInfo from FilterBuilder, which is also used in CardView and imported in generated components directly from 'filter_builder' module. This change will lead to re-generation in CardView and other grids with import { FieldInfo } from 'filter_builder' replaced to import { FieldInfo } from 'card_view' which is totally wrong. So we can revert renaming or use WA with prefixed internal type.

* @docid
* @type object
*/
value: unknown;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not 100% sure about that, but it seems that we shouldn't use unknown for values that can be passed to consuming code. It's generallly a good practise, but all the other components currently use any.

Suggested change
value: unknown;
value: any;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agree with u here
Let's not risk and use any for now

Btw, @IlyaKhD, what do u think about unknown in generics?
I guess it should be alright, but at night it started worried me a bit because unknown !== any

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, I don't have a general idea for all generics, better investigate some samples. We have reasons to avoid giving unknown outside, and can leverage from marking as unknown for data going inside. Is there a gurantee that generics never given outside - hm... not sure about that, it will be hard to lint, I think. Not sure about possible covariance / contravariance issues, 2-way bindings in Angualr, etc

* @public
* @type object
*/
dragging?: Pick<dxSortableOptions, 'dropFeedbackMode' | 'scrollSpeed' | 'scrollSensitivity' | 'onDragChange' | 'onDragEnd' | 'onDragMove' | 'onDragStart' | 'onRemove' | 'onReorder'>;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion
Maybe it would be useful to extract a public type for dragging:HeaderPanelDragging

Copy link
Copy Markdown
Contributor

@wdevfx wdevfx May 7, 2025

Choose a reason for hiding this comment

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

Agree -> fixed

@IlyaKhD, could u please re-check that I did it right (I may have missed some jsDoc comment or sth)?
But as far as I can see, everything should be alright :)

* @docid
* @public
*/
filterBuilder?: dxFilterBuilderOptions;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can create a local public alias, so that it get into card_view_types.d.ts and aggregated exports in framework packages:

/**
 * @public
 */
type FilterBuilder = dxFilterBuilderOptions;
export {
  FilterBuilder
}
Suggested change
filterBuilder?: dxFilterBuilderOptions;
filterBuilder?: FilterBuilder;

Copy link
Copy Markdown
Contributor

@wdevfx wdevfx May 7, 2025

Choose a reason for hiding this comment

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

This change leads to the following error:

System.Exception: "dxCardView.filterBuilder" has unknown type. Try adding @docid tag to the type.
         at DevExtreme.NgSmdGenerator.StrongType.Builder.Build(String optionName, String optionFullName, DataType[] types, Boolean hasAcceptableValues, Boolean hasNestedOptions)
         at DevExtreme.NgSmdGenerator.Program.BuildStrongType(Builder builder, HierarchicalMember member, String optionFullName)
         at DevExtreme.NgSmdGenerator.Program.<>c__DisplayClass2_0.<ToSerializable>b__5(HierarchicalMember m)
         at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer)
         at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector)
         at DevExtreme.NgSmdGenerator.Program.ToSerializable(HierarchicalMember member, Builder typesBuilder, String name)
         at DevExtreme.NgSmdGenerator.Program.<>c__DisplayClass1_0.<MainCore>b__3(HierarchicalMember m)
         at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](TSource[] source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer)
         at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer)
         at DevExtreme.NgSmdGenerator.Program.MainCore(ProgramConfig programConfig)
         at DevExtreme.NgSmdGenerator.Program.Main(String[] args)

As logs adviced -> I added the docid:

/**
 * @docid
 * @public
 */
export type FilterBuilder = dxFilterBuilderOptions;

But I'm not sure that it's ok (we already have docid on the dxFilterBuilderOptions)
And get the following error on the last Angular regenerate step:

 [04:00:49] TypeError: Cannot set properties of undefined (setting 'baseClass')
          at ComponentMetadataGenerator.generateComplexOptionByType (/Users/wdevfx/work/devextreme_prod/node_modules/.pnpm/devextreme-internal-tools@18.0.0-beta.1/node_modules/devextreme-internal-tools/ts/angular-components-generator/metadata-generator.js:444:43)
          at ComponentMetadataGenerator.generate (/Users/wdevfx/work/devextreme_prod/node_modules/.pnpm/devextreme-internal-tools@18.0.0-beta.1/node_modules/devextreme-internal-tools/ts/angular-components-generator/metadata-generator.js:283:47)
          at /Users/wdevfx/work/devextreme_prod/packages/devextreme-angular/gulpfile.js:38:13
          at bound (node:domain:433:15)
          at runBound (node:domain:444:12)
          at asyncRunner (/Users/wdevfx/work/devextreme_prod/node_modules/.pnpm/async-done@1.3.2/node_modules/async-done/index.js:55:18)
          at process.processTicksAndRejections (node:internal/process/task_queues:77:11)

@IlyaKhD, may we temporarily leave it as is with direct dxFilterBuilderOptions type?

@wdevfx wdevfx requested a review from IlyaKhD May 8, 2025 00:08
@mpreyskurantov mpreyskurantov merged commit 7b74d23 into 25_1 May 8, 2025
300 checks passed
@mpreyskurantov mpreyskurantov deleted the grids/cardview/dts_1_of_3 branch May 8, 2025 16:45
mpreyskurantov added a commit that referenced this pull request May 8, 2025
haksur pushed a commit to haksur/DevExtreme that referenced this pull request May 15, 2025
Co-authored-by: Mikhail Preyskurantov <5574159+mpreyskurantov@users.noreply.github.com>
Co-authored-by: wdevfx <ilya.vinogradov@devexpress.com>
Co-authored-by: alexlavrov <alex.s.lavrov@gmail.com>
ajivanyandev pushed a commit that referenced this pull request May 23, 2025
Co-authored-by: Mikhail Preyskurantov <5574159+mpreyskurantov@users.noreply.github.com>
Co-authored-by: wdevfx <ilya.vinogradov@devexpress.com>
Co-authored-by: alexlavrov <alex.s.lavrov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants