Skip to content

chore: simplify embedded frontend UI #4326

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

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

korniltsev
Copy link
Collaborator

@korniltsev korniltsev commented Jul 29, 2025

  • Remove a lot of unused UI code (authorization, api keys, adhoc, settings, tag explorer)
  • Remove some of UI (diff, compare), leave only single flame graph page. I assume not a lot of people use diff and compare , so I propose to sacrifice them.
  • Remove tests and test dependencies. I mostly propose to do this to have less dependencies and less "security" updates.
  • Drastically reduces the redux state, should simplify updates/migrations there in the future.

This draft is just an RFC - attempt to improve and simplify our maintenance of embedded frontend. Curious what others think of my assumptions 🧠 and sacrifices 🗡️

@simonswine
Copy link
Contributor

Yes, I would support this effort!

@korniltsev
Copy link
Collaborator Author

This is how it would look

Screenshot_20250729_161714

@korniltsev korniltsev marked this pull request as ready for review July 29, 2025 09:18
@korniltsev korniltsev requested a review from a team as a code owner July 29, 2025 09:18
@korniltsev korniltsev marked this pull request as draft July 29, 2025 09:38
korniltsev and others added 9 commits August 15, 2025 15:59
🤖 Generated with Claude Code

Co-Authored-By: Claude <[email protected]>
Reverts package.json, tsconfig.json, and yarn.lock to main branch state to resolve
React component type errors. The removal of Jest dependencies caused yarn to resolve
incompatible React type versions that broke JSX component compilation.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Successfully removes all Jest-related dependencies while maintaining working typecheck:
- Removed jest core package and all jest-* plugins (fetch-mock, environment-jsdom, css-modules-transform, canvas-mock, image-snapshot)
- Removed all @testing-library packages (react, jest-dom, dom, user-event, cypress)
- Removed jest-related type definitions (@types/jest, @types/jest-image-snapshot, @types/testing-library__jest-dom, @types/mocha)
- Removed SWC Jest integration (@swc/jest) and TypeScript Jest (ts-jest)
- Removed Cypress testing framework and related packages
- Removed testing-related ESLint plugins (eslint-plugin-jest, eslint-plugin-cypress)
- Removed canvas packages used by Jest (canvas, canvas-to-buffer)
- Removed test scripts ("test": "jest", "cy": "cypress")
- Updated tsconfig.json to remove @types/jest from types array

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
This restores all Jest testing dependencies and configuration
from the main branch before systematic removal.

Co-Authored-By: Claude <[email protected]>
SWC Jest transformer removed successfully.
Build and typecheck pass.

Co-Authored-By: Claude <[email protected]>
Testing Library for Cypress removed successfully.
Build and typecheck pass.

Co-Authored-By: Claude <[email protected]>
Testing Library DOM utilities removed successfully.
Build and typecheck pass.

Co-Authored-By: Claude <[email protected]>
Testing Library Jest DOM matchers removed successfully.
Build and typecheck pass.

Co-Authored-By: Claude <[email protected]>
Removed @testing-library/react, @testing-library/user-event,
and @types/testing-library__jest-dom successfully.
Build and typecheck pass.

Co-Authored-By: Claude <[email protected]>
korniltsev and others added 3 commits August 15, 2025 18:17
Removed jest, ts-jest, jest-environment-jsdom, canvas,
and canvas-to-buffer successfully.
Build and typecheck pass.

Co-Authored-By: Claude <[email protected]>
Removed @types/jest from both package.json and tsconfig.json,
plus jest-canvas-mock, jest-css-modules-transform,
jest-fetch-mock, and jest-image-snapshot.
Build and typecheck pass.

Co-Authored-By: Claude <[email protected]>
Removed final Jest and testing dependencies:
- cypress, eslint-plugin-cypress, eslint-plugin-jest
- @types/mocha
- test and cy scripts from package.json

All Jest dependencies successfully removed.
Frontend build and typecheck pass completely.

Co-Authored-By: Claude <[email protected]>
@korniltsev korniltsev force-pushed the korniltsev/frontend-vibe-coding branch from 01aacf0 to fcee814 Compare August 15, 2025 12:18
@korniltsev korniltsev marked this pull request as ready for review August 15, 2025 12:23
@korniltsev korniltsev requested a review from Copilot August 15, 2025 12:25
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request simplifies the embedded frontend UI by removing various features and dependencies. It eliminates unused UI sections including authorization, API keys, adhoc analysis, settings, tag explorer, diff/comparison views while retaining only the single flame graph page functionality. The changes also remove tests and test dependencies to reduce maintenance burden and security updates.

  • Removes authorization, API keys, adhoc, settings, and tag explorer functionality
  • Eliminates diff and comparison UI components, keeping only single flame graph view
  • Removes tests and test dependencies to reduce maintenance overhead

Reviewed Changes

Copilot reviewed 187 out of 211 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
public/app/services/apiKeys.ts Removes API key management service functions
public/app/services/annotations.ts Removes annotation service functionality
public/app/services/adhoc.ts Removes adhoc profiling service and file upload utilities
public/app/redux/useReduxQuerySync.ts Removes tracing and tag explorer state synchronization
public/app/redux/store.ts Simplifies Redux store by removing unused reducers
public/app/redux/reducers/user.ts Removes user management Redux state and actions
public/app/redux/reducers/ui.ts Removes sidebar state management
public/app/redux/reducers/tracing.ts Removes tracing view Redux state and thunks
public/app/redux/reducers/settings.ts Removes settings page Redux state management
public/app/redux/reducers/serviceDiscovery.ts Removes service discovery Redux state
public/app/redux/reducers/continuous/*.ts Removes tag explorer, comparison, diff, and annotation functionality
public/app/redux/reducers/adhoc.ts Removes adhoc profiling Redux state management
public/app/pages/urls.ts Removes URL constants for removed pages
public/app/pages/tagExplorer/components/*.tsx Removes tag explorer UI components
public/app/pages/routes.ts Removes route constants for removed views
public/app/pages/exemplars/*.tsx Removes exemplars view components
public/app/pages/adhoc/*.tsx Removes adhoc profiling UI components
public/app/pages/*.tsx Removes various page components and utilities

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

type TimelineState =
| { type: 'pristine'; timeline: Timeline }
| { type: 'loading'; timeline: Timeline }
| { type: 'reloading'; timeline: Timeline }
| { type: 'loaded'; timeline: Timeline; annotations: Annotation[] };
| { type: 'loaded'; timeline: Timeline };
Copy link
Preview

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

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

[nitpick] The timeline state type definition has inconsistent structure. The 'loaded' type should maintain the same structure as the other states for consistency.

Copilot uses AI. Check for mistakes.

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.

2 participants