This is an automated email from the ASF dual-hosted git repository. maximebeauchemin pushed a commit to branch js-to-ts in repository https://gitbox.apache.org/repos/asf/superset.git
commit 7c745ac6226ffe55ea1d8420e06ac30682f3d69c Author: Maxime Beauchemin <[email protected]> AuthorDate: Sun Sep 7 13:47:13 2025 -0700 refactor: clarify atomic migration strategy for core files + tests/mocks - Coordinators now target only core files (no tests/mocks) - Agents migrate core file + all related tests/mocks atomically as one unit - Updated commands and documentation to reflect atomic migration workflow - Clear separation of concerns: coordinators identify, agents execute atomically π€ Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> --- .claude/commands/js-to-ts.md | 135 ++++++++++ .claude/projects/js-to-ts/INSTRUCTIONS.md | 405 ++++++++++++++++++++++++++++++ .claude/projects/js-to-ts/PROJECT.md | 185 ++++++++++++++ 3 files changed, 725 insertions(+) diff --git a/.claude/commands/js-to-ts.md b/.claude/commands/js-to-ts.md new file mode 100644 index 0000000000..ff9c0ab36a --- /dev/null +++ b/.claude/commands/js-to-ts.md @@ -0,0 +1,135 @@ +# JavaScript to TypeScript Migration Command + +Atomically migrate a core JS/JSX file to TypeScript along with all related tests and mocks. + +## Usage +``` +/js-to-ts <core-filename> +``` +- `<core-filename>` - Path to CORE file relative to `superset-frontend/` (e.g., `src/utils/common.js`, `src/middleware/loggerMiddleware.js`) +- **CORE FILES ONLY**: No test files, mock files - agent will find and migrate related files automatically + +--- + +## Agent Instructions + +**Role:** Atomic migration unit - migrate the core file + ALL related tests/mocks as one cohesive unit. Use `git mv` to preserve history, NO `git commit`. NO global import changes. Report to coordinator for integration. + +**Technical Reference:** Follow patterns in [.claude/projects/js-to-ts/INSTRUCTIONS.md](../projects/js-to-ts/INSTRUCTIONS.md) for type organization, migration recipes, and quality gates. + +**Atomic Migration Scope:** +For core file `src/utils/example.js`, also migrate: +- `src/utils/example.test.js` / `src/utils/example.test.jsx` +- `src/utils/example.spec.js` / `src/utils/example.spec.jsx` +- `src/utils/__mocks__/example.js` +- Any other related test/mock files found by pattern matching + +### Step 0: Dependency Check (MANDATORY) + +**Command:** +```bash +grep -E "from '\.\./.*\.jsx?'|from '\./.*\.jsx?'|from 'src/.*\.jsx?'" superset-frontend/{filename} +``` + +**Decision:** +- β No matches β Proceed with atomic migration (core + tests + mocks) +- β Matches found β EXIT with dependency report + +### Step 1: Identify Related Files (REQUIRED) + +**Find all related test and mock files:** +```bash +# Pattern-based search for related files +basename=$(basename {filename} .js) +dirname=$(dirname superset-frontend/{filename}) + +# Find test files +find "$dirname" -name "${basename}.test.js" -o -name "${basename}.test.jsx" +find "$dirname" -name "${basename}.spec.js" -o -name "${basename}.spec.jsx" + +# Find mock files +find "$dirname" -name "__mocks__/${basename}.js" +find "$dirname" -name "${basename}.mock.js" +``` + +**Migration Requirement:** All discovered related files MUST be migrated together as one atomic unit. + +### Success Report Format +``` +SUCCESS: Atomic Migration of {core-filename} + +## Files Migrated (Atomic Unit) +- Core: {core-filename} β {core-filename.ts/tsx} +- Tests: {list-of-test-files} β {list-of-test-files.ts/tsx} +- Mocks: {list-of-mock-files} β {list-of-mock-files.ts} +- Type files modified: {list-of-type-files} + +## Types Created/Improved +- {TypeName}: {location} ({scope}) - {rationale} +- {ExistingType}: enhanced in {location} - {improvement-description} + +## Documentation Recommendations +- ADD_TO_DIRECTORY: {TypeName} - {reason} +- NO_DOCUMENTATION: {TypeName} - {reason} + +## Quality Validation +- File-level TypeScript compilation: β PASS +- Zero any types: β PASS +- Local imports resolved: β PASS +- Functionality preserved: β PASS +- Tests pass (if test file): β PASS +- Coordinator action required: {YES/NO} + +## Migration Learnings +- Type conflicts encountered: {describe any multiple type definitions} +- Mock patterns used: {describe test mocking approaches} +- Import hierarchy decisions: {note authoritative type sources used} + +## Improvement Suggestions for Documentation +- INSTRUCTIONS.md enhancement: {suggest additions to migration guide} +- Common pattern identified: {note reusable patterns for future migrations} +``` + +### Dependency Block Report Format +``` +DEPENDENCY_BLOCK: Cannot migrate {filename} + +## Blocking Dependencies +- {path}: {type} - {usage} - {priority} + +## Impact Analysis +- Estimated types: {number} +- Expected locations: {list} +- Cross-domain: {YES/NO} + +## Recommended Order +{ordered-list} +``` + +### Agent Constraints (CRITICAL) +1. **Use git mv** - Run `git mv file.js file.ts` to preserve git history, but NO `git commit` +2. **NO global import changes** - Don't update imports across codebase +3. **Type files OK** - Can modify existing type files to improve/align types +4. **Targeted validation** - Run `tsc --noEmit` on modified files only +5. Zero `any` types - use proper TypeScript types +6. Search existing types before creating new ones +7. Follow patterns from INSTRUCTIONS.md + +--- + +## Coordinator Actions + +### Global Integration (Coordinator Only) +When agents report `SUCCESS`: +- Review agent's type improvements for consistency +- Agent already used `git mv` to preserve history +- Update imports across codebase if needed +- Run full TypeScript compilation +- Update Type Reference Map in `INSTRUCTIONS.md` +- Merge any type file conflicts + +### Linear Scheduling +When agents report `DEPENDENCY_BLOCK`: +- Queue dependencies in linear order +- Process one file at a time to avoid conflicts +- Handle cascading type changes between files diff --git a/.claude/projects/js-to-ts/INSTRUCTIONS.md b/.claude/projects/js-to-ts/INSTRUCTIONS.md new file mode 100644 index 0000000000..0d661999d3 --- /dev/null +++ b/.claude/projects/js-to-ts/INSTRUCTIONS.md @@ -0,0 +1,405 @@ +# TypeScript Migration Guide for Apache Superset + +Comprehensive reference for converting JavaScript/JSX files to TypeScript/TSX in Apache Superset frontend. + +## π― Migration Principles + +1. **Atomic migration units** - Core file + all related tests/mocks migrate together +2. **Zero `any` types** - Use proper TypeScript throughout +3. **Leverage existing types** - Reuse established definitions +4. **Type inheritance** - Derivatives extend base component types +5. **Strategic placement** - File types for maximum discoverability +6. **Surgical improvements** - Enhance existing types during migration + +## βοΈ Atomic Migration Strategy + +**Unit of Work**: Each migration includes: +- **1 core file** (production code) +- **All related test files** (*.test.js/jsx, *.spec.js/jsx) +- **All related mock files** (__mocks__/, *.mock.js) + +**Type Consistency**: Tests and mocks must use the same types as the core file to prevent type fragmentation. + +--- + +## πΊοΈ Type Reference Map + +### From `@superset-ui/core` +```typescript +// Data & Query +QueryFormData, QueryData, JsonObject, AnnotationData, AdhocMetric +LatestQueryFormData, GenericDataType, DatasourceType, ExtraFormData +DataMaskStateWithId, NativeFilterScope, NativeFiltersState, NativeFilterTarget + +// UI & Theme +FeatureFlagMap, LanguagePack, ColorSchemeConfig, SequentialSchemeConfig +``` + +### From `@superset-ui/chart-controls` +```typescript +Dataset, ColumnMeta, ControlStateMapping +``` + +### From Local Types (`src/types/`) +```typescript +// Authentication +User, UserWithPermissionsAndRoles, BootstrapUser, PermissionsAndRoles + +// Dashboard +Dashboard, DashboardState, DashboardInfo, DashboardLayout, LayoutItem +ComponentType, ChartConfiguration, ActiveFilters + +// Charts +Chart, ChartState, ChartStatus, ChartLinkedDashboard, Slice, SaveActionType + +// Data +Datasource, Database, Owner, Role + +// UI Components +TagType, FavoriteStatus, Filter, ImportResourceName +``` + +### From Domain Types +```typescript +// src/dashboard/types.ts +RootState, ChartsState, DatasourcesState, FilterBarOrientation +ChartCrossFiltersConfig, ActiveTabs, MenuKeys + +// src/explore/types.ts +ExplorePageInitialData, ExplorePageState, ExploreResponsePayload, OptionSortType + +// src/SqlLab/types.ts +[SQL Lab specific types] +``` + +--- + +## ποΈ Type Organization Strategy + +### Type Placement Hierarchy + +1. **Component-Colocated** (90% of cases) + ```typescript + // Same file as component + interface MyComponentProps { + title: string; + onClick: () => void; + } + ``` + +2. **Feature-Shared** + ```typescript + // src/[domain]/components/[Feature]/types.ts + export interface FilterConfiguration { + filterId: string; + targets: NativeFilterTarget[]; + } + ``` + +3. **Domain-Wide** + ```typescript + // src/[domain]/types.ts + export interface ExploreFormData extends QueryFormData { + viz_type: string; + } + ``` + +4. **Global** + ```typescript + // src/types/[TypeName].ts + export interface ApiResponse<T> { + result: T; + count?: number; + } + ``` + +### Type Discovery Commands +```bash +# Search existing types before creating +find superset-frontend/src -name "types.ts" -exec grep -l "[TypeConcept]" {} \; +grep -r "interface.*Props\|type.*Props" superset-frontend/src/ +``` + +### Derivative Component Patterns + +**Rule:** Components that extend others should extend their type interfaces. + +```typescript +// β Base component type +interface SelectProps { + value: string | number; + options: SelectOption[]; + onChange: (value: string | number) => void; + disabled?: boolean; +} + +// β Derivative extends base +interface ChartSelectProps extends SelectProps { + charts: Chart[]; + onChartSelect: (chart: Chart) => void; +} + +// β Derivative with modified props +interface DatabaseSelectProps extends Omit<SelectProps, 'value' | 'onChange'> { + value: number; // Narrowed type + onChange: (databaseId: number) => void; // Specific signature +} +``` + +**Common Patterns:** +- **Extension:** `extends BaseProps` - adds new props +- **Omission:** `Omit<BaseProps, 'prop'>` - removes props +- **Modification:** `Omit<BaseProps, 'prop'> & { prop: NewType }` - changes prop type +- **Restriction:** Override with narrower types (union β specific) + +--- + +## π Migration Recipe + +### Step 0: Dependency Check (MANDATORY) +```bash +grep -E "from '\.\./.*\.jsx?'|from '\./.*\.jsx?'|from 'src/.*\.jsx?'" [target-file] +``` +- No matches β Proceed +- Matches found β Report dependencies and exit + +### Step 1: File Conversion (Agent-Safe) +```bash +# Create TypeScript file alongside original (coordinator handles git mv) +cp component.js component.ts # Agent creates, coordinator moves +cp Component.jsx Component.tsx +``` + +### Step 2: Import & Type Setup +```typescript +// Import order (enforced by linting) +import React, { FC, ReactNode } from 'react'; +import { JsonObject, QueryFormData } from '@superset-ui/core'; +import { Dataset } from '@superset-ui/chart-controls'; +import type { Dashboard } from 'src/types/Dashboard'; +``` + +### Step 3: Function & Component Typing +```typescript +// Functions with proper parameter/return types +export function processData( + data: Dataset[], + config: JsonObject +): ProcessedData[] { + // implementation +} + +// Component props with inheritance +interface ComponentProps extends BaseProps { + data: Chart[]; + onSelect: (id: number) => void; +} + +const Component: FC<ComponentProps> = ({ data, onSelect }) => { + // implementation +}; +``` + +### Step 4: State & Redux Typing +```typescript +// Hooks with specific types +const [data, setData] = useState<Chart[]>([]); +const [selected, setSelected] = useState<number | null>(null); + +// Redux with existing RootState +const mapStateToProps = (state: RootState) => ({ + charts: state.charts, + user: state.user, +}); +``` + +--- + +## π¨ Anti-Patterns to Avoid + +```typescript +// β Never use any +const obj: any = {}; + +// β Use proper types +const obj: Record<string, JsonObject> = {}; + +// β Don't recreate base component props +interface ChartSelectProps { + value: string; // Duplicated from SelectProps + onChange: () => void; // Duplicated from SelectProps + charts: Chart[]; // New prop +} + +// β Inherit and extend +interface ChartSelectProps extends SelectProps { + charts: Chart[]; // Only new props +} + +// β Don't create duplicate types +interface UserInfo { + name: string; + email: string; +} + +// β Extend existing types +import { User } from 'src/types/bootstrapTypes'; +type UserDisplayInfo = Pick<User, 'firstName' | 'lastName' | 'email'>; +``` + +--- + +## π§ͺ Test File Migration Patterns + +### Test File Priority +- **Always migrate test files** alongside production files +- **Test files are often leaf nodes** - good starting candidates +- **Create tests if missing** - Leverage new TypeScript types for better test coverage + +### Test-Specific Type Patterns +```typescript +// Mock interfaces for testing +interface MockStore { + getState: () => Partial<RootState>; // Partial allows minimal mocking +} + +// Type-safe mocking for complex objects +const mockDashboardInfo: Partial<DashboardInfo> as DashboardInfo = { + id: 123, + json_metadata: '{}', +}; + +// Sinon stub typing +let postStub: sinon.SinonStub; +beforeEach(() => { + postStub = sinon.stub(SupersetClient, 'post'); +}); + +// Use stub reference instead of original method +expect(postStub.callCount).toBe(1); +expect(postStub.getCall(0).args[0].endpoint).toMatch('/api/'); +``` + +### Test Migration Recipe +1. **Migrate production file first** (if both need migration) +2. **Update test imports** to point to `.ts/.tsx` files +3. **Add proper mock typing** using `Partial<T> as T` pattern +4. **Fix stub typing** - Use stub references, not original methods +5. **Verify all tests pass** with TypeScript compilation + +--- + +## π§ Type Conflict Resolution + +### Multiple Type Definitions Issue +**Problem**: Same type name defined in multiple files causes compilation errors. + +**Example**: `DashboardInfo` defined in both: +- `src/dashboard/reducers/types.ts` (minimal) +- `src/dashboard/components/Header/types.ts` (different shape) +- `src/dashboard/types.ts` (complete - used by RootState) + +### Resolution Strategy +1. **Identify the authoritative type**: + ```bash + # Find which type is used by RootState/main interfaces + grep -r "DashboardInfo" src/dashboard/types.ts + ``` + +2. **Use import from authoritative source**: + ```typescript + // β Import from main domain types + import { RootState, DashboardInfo } from 'src/dashboard/types'; + + // β Don't import from component-specific files + import { DashboardInfo } from 'src/dashboard/components/Header/types'; + ``` + +3. **Mock complex types in tests**: + ```typescript + // For testing - provide minimal required fields + const mockInfo: Partial<DashboardInfo> as DashboardInfo = { + id: 123, + json_metadata: '{}', + // Only provide fields actually used in test + }; + ``` + +### Type Hierarchy Discovery Commands +```bash +# Find all definitions of a type +grep -r "interface.*TypeName\|type.*TypeName" src/ + +# Find import usage patterns +grep -r "import.*TypeName" src/ + +# Check what RootState uses +grep -A 10 -B 10 "TypeName" src/*/types.ts +``` + +--- + +## π Coordinator Integration Patterns + +### Side-Effect Categories + +**Automatic Integration** β (No coordinator action needed): +- TypeScript compilation passes immediately +- No import updates needed across codebase + +**Coordinator Integration** π§ (Common - expect this): +- TypeScript compilation fails BUT subagent work is high quality +- Type import conflicts need resolution +- Mock objects need complete type satisfaction +- **Action**: Fix type conflicts, maintain subagent's good work + +**Rollback Required** β (Rare): +- Subagent introduced `any` types or poor patterns +- Fundamental approach needs rework + +### Integration Workflow +1. **Review subagent's types** - Check for `any`, proper inheritance, good placement +2. **Run TypeScript compilation** - `npm run type` +3. **Resolve type conflicts**: + - Import from authoritative sources + - Complete mock objects with proper typing + - Fix stub method references +4. **Verify tests still pass** - `npm test -- filename.test.ts` +5. **Commit with git history** - Git usually auto-detects renames + +--- + +## π― Quality Gates + +**Before completing migration:** +- [ ] TypeScript compilation passes +- [ ] Zero `any` types used +- [ ] All imports resolved +- [ ] Functionality unchanged +- [ ] Types placed in discoverable locations +- [ ] Existing types reused where possible + +--- + +## π Quick Reference + +**Type Utilities:** +- `Record<K, V>` - Object with specific key/value types +- `Partial<T>` - All properties optional +- `Pick<T, K>` - Subset of properties +- `Omit<T, K>` - Exclude specific properties +- `NonNullable<T>` - Exclude null/undefined + +**Event Types:** +- `MouseEvent<HTMLButtonElement>` +- `ChangeEvent<HTMLInputElement>` +- `FormEvent<HTMLFormElement>` + +**React Types:** +- `FC<Props>` - Functional component +- `ReactNode` - Any renderable content +- `CSSProperties` - Style objects + +--- + +**Remember:** Every type should add value and clarity. The goal is meaningful type safety that catches bugs and improves developer experience. diff --git a/.claude/projects/js-to-ts/PROJECT.md b/.claude/projects/js-to-ts/PROJECT.md new file mode 100644 index 0000000000..e8729e7d86 --- /dev/null +++ b/.claude/projects/js-to-ts/PROJECT.md @@ -0,0 +1,185 @@ +# JavaScript to TypeScript Migration Project + +Apache Superset frontend migration from 219 JS/JSX files to TypeScript with dependency-aware coordination. + +## π Migration Overview + +**Scope**: 219 files total (112 JS + 107 JSX) +- Production files: 139 (63%) +- Test files: 80 (37%) + +**Key Directories**: +- Dashboard: 118 files (54% of total) +- Explore: 60 files (27% of total) +- Components: 19 files (9% of total) +- Utils: 8 files (4% of total) + +## π― Migration Commands + +### Agent Command +```bash +/js-to-ts <core-filename> +``` +**Agent Role**: Atomic migration of core file + all related tests/mocks. Use `git mv`, NO global changes. Coordinator reviews and commits. See [../../commands/js-to-ts.md](../../commands/js-to-ts.md) + +**Migration Patterns**: Reference [.claude/projects/js-to-ts/INSTRUCTIONS.md](./INSTRUCTIONS.md) for type organization, inheritance patterns, and quality gates. + +### Coordinator Responsibilities + +#### 1. Core File Selection Strategy +**Target ONLY Core Files**: Coordinators identify core files (production code), agents handle related tests/mocks atomically. + +**File Analysis Commands**: +```bash +# Find CORE files with no JS/JSX dependencies (exclude tests/mocks) +find superset-frontend/src -name "*.js" -o -name "*.jsx" | grep -v "test\|spec\|mock" | head -10 + +# Check dependencies for core files only +for file in <core-files>; do + grep -E "from '\.\./.*\.jsx?'|from '\./.*\.jsx?'|from 'src/.*\.jsx?'" "$file" || echo "LEAF CANDIDATE: $file" +done + +# Identify heavily imported files (migrate last) +grep -r "from.*utils/common" superset-frontend/src/ | wc -l +``` + +**Priority Order**: +1. **Core leaf files** - No JS/JSX imports, only external libraries (utils, middleware) +2. **Core utility files with minimal dependencies** +3. **Core component files importing only already-migrated files** +4. **Core foundational files** (utils/common.js, controls.jsx) - migrate last + +**Migration Unit**: Each agent call migrates: +- 1 core file (primary target) +- All related `*.test.js/jsx` files +- All related `*.mock.js` files +- All related `__mocks__/` files + +#### 2. Agent Control Workflow +**For Each File**: +1. **Execute**: `/js-to-ts <filename>` +2. **Review**: Evaluate agent's work against criteria +3. **Test**: Run `npm run type` for global TypeScript compilation +4. **Gate**: Decide push forward or rollback based on evaluation + +**Agent SUCCESS Report Review**: +- β **Type Usage**: Proper types used, no `any` types +- β **Type Filing**: Types placed in correct hierarchy (component β feature β domain β global) +- β **Side Effects**: No unintended changes to other files +- β **Import Alignment**: Proper .ts/.tsx import extensions + +#### 3. Integration Decision Framework + +**Automatic Integration** β : +- `npm run type` passes without errors +- Agent created clean TypeScript with proper types +- Types appropriately filed in hierarchy + +**Coordinator Integration** (Fix Side-Effects) π§: +- `npm run type` fails BUT agent's work is high quality +- Good type usage, proper patterns, well-organized +- Side-effects are manageable TypeScript compilation errors +- **Coordinator Action**: Integrate the change, then fix global compilation issues + +**Rollback Only** β: +- Agent introduced `any` types or poor type choices +- Types poorly organized or conflicting with existing patterns +- Fundamental approach issues requiring complete rework + +**Integration Process**: +1. **Review**: Agent already used `git mv` to preserve history +2. **Fix Side-Effects**: Update dependent files with proper import extensions +3. **Resolve Types**: Fix any cascading type issues across codebase +4. **Validate**: Ensure `npm run type` passes after fixes + +### Learned Integration Patterns + +**Common Side-Effects (Expect These)**: +- **Type import conflicts**: Multiple definitions of same type name +- **Mock object typing**: Tests need complete type satisfaction +- **Stub method references**: Use stub vars instead of original methods + +**Coordinator Fixes (Standard Process)**: +1. **Import Resolution**: + ```bash + # Find authoritative type source + grep -r "TypeName" src/*/types.ts + # Import from domain types (src/dashboard/types.ts) not component types + ``` + +2. **Test Mock Completion**: + ```typescript + // Use Partial<T> as T pattern for minimal mocking + const mockDashboard: Partial<DashboardInfo> as DashboardInfo = { + id: 123, + json_metadata: '{}', + }; + ``` + +3. **Stub Reference Fixes**: + ```typescript + // β Use stub variable + expect(postStub.callCount).toBe(1); + // β Don't use original method + expect(SupersetClient.post.callCount).toBe(1); + ``` + +4. **Validation Commands**: + ```bash + npm run type # TypeScript compilation + npm test -- filename # Test functionality + git status # Should show rename, not add/delete + ``` + +## π― File Categories for Coordinator + +### Leaf Files (Start Here) +**Self-contained files with minimal JS/JSX dependencies**: +- Test files (80 files) - Usually only import the file being tested +- Utility files without internal dependencies +- Components importing only external libraries + +### Heavily Imported Files (Migrate Last) +**Core files that many others depend on**: +- `utils/common.js` - Core utility functions +- `utils/reducerUtils.js` - Redux helpers +- `@superset-ui/core` equivalent files +- Major state management files (`explore/store.js`, `dashboard/actions/`) + +### Complex Components (Middle Priority) +**Large files requiring careful type analysis**: +- `components/Datasource/DatasourceEditor.jsx` (1,809 lines) +- `explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx` (1,031 lines) +- `explore/components/ExploreViewContainer/index.jsx` (911 lines) + +## π Coordinator Success Metrics + +**Per-File Gates**: +- β `npm run type` passes after each migration +- β Zero `any` types introduced +- β All imports properly typed +- β Types filed in correct hierarchy + +**Overall Progress**: +- Track leaf-to-core migration progress +- Document rollback reasons for learning +- Maintain 100% TypeScript compilation throughout +- Ensure type safety improves with each successful migration + +## π Continuous Improvement Process + +**After Each Migration**: +1. **Update guides** with new patterns discovered +2. **Document coordinator fixes** that become common +3. **Enhance subagent instructions** based on recurring issues +4. **Track success metrics** - automatic vs coordinator integration rates + +**Guide Enhancement Areas**: +- **INSTRUCTIONS.md**: Type patterns, conflict resolution, test strategies +- **js-to-ts command**: Success report format, learning capture +- **PROJECT.md**: Integration workflows, side-effect patterns + +**Success Indicators**: +- Higher automatic integration rate (fewer coordinator fixes needed) +- Faster coordinator resolution of common issues +- Improved subagent type quality and placement decisions
