Skip to content

refactor(igniteui-angular): enabling strict true for the library - #17483

Open
kdinev wants to merge 27 commits into
masterfrom
strict-true
Open

refactor(igniteui-angular): enabling strict true for the library#17483
kdinev wants to merge 27 commits into
masterfrom
strict-true

Conversation

@kdinev

@kdinev kdinev commented Aug 5, 2026

Copy link
Copy Markdown
Member

Closes #17403

Description

Motivation / Context

Type of Change (check all that apply):

  • Bug fix
  • New functionality
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (no functional changes)
  • Documentation
  • Demos
  • CI/CD
  • Tests
  • Changelog
  • Skills/Agents

Component(s) / Area(s) Affected:

How Has This Been Tested?

  • Unit tests
  • Manual testing
  • Automated e2e tests

Test Configuration:

  • Angular version:
  • Browser(s):
  • OS:

Screenshots / Recordings

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them
  • Accessibility (ARIA, keyboard navigation, focus management) has been verified

Copilot AI review requested due to automatic review settings August 5, 2026 08:41

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Comment thread projects/igniteui-angular/core/src/services/overlay/overlay.ts Fixed
if (queue.hasOwnProperty(id)) {
const fn = queue[id];
delete queue[id];
fn();
}
};

const listener = (event) => run(event.data);
const listener = (event: MessageEvent) => run(event.data);
Copilot AI review requested due to automatic review settings August 5, 2026 08:49

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

…ariable'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 5, 2026 08:50

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@kdinev
kdinev requested a review from Copilot August 5, 2026 08:51

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Copilot AI review requested due to automatic review settings August 5, 2026 10:33

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Copilot AI review requested due to automatic review settings August 5, 2026 14:34

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Copilot AI review requested due to automatic review settings August 5, 2026 15:56

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Copilot AI review requested due to automatic review settings August 5, 2026 16:08

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@kdinev kdinev changed the title refactor(igniteui-angular): enabling strict: true for the library refactor(igniteui-angular): enabling strict true for the library Aug 5, 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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Copilot AI review requested due to automatic review settings August 6, 2026 13:36

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Copilot AI review requested due to automatic review settings August 7, 2026 07:48

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Copilot AI review requested due to automatic review settings August 7, 2026 09:02

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@rkaraivanov
rkaraivanov self-requested a review August 7, 2026 13:00
// export { IgxRadioGroupDirective } from 'igniteui-angular/radio';
export * from './directives/ripple/ripple.directive';
export * from './directives/scroll-inertia/scroll_inertia.directive';
export * from './directives/size/ig-size.directive';

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 looks like IgSizeDirective was intentionally moved from the library into the demo application, but AppComponent still imports it from igniteui-angular/directives. This causes npm start to fail because the corresponding public export was removed. Could we update the demo to import the new local directive instead?

Since this also removes an existing public export, could we confirm whether any changelog or migration follow-up is needed?

Image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that this was never meant to be public and I dont think anyone has ever used it. So we should be safe without changelog entry and migrations

@@ -3074,7 +3075,7 @@ export abstract class IgxGridBaseDirective implements GridType,
/**
* @hidden
*/
protected _pagingState;
protected _pagingState!: IPagingState;

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.

This typing change exposes a pre-existing issue in src/app/grid/grid.sample.html: it accesses grid2.pagingState?.paging?.index, but IPagingState exposes index directly. Could we update it to grid2.pagingState?.index as part of this PR? This currently causes npm start to fail.

Image

appTree = setupTestTree();
});

const migrationName = 'migration-58';

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 looks like this spec runs migration-58, while the new import migration is registered as migration-59. Could we update migrationName accordingly? The current value runs the existing update-22_1_0 migration instead and causes these tests to fail.

Suggested change
const migrationName = 'migration-58';
const migrationName = 'migration-59';

// Add remaining imports for the original entry point first
if (remaining.length > 0) {
const sortedImports = remaining.sort();
newImports.push(`import { ${sortedImports.join(', ')} } from '${importPath}';`);

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.

Could we also preserve type-only imports during the rewrite? For example, import type { IGroupingDoneEventArgs } ... is currently regenerated as a regular import { IGroupingDoneEventArgs } ..., which may affect projects using verbatimModuleSyntax.

// Add moved imports
for (const [entryPoint, imports] of entryPointGroups) {
const sortedImports = imports.sort();
newImports.push(`import { ${sortedImports.join(', ')} } from '${source.basePackage}/${entryPoint}';`);

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.

Could we merge migrated symbols into an existing import from the target entry point? For example, when IgxSummaryResult is already imported from igniteui-angular/core, migrating the summary operands currently creates a second import from the same module.

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review & update Grid summaries API surface

7 participants