[refactor] Delete duplicate InfiniteVirtualTable, migrate to @agenta/ui#5471
Draft
ardaerzin wants to merge 2 commits into
Draft
[refactor] Delete duplicate InfiniteVirtualTable, migrate to @agenta/ui#5471ardaerzin wants to merge 2 commits into
ardaerzin wants to merge 2 commits into
Conversation
Prep for retiring the duplicate InfiniteVirtualTable in oss/src/components (55 files, a strict subset of the 67-file @agenta/ui copy). The two copies had grown parallel abstractions for the same concept: the package's ExtendedColumn/ExtendedColumnProps and an ExtendedColumnType added to the OSS copy. Their bases were already equivalent (ColumnsType[number] IS ColumnType | ColumnGroupType), so the package type just gained the three props it lacked (defaultHidden, maxWidth, exportEnabled) and the OSS type is now an alias of it. Three files imported BOTH copies at once (TestcasesTableShell, getObservabilityColumns, getSessionColumns); each needed only the column type from the OSS side, so they now take it from the package. No file imports both copies anymore.
…ta/ui
The library existed twice: 55 files in oss/src/components/InfiniteVirtualTable
(a strict subset) and the canonical 67-file copy in @agenta/ui. Fixes to
either had to be applied to both — this session alone hit that with the
rowSelection.fixed widening and the ExtendedColumnType consolidation.
- Migrated all 33 app consumers from @/oss/components/InfiniteVirtualTable to
@agenta/ui/table (the barrel already re-exported every symbol they used).
- Deleted the 55-file OSS copy.
- Package exports ColumnVisibilityNodeMeta / ColumnVisibilityNodeMetaResolver
(needed by a migrated consumer).
Preserved the export permission gate. The retired OSS shell computed
exportEnabled = enableExport && canExportData internally; the package shell has
no permission check and defaults enableExport=true. The two consumers that
relied on the implicit gate (EvalRunDetails/Table.tsx,
EvaluationRunsTablePOC/.../EvaluationRunsTable/index.tsx) now pass
enableExport={canExportData} explicitly. tsc could not have caught this — both
sides are boolean.
TestsetsTable needed two behavior-preserving adjustments where the package
types are wider: String(record.key) at two Set<string>/string[] lookups (keys
are UUIDs, so a no-op) and 'as never' on the invariant datasetStore generic
(mirrors DeploymentsTable).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
InfiniteVirtualTableexisted twice: 55 files inoss/src/components/InfiniteVirtualTableand the canonical 67-file copy in@agenta/ui. The OSS copy was a strict subset (zero files it had that the package didn't; 21 of its 55 were byte-identical). Any fix to the table library had to be applied to both copies or they silently diverged. This session hit that twice on its own: therowSelection.fixedwidening and theExtendedColumnTypeconsolidation each had to be duplicated.This deletes the OSS copy and points its 33 consumers at the package.
Changes
@/oss/components/InfiniteVirtualTableto@agenta/ui/table. The package barrel already re-exported every symbol they used, so most were pure import-path swaps.ColumnVisibilityNodeMeta/ColumnVisibilityNodeMetaResolver, which one migrated consumer needs.Net: +93 / −10,073 lines.
The permission gate (the one thing tsc could not catch)
The retired OSS shell computed export visibility internally:
The package shell has no permission check and defaults
enableExport = true, expecting the consumer to gate it (its existing consumers already do). Two OSS consumers passed noenableExportand relied entirely on the OSS shell's implicit gate:EvalRunDetails/Table.tsxandEvaluationRunsTablePOC/.../EvaluationRunsTable/index.tsx. Migrating them as-is would have shown an export button to users without export permission, and both sides areboolean, so no type check would flag it. Both now passenableExport={canExportData}explicitly.TestsetsTable type reconciliations
Two spots where the package types are wider than the OSS copy's, both behavior-preserving:
String(record.key)at twoSet<string>/string[]lookups. The package row base typeskeyasReact.Key(string | number); testset keys are UUIDs, so the coercion is a runtime no-op.as neveron thedatasetStoreprop. The store generic is invariant on its ApiRow parameter; this mirrorsDeploymentsTable, an existing package consumer.Tests / notes
pnpm --filter @agenta/oss exec tsc --noEmitandpnpm --filter @agenta/ee exec tsc --noEmit: 0 errors each.pnpm turbo run build --filter=@agenta/uiandlint --filter=@agenta/oss: clean.What to QA
The migration is import-level, so the risk is behavioral parity of the two table implementations, concentrated on export permissions.