Closed unknown closed 5 months ago
looks good otherwise i think!
in general a bit worried about refactors since every single part of our production code relies on priompt, and even well-intentioned and well-thought-through refactors have a risk of introducing bugs (sometimes just because the previous implementation was buggy and other code unintentionally relied on that specific bugginess!)
looks good otherwise i think!
in general a bit worried about refactors since every single part of our production code relies on priompt, and even well-intentioned and well-thought-through refactors have a risk of introducing bugs (sometimes just because the previous implementation was buggy and other code unintentionally relied on that specific bugginess!)
Understand this completely! I've tried to keep the changes as minimal as possible and focused on just making Priompt's element types more similar to React's. There aren't any significant logic changes other than removing .flat()
from some children props and the bugs in computePriorityLevels
, validateNotBothAbsoluteAndRelativePriority
, and validateNoChildrenHigherPriorityThanParent
you highlighted.
This PR makes
PromptNode
andPromptElement
types more similar toReact.ReactNode
andReact.ReactElement
, respectively. That is, thePromptElement
type now encapsulates all the internal object representations of elements created throughPrompt.createElement
and JSX transpilation, andPromptNode
now encapsulates all types that can be rendered (thePromptElement
type in addition to literal types like strings and numbers).Fixing these types allows some
Array.prototype.flat()
calls that were used to wrangle with Typescript to be removed.NOTE: This change may require changing the return types of Priompt components from
PromptElement
toPromptNode
. For example, the following would cause a type error because thePromptElement
type does not include strings, even thoughExamplePrompt
is a renderable component.Changing the
PromptElement
type toPromptNode
would fix this type error.