Skip to content

Events_strategy: improve typing - #35135

Open
EugeniyKiyashko wants to merge 3 commits into
DevExpress:mainfrom
EugeniyKiyashko:typescript/core/events_strategy_5110
Open

Events_strategy: improve typing#35135
EugeniyKiyashko wants to merge 3 commits into
DevExpress:mainfrom
EugeniyKiyashko:typescript/core/events_strategy_5110

Conversation

@EugeniyKiyashko

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI lite review requested due to automatic review settings September 9, 2026 22:42
@EugeniyKiyashko EugeniyKiyashko self-assigned this Sep 9, 2026

Copilot AI left a comment

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.

🟢 Approval recommended

The changes are a straightforward internal rename/refactor with verified reference updates and no remaining usages of the removed module.

Pull request overview

This PR updates the internal EventsStrategy implementation to a better-typed TypeScript module and adjusts entry points/imports to reference the new module, removing the legacy m_events_strategy file.

Changes:

  • Replaced __internal/core/m_events_strategy.ts with a new __internal/core/events_strategy.ts that adds explicit TS types/interfaces.
  • Updated the deprecated public entry js/core/events_strategy.js to re-export from the new internal module.
  • Updated Knockout validation integration to import EventsStrategy from the new @ts/core/events_strategy module.
File summaries
File Description
packages/devextreme/js/core/events_strategy.js Deprecated entry now re-exports EventsStrategy from the new internal module path.
packages/devextreme/js/__internal/integration/knockout/validation.ts Updates import to @ts/core/events_strategy to follow the renamed internal module.
packages/devextreme/js/__internal/core/m_events_strategy.ts Removes the legacy implementation file.
packages/devextreme/js/__internal/core/events_strategy.ts Adds the new typed EventsStrategy implementation and related TS interfaces/types.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/devextreme/js/__internal/core/events_strategy.ts Outdated
pharret31
pharret31 previously approved these changes Sep 9, 2026

Copilot AI left a comment

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.

🔵 Needs a closer look

The new TypeScript implementation introduces typing/unused-parameter issues that are likely to be flagged by lint/TS rules and should be corrected before merging.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

packages/devextreme/js/__internal/core/events_strategy.ts:5

  • Avoid using the Function type for event handlers; it disables parameter/return type checking and is inconsistent with other callback typing in the codebase (e.g. document_size_callbacks.ts uses a concrete function signature). Use an explicit callable signature instead.
    packages/devextreme/js/__internal/core/events_strategy.ts:100
  • The eventName parameter in this callback is unused; this is likely to be flagged by TS/ESLint unused-parameter rules. Use _ (or prefix with _) to mark it intentionally unused.
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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.

3 participants