Skip to content

#8459 – Refactor: Variables are missing in props validation (let's rewrite JS to TS) 10 - #8519

Merged
AlexeyGirin merged 3 commits into
masterfrom
copilot/refactor-props-validation-to-ts-yet-again
Nov 20, 2025
Merged

AlexeyGirin merged 3 commits into
masterfrom
copilot/refactor-props-validation-to-ts-yet-again

Conversation

Copilot AI commented Nov 14, 2025 •

Copy link
Copy Markdown
Contributor

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

Converted Analyse.jsx to TypeScript to enforce compile-time type safety for component props, eliminating prop validation gaps.

Type definitions added:

  • AnalyseValues – Server response structure (gross formula, molecular weight, exact mass, elemental analysis)
  • RoundSettings – Decimal precision configuration for numeric values
  • AnalyseItem – Display item configuration with optional rounding
  • AnalyseDialogProps – Redux state-derived props (extends DialogParams)
  • AnalyseDialogCallProps – Action callbacks (onAnalyse, onChangeRound)

Code improvements:

  • Typed roundOff helper with explicit string | number → string signature
  • Typed renderInputComponent with ReactElement return type
  • Extracted inline items array to typed constant for maintainability
  • Added context type declaration for ErrorsContext
  • Fixed Select component value prop to pass numeric values instead of strings, ensuring decimal places dropdown displays selected values correctly

Before (JavaScript):

class AnalyseDialog extends Component {
  renderInputComponent(item, values, loading, round) {
    // No type checking on parameters or return value
  }
}

After (TypeScript):

class AnalyseDialog extends Component<Props> {
  renderInputComponent(
    item: AnalyseItem,
    values: AnalyseValues | null,
    loading: boolean,
    round: RoundSettings,
  ): ReactElement {
    // Compile-time type safety enforced
  }
}

Bug fix:
During TypeScript conversion, the Select component's value prop was incorrectly converted to a string (.toString()), but the Select options contain numeric values (0-7). This type mismatch prevented the selected decimal places from being displayed in the dropdown. Fixed by passing numeric values directly to match the option values type.

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
Original prompt

This section details on the original issue you should resolve

<issue_title>Refactor: Variables are missing in props validation (let's rewrite JS to TS) 10</issue_title>
<issue_description>Problem: Variables are missing in props validation
React components should validate prop types

Why is this an issue?
In JavaScript, props are typically passed as plain objects, which can lead to errors and confusion when working with components that have specific prop requirements. However, it lacks of type safety and clarity when passing props to components in a codebase.

By defining types for component props, developers can enforce type safety and provide clear documentation for the expected props of a component. This helps catch potential errors at compile-time. It also improves code maintainability by making it easier to understand how components should be used and what props they accept.

How can I fix it?
Let re-write the entire file on TypeScript and define types for all props.

Problem locations:
packages/ketcher-react/src/script/ui/views/modal/components/process/Analyse/Analyse.jsx</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: AlexeyGirin <26869421+AlexeyGirin@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor variables to enforce props validation with TypeScript #8459 – Refactor: Variables are missing in props validation (let's rewrite JS to TS) 10 Nov 14, 2025
Copilot AI requested a review from AlexeyGirin November 14, 2025 13:52
@AlexeyGirin
AlexeyGirin marked this pull request as ready for review November 14, 2025 13:59
@AlexeyGirin
AlexeyGirin requested review from rrodionov91 and removed request for AlexeyGirin and NataliaLoginova November 14, 2025 13:59
@AlexeyGirin

AlexeyGirin commented Nov 18, 2025 •

Copy link
Copy Markdown
Collaborator

@copilot in resulted Calculated Values dialog values in Decimal places dropdowns are not shown. I can select different value (e.g. 5) and see it applied to values in Molecular Weight and Exact Mass edit boxes but values itself in Decimal places are not shown. Please fix that

image

Changed from `.toString()` to pass numeric values directly to Select component. The Select component expects values that match the option values, which are numbers (0-7), not strings. Converting to string caused a type mismatch preventing the selected value from being displayed.

Co-authored-by: AlexeyGirin <26869421+AlexeyGirin@users.noreply.github.com>

Copilot AI commented Nov 18, 2025

Copy link
Copy Markdown
Contributor Author

@copilot in resulted Calculated Values dialog values in Decimal places dropdowns are not shown. I can select different value (e.g. 5) and see it applied to values in Molecular Weight and `Exac...

Fixed in b799d32. The issue was that I converted the Select value to string (.toString()), but the Select component's options contain numeric values (0-7). This type mismatch prevented the component from finding and displaying the selected option. Now passing the numeric value directly to match the option values.

@AlexeyGirin
AlexeyGirin merged commit 1043e77 into master Nov 20, 2025
27 of 29 checks passed
@AlexeyGirin
AlexeyGirin deleted the copilot/refactor-props-validation-to-ts-yet-again branch November 20, 2025 11:09
AlexeyGirin added a commit that referenced this pull request Nov 20, 2025
…write JS to TS) 10 (#8519)

* Initial plan

* Convert Analyse.jsx to TypeScript with proper type definitions

Co-authored-by: AlexeyGirin <26869421+AlexeyGirin@users.noreply.github.com>

* Fix: Display values in Decimal places dropdowns

Changed from `.toString()` to pass numeric values directly to Select component. The Select component expects values that match the option values, which are numbers (0-7), not strings. Converting to string caused a type mismatch preventing the selected value from being displayed.

Co-authored-by: AlexeyGirin <26869421+AlexeyGirin@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: AlexeyGirin <26869421+AlexeyGirin@users.noreply.github.com>
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: Variables are missing in props validation (let's rewrite JS to TS) 10

3 participants