Skip to content

#10979 - Refactor: Explicit/implicit Any type clean up from packages/ketcher-core/src/domain/entities/bond.ts file - #11660

Open
ALexanderSPB wants to merge 6 commits into
masterfrom
10979-refactor-bond
Open

ALexanderSPB wants to merge 6 commits into
masterfrom
10979-refactor-bond

Conversation

@ALexanderSPB

Copy link
Copy Markdown
Collaborator

How the feature works? / How did you fix the issue?

(Screenshots, videos, or GIFs, if applicable)

Check list

  • unit-tests written
  • e2e-tests written
  • documentation updated
  • PR name follows the pattern #1234 – issue name
  • branch name doesn't contain '#'
  • PR is linked with the issue
  • base branch (master or release/xx) is correct
  • task status changed to "Code review"
  • reviewers are notified about the pull request

@AlexeyGirin AlexeyGirin changed the title #10979 - Refactor: bond #10979 - Refactor: Explicit/implicit Any type clean up from packages/ketcher-core/src/domain/entities/bond.ts file Sep 3, 2026
@NataliaLoginova
NataliaLoginova self-requested a review September 8, 2026 12:49
@AlexeyGirin AlexeyGirin reopened this Sep 14, 2026

@la-nuta la-nuta left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two non-blocking test nits; tsc and the entity suite pass with both applied.

Comment on lines +158 to +163
expect(() =>
SGroup.getMassCentre(
{ atoms } as unknown as StructAtomsAndBondsAccess,
[1],
),
).toThrow('SGroup.getMassCentre: atom 1 is not found');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

getMassCentre takes Pick<Struct, 'atoms'>, which { atoms } already satisfies — no cast needed here.

Suggested change
expect(() =>
SGroup.getMassCentre(
{ atoms } as unknown as StructAtomsAndBondsAccess,
[1],
),
).toThrow('SGroup.getMassCentre: atom 1 is not found');
expect(() => SGroup.getMassCentre({ atoms }, [1])).toThrow(
'SGroup.getMassCentre: atom 1 is not found',
);


expect(bond.getCenter(struct)).toStrictEqual(bondCenter);
expect(
bond.getCenter(struct as unknown as StructAtomsAndBondsAccess),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The types are comparable, so a single as StructAtomsAndBondsAccess compiles here and at the other five sites — the as unknown as hop isn't needed.

@AlexeyGirin

Copy link
Copy Markdown
Collaborator

@copilot please make fix based on review comments provided

Copilot AI commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

@copilot please make fix based on review comments provided

Implemented the requested review fixes in commit 09519fa: removed the unnecessary cast in SGroup.getMassCentre and replaced as unknown as StructAtomsAndBondsAccess with single casts at the remaining test sites.

Copilot AI requested a review from AlexeyGirin September 14, 2026 13:30
@AlexeyGirin AlexeyGirin changed the title #10979 - Refactor: Explicit/implicit Any type clean up from packages/ketcher-core/src/domain/entities/bond.ts file #10979 - Refactor: Explicit/implicit Any type clean up from packages/ketcher-core/src/domain/entities/bond.ts file Oct 4, 2026
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.

Refactor: Explicit/implicit Any type clean up from packages/ketcher-core/src/domain/entities/bond.ts file

5 participants