Skip to content

fix: conflicting ts field type #294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 7, 2025

Conversation

tiagoapolo
Copy link
Contributor

Fix ts class field conflicting type definitions

This was causing issues when defining flagsmith.json type to IState as reported in #292

@tiagoapolo tiagoapolo requested a review from Copilot March 4, 2025 18:59
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This pull request aims to resolve conflicting type definitions for the ts field in the Flagsmith class, addressing issues with the flagsmith.json type.

  • Updated the ts field from "number | null = null" to an optional "ts?: number".
  • Removes the default null assignment to adhere to type definitions expected by IState.

Reviewed Changes

File Description
flagsmith-core.ts Changed ts field type to an optional number to fix type conflicts.

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

flagsmith-core.ts:264

  • Changing 'ts' from a field initialized to null to an optional number means it may be undefined in some cases. Please verify that all usages of 'ts' properly handle undefined values to maintain consistent behavior.
ts?: number

@tiagoapolo tiagoapolo self-assigned this Mar 4, 2025
@tiagoapolo tiagoapolo merged commit 23f90b5 into main Mar 7, 2025
1 check passed
@tiagoapolo tiagoapolo deleted the fix/conflicting-ts-field-type--292 branch March 7, 2025 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants