Skip to content

feat: Migrate to Charmbracelet Bubbletea and Bubbles libraries to v2.#757

Open
marwan562 wants to merge 5 commits intogoharbor:mainfrom
marwan562:update_bubbleteaV2
Open

feat: Migrate to Charmbracelet Bubbletea and Bubbles libraries to v2.#757
marwan562 wants to merge 5 commits intogoharbor:mainfrom
marwan562:update_bubbleteaV2

Conversation

@marwan562
Copy link
Copy Markdown
Contributor

@marwan562 marwan562 commented Mar 19, 2026

Description

Upgrade CLI to support bubbletea v2

Type of Change

Please select the relevant type.

  • Bug fix
  • New feature
  • Refactor
  • Documentation update
  • Chore / maintenance

@NucleoFusion
Copy link
Copy Markdown
Contributor

DCO is failing btw

Signed-off-by: marwan562 <mixing.gamer546@gmail.com>
@marwan562 marwan562 force-pushed the update_bubbleteaV2 branch from 7e8f6ee to bd7d04d Compare March 19, 2026 13:52
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 19, 2026

Codecov Report

❌ Patch coverage is 0% with 88 lines in your changes missing coverage. Please review.
✅ Project coverage is 7.79%. Comparing base (60ad0bd) to head (cf5cb68).
⚠️ Report is 119 commits behind head on main.

Files with missing lines Patch % Lines
pkg/views/base/multiselect/model.go 0.00% 15 Missing ⚠️
pkg/views/base/selection/model.go 0.00% 8 Missing ⚠️
cmd/harbor/root/robot/update.go 0.00% 6 Missing ⚠️
pkg/views/base/tablegrid/model.go 0.00% 6 Missing ⚠️
pkg/views/base/tablelist/model.go 0.00% 4 Missing ⚠️
cmd/harbor/root/robot/create.go 0.00% 3 Missing ⚠️
pkg/views/project/select/view.go 0.00% 2 Missing ⚠️
pkg/views/robot/create/view.go 0.00% 2 Missing ⚠️
pkg/views/robot/select/view.go 0.00% 2 Missing ⚠️
pkg/views/vulnerability/summary/view.go 0.00% 2 Missing ⚠️
... and 38 more
Additional details and impacted files
@@            Coverage Diff            @@
##             main    #757      +/-   ##
=========================================
- Coverage   10.99%   7.79%   -3.20%     
=========================================
  Files         173     270      +97     
  Lines        8671   13173    +4502     
=========================================
+ Hits          953    1027      +74     
- Misses       7612   12033    +4421     
- Partials      106     113       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@NucleoFusion
Copy link
Copy Markdown
Contributor

From the list in the issue, can you tell us which of the list items applied to harbor, which didnt, and which are completed?
So we can assure everything works

@marwan562
Copy link
Copy Markdown
Contributor Author

Hey @NucleoFusion! Here's a breakdown of every item from the upgrade guide mapped to this PR:


Bubbletea v2 upgrade checklist

# Item Status Notes
1 Update import paths ✅ Done All 66 .go files updated from github.com/charmbracelet/*charm.land/*/v2. go.mod updated too.
2 View() stringView() tea.View ✅ Done Updated in multiselect/model.go and selection/model.go. Returns tea.NewView(...) with v.AltScreen = true.
3 Replace tea.KeyMsgtea.KeyPressMsg ✅ Done Updated in both model files. Switch cases now use tea.KeyPressMsg.
4 Update key fields: msg.Type / msg.Runes / msg.Alt — N/A Harbor-cli only uses msg.String() for key handling — .Type, .Runes, .Alt are never accessed directly.
5 Replace case " ":case "space": ✅ Done Fixed in multiselect/model.go.
6 Update mouse message usage — N/A Harbor-cli has no mouse event handling.
7 Rename mouse button constants — N/A No mouse button constants used anywhere in the codebase.
8 Remove old program options → use View fields ✅ Done tea.WithAltScreen() removed from all NewProgram() calls. AltScreen is now set via v.AltScreen = true on the tea.View return.
9 Remove imperative commands → use View fields ✅ Done Covered by the AltScreen change above. No other imperative program commands were used.
10 Remove old program methods — N/A Harbor-cli uses a fire-and-forget pattern with NewProgram().Run() — no p.Send() or p.Quit() calls exist.
11 tea.WindowSize()tea.RequestWindowSize — N/A Window sizing is handled reactively via tea.WindowSizeMsg. The function is never called directly.
12 tea.Sequentially(...)tea.Sequence(...) — N/A tea.Sequentially was never used in harbor-cli.

Bonus (viewport API also changed in v2, handled in this PR):

Item Status Notes
viewport.New(w, h) → functional options ✅ Done Updated in multiselect/model.go to use viewport.WithWidth / viewport.WithHeight.
.Width field → .Width() method ✅ Done Updated in headerView and footerView.
Remove useHighPerformanceRenderer + viewport.Sync() ✅ Done Both removed from multiselect/model.go.

Summary: All 6 applicable items are complete. The remaining 6 are not applicable to harbor-cli as those patterns simply don't exist in the codebase.

Copy link
Copy Markdown
Contributor

@NucleoFusion NucleoFusion left a comment

Choose a reason for hiding this comment

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

huh has been updated to v2 as well
please change the imports for those too
Run a go mod tidy as well if you can

I also seem to forgot to mention this in the original issue, but if we can, we should also replace

switch msg.String() {
case "ctrl+c":
// more
}

With

switch msg.Code {
case tea.KeyCtrlC:
// more
}

Signed-off-by: marwan562 <mixing.gamer546@gmail.com>
… commands.

Signed-off-by: marwan562 <mixing.gamer546@gmail.com>
@marwan562
Copy link
Copy Markdown
Contributor Author

marwan562 commented Mar 23, 2026

I have an issue when replacing

switch msg.String() {
case "ctrl+c":
    // more
}

With

switch msg.Code {
case tea.KeyCtrlC:
    // more
}

It gives me undefined: tea.KeyCtrlC even after updating all dependencies and running go mod tidy.

and import path is

import (
   tea "charm.land/bubbletea/v2"
)
issue

Signed-off-by: marwan562 <mixing.gamer546@gmail.com>
@NucleoFusion
Copy link
Copy Markdown
Contributor

Leave it as string() then

@marwan562
Copy link
Copy Markdown
Contributor Author

Ok I leave it as String(), I thought we finished our upgrade (bubbletea, huh to v2).

@NucleoFusion
Copy link
Copy Markdown
Contributor

Yep, it seems to be there
I will have a look at the PR, give me a minute

@marwan562
Copy link
Copy Markdown
Contributor Author

For sure 😊

Copy link
Copy Markdown
Contributor

@NucleoFusion NucleoFusion left a comment

Choose a reason for hiding this comment

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

The current PR state results in malformed output when using the models
This is due to the new nature of model render state to be kept in the view instead of the model itself.
Due to this the previous model structure of Init(),

func (m Model) Init() tea.Cmd {
	return tea.Quit
}

Doesnt work anymore, because the program quits before the full output is done.

We need to find a solution for this. I will let you know soon. If you have any proposed solutions on how to make this work. Please do tell.

@marwan562
Copy link
Copy Markdown
Contributor Author

Hey @NucleoFusion, I've looked into the issue and I think I have a solution.

Instead of returning tea.Quit directly from Init(), we can return a custom command that fires a doneMsg, then call tea.Quit from inside Update() after that message is received. This guarantees at least one full render cycle completes before the program exits, which is what v2 requires since render state is now managed by the framework and not the model itself.

type doneMsg struct{}

func (m Model) Init() tea.Cmd {
    return func() tea.Msg {
        return doneMsg{}
    }
}

func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
    switch msg.(type) {
    case doneMsg:
        return m, tea.Quit
    }
    return m, nil
}

This way View() is called and the output is fully rendered before the quit signal is processed. I'll apply this pattern across all the affected models. Let me know if you think this approach works or if you had something else in mind!

@NucleoFusion
Copy link
Copy Markdown
Contributor

@marwan562 I believe I tried something similar. Have you tried running project list or something similar? it worked?

@marwan562
Copy link
Copy Markdown
Contributor Author

Hey @NucleoFusion, you're right to flag that — I haven't fully tested it against project list yet.

Thinking about it more, I realize the DoneMsg pattern alone won't work for data-fetching models. The problem there is that DoneMsg fires before the API response comes back, so View() renders with empty data before quitting.

The correct fix for those models should be:

  1. Init() fires the data fetch command (not DoneMsg)
  2. Update() receives the loaded data, stores it in the model, then returns tea.Quit
  3. View() renders with the fully loaded data before the program exits

So the fix is actually two different patterns depending on the model type:

  • Pure output models (no async) → DoneMsg in Init(), quit in Update()
  • Data-fetching models (like project list) → fetch in Init(), quit in Update() after data arrives

I'll test project list and other commands locally and update the PR accordingly. Thanks for catching this!

@NucleoFusion
Copy link
Copy Markdown
Contributor

The issue is not the API response. The issue is that somehow view() and Init() run concurrently or something. Which makes it a race condition or something and the View() interprets the tea.Quit before it does the whole render.
The solution to this would be removing the model altogether and instead of the NewModel returning a wrapper model, we make it return the table itself.
Which would then need to be rendered via it's .View() which still returns string.

Also look into how you can split this PR up, into separates. For example, updating lipgloss in one, bubbles in another and bubbletea in another. Check the plausibility of it and let me know.

@marwan562
Copy link
Copy Markdown
Contributor Author

Fix Applied

The race condition is fixed in commit df61f7a.

Root cause confirmed: The Model wrapper was running tea.Quit via doneMsg in Init(), but bubbletea v2 now manages render state internally — the quit signal fired before the frame was flushed to the terminal.

Fix: Removed the Model wrapper entirely from pkg/views/base/tablelist/model.go. NewModel() now returns table.Model directly. A Render(t table.Model) string helper applies BaseStyle and returns the rendered string. All 41 affected view files now call fmt.Print(tablelist.Render(m)) directly — no tea.Program involved. Interactive selection views (selection.NewModel) are unaffected.


PR Splitting Feasibility

Here is the analysis on splitting into separate PRs:

lipgloss v1 -> v2 (possible as standalone)

  • Only import path change: github.com/charmbracelet/lipgloss -> charm.land/lipgloss/v2
  • No bubbletea dependency — can be done independently.
  • Creates an intermediate state where lipgloss v2 runs alongside bubbletea v1; this works but some rendering differences may appear until bubbletea is upgraded too.

bubbletea v1 -> v2 + bubbles v1 -> v2 (must stay together)

  • These are tightly coupled — bubbles v2 components are bubbletea v2 tea.Models. Upgrading one without the other causes type incompatibility. Cannot be separated.
  • This PR would contain the main API changes: View() returning tea.View, KeyPressMsg, WithAltScreen removal, viewport API changes, and the wrapper model fix.

huh v1 -> v2 (possible as standalone)

  • Huh v2 requires charm.land/lipgloss/v2 and charm.land/bubbletea/v2, so this must land after the first two.
  • Theme initialization and form API changes are already in separate commits (8310134 and e22a348).

Proposed split:

  1. feat: upgrade lipgloss to v2 — import path only
  2. feat: upgrade bubbletea and bubbles to v2 — API migration + wrapper model fix
  3. feat: upgrade huh to v2 — form/theme changes

This is feasible since the commits are already separated this way (bd7d04d = bubbletea+bubbles, 8310134 = huh). Creating the separate branches would involve cherry-picking and splitting the lipgloss changes out of bd7d04d. Let me know if you want me to proceed.

…a v2

The tablelist.Model wrapper caused a race condition in bubbletea v2 where
tea.Quit fired before View() completed its render cycle, resulting in
malformed or missing output.

Fix: remove the Model wrapper entirely. NewModel() now returns table.Model
directly, and a new Render() helper applies BaseStyle and returns a string.
All 41 view files updated to call fmt.Print(tablelist.Render(m)) instead of
running a tea.Program. Interactive selection views (using selection.NewModel)
are unaffected.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: marwan562 <mixing.gamer546@gmail.com>
@marwan562 marwan562 force-pushed the update_bubbleteaV2 branch from df61f7a to 0da05be Compare April 8, 2026 05:42
@marwan562
Copy link
Copy Markdown
Contributor Author

Hey @NucleoFusion, I've finished all the fixes, updates, and tests.

I also tested project list locally and it's working correctly now — output renders cleanly with no malformed results.
harbor-cli

Let me know if you'd like me to test anything else or if there's anything you want me to change before we move forward with splitting the PR!

@NucleoFusion
Copy link
Copy Markdown
Contributor

Lets keep this PR on hold for now
Since I want to discuss and perform some changes to the model before we migrate to v2. Which may allow us to not remove the model.

@marwan562
Copy link
Copy Markdown
Contributor Author

Well, if you need anything to update or change in this PR, we can go ahead and move forward on it. Just let me know 👍

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.

[update]: Upgrade CLI to support bubbletea v2

2 participants