Skip to content
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

Components do not satisfy tea.Model interface #190

Open
j-cr opened this issue Jul 7, 2022 · 6 comments
Open

Components do not satisfy tea.Model interface #190

j-cr opened this issue Jul 7, 2022 · 6 comments
Assignees
Labels
enhancement New feature or request v2

Comments

@j-cr
Copy link

j-cr commented Jul 7, 2022

Rationale: I want to write a function that works with any kind of component. I'm not seeing how to achieve this cleanly, since components actually do not satisfy the github.com/charmbracelet/bubbletea.Model interface. Please correct me if I'm misunderstanding something; I'm totally new to bubbletea!


Problem 1. Missing Init method. Affected components:

  • cursor
  • help
  • list
  • paginator
  • spinner
  • textarea
  • textinput

Not affected: progress, stopwatch, timer, viewport.

var m tea.Model = textinput.New()
// cannot use textinput.New() (value of type textinput.Model) 
// as tea.Model value in variable declaration: 
// missing method Init

Problem 2. The Update method returns concrete component's Model, which makes it incompatible with tea.Model:

var m tea.Model = timer.New(1 * time.Second)
// cannot use timer.New(1 * time.Second) (value of type timer.Model) 
// as tea.Model value in variable declaration: wrong type for method Update (
// have func(msg github.com/charmbracelet/bubbletea.Msg) (github.com/charmbracelet/bubbles/timer.Model, github.com/charmbracelet/bubbletea.Cmd),
// want func(github.com/charmbracelet/bubbletea.Msg) (github.com/charmbracelet/bubbletea.Model, github.com/charmbracelet/bubbletea.Cmd))

Backwards compatibility aside, consider using a type parameter in declaration of tea.Model:

type MyModel[T any] interface {
	Update() MyModel[T]
	// skipped for brevity:
	// Init() tea.Cmd
	// Update(tea.Msg) (MyModel[T], tea.Cmd)
	// View() string
}

type timerModel struct{}
type viewportModel struct{}

func (m timerModel) Update() MyModel[timerModel]       { return m }
func (m viewportModel) Update() MyModel[viewportModel] { return m }

func updateModel[T any](m *MyModel[T]) {
	// e.g.: updateModel(msg, &cmds, &m.timer) instead of:
	// m.timer, cmd = m.timer.Update(msg); cmds = append(cmds, cmd)
	// simplified for brevity
	newM := (*m).Update()
	*m = newM
}

func test() {
	var x1 MyModel[timerModel] = timerModel{}
	var x2 MyModel[viewportModel] = viewportModel{}

	updateModel(&x1)
	updateModel(&x2)
}

This is still suboptimal for sure; alternatively Update could simply return tea.Model which the user then would have to downcast back to component's specific model.

@vito
Copy link

vito commented Jul 20, 2022

Just ran into this myself when trying to make a spinner UI component configurable. I'm not blocked on this since I'll probably just use it as a reason to break up my monolithic UI and use composition instead of configuration, but it does seem like these types should conform to tea.Model in principle.

@bashbunni bashbunni added the enhancement New feature or request label Oct 6, 2022
@nickwitha-k
Copy link

This would indeed be useful. In order to create a form which supports multiple input types, in an extensible manner, I've had to create a new interface in order to compensate for the method signatures not matching.

@meowgorithm
Copy link
Member

meowgorithm commented Feb 6, 2023

Just a note that if this is something you need right now you can go ahead and wrap existing components so that they satisfy tea.Model. For example:

func (m myViewport) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
    var cmd tea.Cmd
    m.actualViewport, cmd = m.actualViewport.Update(msg)
    return m
}

// ...and then also wrap Init and View.

Otherwise, in terms of idiomatic Go, the saying goes “accept interfaces, return structs.” While the gains here will be composability, the tradeoff will be additional assertions on returned tea.Models to assure that they are indeed the types we expect.

// Update a child model.
newModel, cmd := viewport.Update(msg)
if vp, ok := newModel.(viewport.Model); ok {
    m.viewport = vp
}

That's not to say we're not in favor of the change, but something to bear in mind.

@colececil
Copy link

I came here wondering about the same thing. I ran into this issue in a text-based game I was working on. It has a Message component, which is a message that is either a short part of a narrative or a prompt asking the user a question. It has a responseComponent (which is a tea.Model) that can either be a "Press Enter to continue" message or a text input component. It made a lot more sense to use the Bubbles text input component than to use my own, but there wasn't an easy way to fit it in there, since it doesn't implement the tea.Model interface. I ended up writing a wrapper for the Bubbles text input component, as @meowgorithm suggested above, but it was definitely a nontrivial amount of code to have to write: https://github.com/colececil/the-floppy-disk-of-forbidden-creatures/blob/main/internal/ui/input.go.

Bubble Tea's architecture and examples seem to encourage composability, so it feels odd that the components in this library are not composable in this manner. I think the "accept interfaces, return structs" point is valid, but at the same time, it could be argued that tea.Model itself breaks that principle, and thus any application using Bubble Tea is forced to break that principle. This makes me wonder if something could be done in Bubble Tea itself to improve this, such as the generics example @j-cr provided.

@bashbunni bashbunni added the v2 label Jan 15, 2025
@bashbunni bashbunni reopened this Jan 15, 2025
@bashbunni bashbunni added this to the clarifying tea.Model milestone Jan 17, 2025
@bashbunni bashbunni reopened this Jan 17, 2025
youyoumu pushed a commit to youyoumu/bubbles that referenced this issue Jan 19, 2025
Based on charmbracelet#462 and charmbracelet#452 by @londek, but fixes maintaining the
current visibility state across altscreen state changes.

This makes the behavior consistent across terminals, some of which
keep separate state for altscreen and regular buffer.

Fixes charmbracelet#190.
@bashbunni
Copy link
Member

Hey, just an update on this - we're working on a solution internally for v2. Will give an update when we come to a conclusion :)

@bashbunni
Copy link
Member

Related: #717

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v2
Projects
None yet
Development

No branches or pull requests

6 participants