-
Notifications
You must be signed in to change notification settings - Fork 5.5k
refactor: Replace Flow with Typescript #26573
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
Conversation
Reviewer's GuideThis PR fully replaces Flow with TypeScript by converting all Flow type annotations to TS, renaming source files to .ts/.tsx, refactoring components for TS compatibility, and updating the build, lint, and packaging configurations to integrate TypeScript support. Class diagram for updated QueryOverview types (Flow to TypeScript)classDiagram
class TaskStatus {
self: string
state: string
}
class TaskStats {
createTimeInMillis: number
elapsedTimeInNanos: number
totalCpuTimeInNanos: number
fullyBlocked: boolean
queuedDrivers: number
runningDrivers: number
blockedDrivers: number
totalDrivers: number
completedDrivers: number
queuedNewDrivers: number
runningNewDrivers: number
totalNewDrivers: number
completedNewDrivers: number
queuedSplits: number
runningSplits: number
totalSplits: number
completedSplits: number
rawInputPositions: number
rawInputDataSizeInBytes: number
totalScheduledTimeInNanos: number
}
class TaskOutputBuffers {
type: string
state: string
totalBufferedBytes: number
}
class Task {
taskId: string
taskStatus: TaskStatus
stats: TaskStats
nodeId: string
outputBuffers: TaskOutputBuffers
}
class RuntimeStat {
name: string
unit: string
sum: number
count: number
max: number
min: number
}
class RuntimeStats {
key_string: RuntimeStat
}
class OutputStage {
stageId: string
self: string
plan: unknown
latestAttemptExecutionInfo: StageExecutionInfo
previousAttemptsExecutionInfos: StageExecutionInfo[]
subStages: OutputStage[]
isRuntimeOptimized: boolean
}
class StageExecutionInfo {
state: string
stats: QueryStats
tasks: Task[]
failureCause: string
}
class QueryStats {
totalScheduledTime: string
totalBlockedTime: string
totalCpuTime: string
cumulativeUserMemory: number
cumulativeTotalMemory: number
userMemoryReservation: string
peakUserMemoryReservation: string
runtimeStats: RuntimeStats
elapsedTime: string
createTime: string
endTime: string
waitingForPrerequisitesTime: string
queuedTime: string
totalPlanningTime: string
executionTime: string
processedInputPositions: number
processedInputDataSize: string
rawInputPositions: number
rawInputDataSize: string
shuffledPositions: number
shuffledDataSize: string
peakTotalMemoryReservation: string
outputPositions: number
outputDataSize: string
writtenOutputPositions: number
writtenOutputLogicalDataSize: string
writtenOutputPhysicalDataSize: string
spilledDataSize: string
}
class FailureInfo {
type: string
message: string
cause: FailureInfo
suppressed: FailureInfo[]
stack: string[]
errorCode: string
errorCause: string
}
class ResourceEstimates {
executionTime: string
cpuTime: string
peakMemory: string
peakTaskMemory: string
key_string: string
}
class SessionRepresentation {
systemProperties: string
catalogProperties: string
resourceEstimates: ResourceEstimates
user: string
principal: string
source: string
catalog: string
schema: string
traceToken: string
timeZoneKey: number
locale: string
remoteUserAddress: string
userAgent: string
clientInfo: string
clientTags: string[]
startTime: number
}
class PrestoWarning {
warningCode: string
message: string
}
class ErrorCode {
code: number
name: string
type: string
retriable: boolean
}
class QueryData {
outputStage: OutputStage
queryId: string
session: SessionRepresentation
preparedQuery: string
warnings: PrestoWarning[]
queryStats: QueryStats
failureInfo: FailureInfo
errorType: string
errorCode: ErrorCode
resourceGroupId: string[]
self: string
memoryPool: string
query: string
}
Task --> TaskStatus
Task --> TaskStats
Task --> TaskOutputBuffers
StageExecutionInfo --> QueryStats
StageExecutionInfo --> Task
OutputStage --> StageExecutionInfo
OutputStage --> OutputStage
QueryStats --> RuntimeStats
RuntimeStats --> RuntimeStat
QueryData --> OutputStage
QueryData --> SessionRepresentation
QueryData --> PrestoWarning
QueryData --> QueryStats
QueryData --> FailureInfo
QueryData --> ErrorCode
SessionRepresentation --> ResourceEstimates
Class diagram for updated PageTitle component types (Flow to TypeScript)classDiagram
class Props {
titles: string[]
urls: string[]
current: number
path: string
}
class State {
noConnection: boolean
lightShown: boolean
info: any | null | undefined
lastSuccess: number
modalShown: boolean
errorText: string | null | undefined
}
Props --> State
Class diagram for updated LivePlan component types (Flow to TypeScript)classDiagram
class StageStatisticsProps {
stage: any
}
class StageNodeInfo {
stageId: string
id: string
root: string
distribution: any
stageStats: any
state: string
nodes: Map<string, any>
}
class OutputStage {
subStages: any
stageId: string
latestAttemptExecutionInfo: any
plan: any
}
class QueryInfo {
outputStage: OutputStage
}
class PlanNodeProps {
id: string
name: string
identifier: string
details: string
sources: string[]
remoteSources: string[]
}
class LivePlanProps {
queryId: string
isEmbedded: boolean
}
class LivePlanState {
initialized: boolean
ended: boolean
query: any | null | undefined
}
OutputStage --> StageNodeInfo
QueryInfo --> OutputStage
Class diagram for updated Split component types (Flow to TypeScript)classDiagram
class QueryState {
query: any | null
failed: boolean
ended: boolean
}
Class diagram for updated SQLInput types (Flow to TypeScript)classDiagram
class PrestoClientConfig {
host: string
port: number
user: string
catalog: string
schema: string
sessions: string
}
class PrestoClient {}
PrestoClientConfig --> PrestoClient
Class diagram for updated SQLClient types (Flow to TypeScript)classDiagram
class SessionValues {
"[key: string]": string
}
Class diagram for updated utility function types (Flow to TypeScript)classDiagram
class UtilityFunctions {
getHumanReadableState(queryState: string, scheduled: boolean, fullyBlocked: boolean, blockedReasons: Array<unknown>, memoryPool: string, errorType: string, errorCodeName: string): string
parseDataSize(value: string): number | null | undefined
parseDuration(value: string): number | null | undefined
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Consider enabling stricter TypeScript compiler options (e.g. strictNullChecks, noImplicitAny) in tsconfig to catch more issues early and improve overall type safety.
- There’s still a lot of
anyusage and@ts-expect-errorshims for third-party libs—adding or sourcing proper type declarations for sparkline, hljs, and jQuery plugins would eliminate many of these ignores and tighten your typings. - You have many inline prop/state types across components—extracting shared interfaces (or using React.FC) could reduce duplication and improve readability/maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider enabling stricter TypeScript compiler options (e.g. strictNullChecks, noImplicitAny) in tsconfig to catch more issues early and improve overall type safety.
- There’s still a lot of `any` usage and `@ts-expect-error` shims for third-party libs—adding or sourcing proper type declarations for sparkline, hljs, and jQuery plugins would eliminate many of these ignores and tighten your typings.
- You have many inline prop/state types across components—extracting shared interfaces (or using React.FC<Props>) could reduce duplication and improve readability/maintainability.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
yhwang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went through the changes and don't see any issue. I will try the changes locally soon and update you.
Great effort! Thanks.
|
@johallar, since you updated the |
|
@johallar one last comment (sorry that I didn't put all comments at once): How do you think about the How do you think? And I know this would lead to one error in the And that's all I have!! Again, thanks for the effort. |
|
@yhwang Yes, definitely think this should be a rule, i'll add and fix the errors! |
yhwang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
/LGTM
Description
This removes flow, and replaces it with typescript, and is the next step in the plan laid out in #25716 (comment)
Motivation and Context
Typescript has better tooling, and has a bigger community around it so this will be another QOL improvement for developers.
Big # line changes is mostly yarn-lock changes, the code diff relatively small
Impact
Test Plan
CI passes, there should be no user facing changes
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.