codeant-ai-for-open-source[bot] commented on code in PR #37499: URL: https://github.com/apache/superset/pull/37499#discussion_r2733955683
########## superset-frontend/packages/superset-core/src/api/editors.ts: ########## @@ -0,0 +1,381 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * @fileoverview Editors API for Superset extension editor contributions. + * + * This module defines the interfaces and types for editor contributions to the + * Superset platform. Extensions can register custom text editor implementations + * (e.g., Monaco, CodeMirror) through the extension manifest, replacing the + * default Ace editor for specific languages. + */ + +import { ForwardRefExoticComponent, RefAttributes } from 'react'; +import { EditorContribution, EditorLanguage } from './contributions'; +import { Disposable, Event } from './core'; +import type { SupersetTheme } from '../ui'; + +// Re-export contribution types for convenience +export type { EditorContribution, EditorLanguage }; + +/** + * Represents a position in the editor (line and column). + */ +export interface Position { + /** Zero-based line number */ + line: number; + /** Zero-based column number */ + column: number; +} + +/** + * Represents a range in the editor with start and end positions. + */ +export interface Range { + /** Start position of the range */ + start: Position; + /** End position of the range */ + end: Position; +} + +/** + * Represents a selection in the editor. + */ +export interface Selection extends Range { + /** Direction of the selection */ + direction?: 'ltr' | 'rtl'; +} + +/** + * Annotation severity levels for editor markers. + */ +export type AnnotationSeverity = 'error' | 'warning' | 'info'; + +/** + * Represents an annotation (marker/diagnostic) in the editor. + */ +export interface EditorAnnotation { + /** Zero-based line number */ + line: number; + /** Zero-based column number (optional) */ + column?: number; + /** Annotation message to display */ + message: string; + /** Severity level of the annotation */ + severity: AnnotationSeverity; + /** Optional source of the annotation (e.g., "linter", "typescript") */ + source?: string; +} + +/** + * Represents a keyboard shortcut binding. + */ +export interface EditorHotkey { + /** Unique name for the hotkey command */ + name: string; + /** Key binding string (e.g., "Ctrl+Enter", "Alt+Enter") */ + key: string; + /** Description of what the hotkey does */ + description?: string; + /** Function to execute when the hotkey is triggered */ + exec: (handle: EditorHandle) => void; +} + +/** + * Completion item kinds for autocompletion. + */ +export type CompletionItemKind = + | 'text' + | 'method' + | 'function' + | 'constructor' + | 'field' + | 'variable' + | 'class' + | 'interface' + | 'module' + | 'property' + | 'unit' + | 'value' + | 'enum' + | 'keyword' + | 'snippet' + | 'color' + | 'file' + | 'reference' + | 'folder' + | 'constant' + | 'struct' + | 'event' + | 'operator' + | 'typeParameter' + | 'table' + | 'column' + | 'schema' + | 'catalog' + | 'database'; + +/** + * Represents a completion item for autocompletion. + */ +export interface CompletionItem { + /** Display label for the completion item */ + label: string; + /** Text to insert when the item is selected */ + insertText: string; + /** Kind of completion item for icon display */ + kind: CompletionItemKind; + /** Optional documentation to show in the completion popup */ + documentation?: string; + /** Optional detail text to show alongside the label */ + detail?: string; + /** Sorting priority (higher numbers appear first) */ + sortText?: string; + /** Text used for filtering completions */ + filterText?: string; +} + +/** + * Context provided to completion providers. + */ +export interface CompletionContext { + /** Character that triggered the completion (if any) */ + triggerCharacter?: string; + /** How the completion was triggered */ + triggerKind: 'invoke' | 'automatic'; + /** Language of the editor */ + language: EditorLanguage; + /** Generic metadata passed from the host (e.g., SQL Lab can pass database context) */ + metadata?: Record<string, unknown>; +} + +/** + * Provider interface for dynamic completions. + */ +export interface CompletionProvider { + /** Unique identifier for this provider */ + id: string; + /** Trigger characters that invoke this provider (e.g., '.', ' ') */ + triggerCharacters?: string[]; + /** + * Provide completions at the given position. + * @param content The editor content + * @param position The cursor position + * @param context Completion context with trigger info and metadata + * @returns Array of completion items or a promise that resolves to them + */ + provideCompletions( + content: string, + position: Position, + context: CompletionContext, + ): CompletionItem[] | Promise<CompletionItem[]>; +} + +/** + * A keyword for editor autocomplete. + * This is a generic format that editor implementations convert to their native format. + */ +export interface EditorKeyword { + /** Display name of the keyword */ + name: string; + /** Value to insert when selected (defaults to name if not provided) */ + value?: string; + /** Category/type of the keyword (e.g., "column", "table", "function") */ + meta?: string; + /** Optional score for sorting (higher = more relevant) */ + score?: number; +} + +/** + * Props that all editor implementations must accept. + */ +export interface EditorProps { + /** Instance identifier */ + id: string; + /** Controlled value */ + value: string; + /** Content change handler */ + onChange: (value: string) => void; + /** Blur handler */ + onBlur?: (value: string) => void; + /** Cursor position change handler */ + onCursorPositionChange?: (pos: Position) => void; + /** Selection change handler */ + onSelectionChange?: (sel: Selection[]) => void; + /** Language mode for syntax highlighting */ + language: EditorLanguage; + /** Whether the editor is read-only */ + readOnly?: boolean; + /** Tab size in spaces */ + tabSize?: number; + /** Whether to show line numbers */ + lineNumbers?: boolean; + /** Whether to enable word wrap */ + wordWrap?: boolean; + /** Linting/error annotations */ + annotations?: EditorAnnotation[]; + /** Keyboard shortcuts */ + hotkeys?: EditorHotkey[]; + /** Static keywords for autocomplete */ + keywords?: EditorKeyword[]; + /** CSS height (e.g., "100%", "500px") */ + height?: string; + /** CSS width (e.g., "100%", "800px") */ + width?: string; + /** Callback when editor is ready with imperative handle */ + onReady?: (handle: EditorHandle) => void; + /** Host-specific context (e.g., database info from SQL Lab) */ + metadata?: Record<string, unknown>; + /** Theme object for styling the editor */ + theme?: SupersetTheme; +} + +/** + * Imperative API for controlling the editor programmatically. + */ +export interface EditorHandle { + /** Focus the editor */ + focus(): void; + /** Get the current editor content */ + getValue(): string; + /** Set the editor content */ + setValue(value: string): void; + /** Get the current cursor position */ + getCursorPosition(): Position; + /** Move the cursor to a specific position */ + moveCursorToPosition(position: Position): void; + /** Get all selections in the editor */ + getSelections(): Selection[]; + /** Set the selection range */ + setSelection(selection: Range): void; + /** Get the selected text */ + getSelectedText(): string; + /** Insert text at the current cursor position */ + insertText(text: string): void; + /** Execute a named editor command */ + executeCommand(commandName: string): void; + /** Scroll to a specific line */ + scrollToLine(line: number): void; + /** Set annotations (replaces existing) */ + setAnnotations(annotations: EditorAnnotation[]): void; + /** Clear all annotations */ + clearAnnotations(): void; + /** + * Register a completion provider for dynamic suggestions. + * @param provider The completion provider to register + * @returns A Disposable to unregister the provider + */ + registerCompletionProvider(provider: CompletionProvider): Disposable; +} + +/** + * React component type for editor implementations. + * Must be a forwardRef component to expose the EditorHandle. + */ +export type EditorComponent = ForwardRefExoticComponent< + EditorProps & RefAttributes<EditorHandle> +>; + +/** + * A registered editor provider with its contribution metadata and component. + */ +export interface EditorProvider { + /** The editor contribution metadata */ + contribution: EditorContribution; + /** The React component implementing the editor */ + component: EditorComponent; +} + +/** + * Event fired when an editor provider is registered. + */ +export interface EditorProviderRegisteredEvent { + /** The registered provider */ + provider: EditorProvider; +} + +/** + * Event fired when an editor provider is unregistered. + */ +export interface EditorProviderUnregisteredEvent { + /** The contribution that was unregistered */ + contribution: EditorContribution; +} + +/** + * Register an editor provider for specific languages. + * When an extension registers an editor, it replaces the default for those languages. + * + * @param contribution The editor contribution metadata from extension.json + * @param component The React component implementing EditorProps + * @returns A Disposable to unregister the provider + * + * @example + * ```typescript + * const disposable = registerEditorProvider( + * { + * id: 'acme.monaco-sql', + * name: 'Monaco SQL Editor', + * languages: ['sql'], + * }, + * MonacoSQLEditor + * ); + * context.disposables.push(disposable); + * ``` + */ +export declare function registerEditorProvider( + contribution: EditorContribution, + component: EditorComponent, +): Disposable; + +/** + * Get the editor provider for a specific language. + * Returns the extension's editor if registered, otherwise undefined. + * + * @param language The language to get an editor for + * @returns The editor provider or undefined if no extension provides one + */ +export declare function getEditorProvider( + language: EditorLanguage, +): EditorProvider | undefined; + +/** + * Check if an extension has registered an editor for a language. + * + * @param language The language to check + * @returns True if an extension provides an editor for this language + */ +export declare function hasEditorProvider(language: EditorLanguage): boolean; + +/** + * Get all registered editor providers. + * + * @returns Array of all registered editor providers + */ +export declare function getAllEditorProviders(): EditorProvider[]; + +/** + * Event fired when an editor provider is registered. + */ +export declare const onDidRegisterEditorProvider: Event<EditorProviderRegisteredEvent>; + +/** + * Event fired when an editor provider is unregistered. + */ +export declare const onDidUnregisterEditorProvider: Event<EditorProviderUnregisteredEvent>; Review Comment: **Suggestion:** The file only declares the editor API functions and events (using `export declare`) but does not provide any runtime implementation; importing and calling these functions at runtime will be undefined and cause TypeError exceptions. Provide a minimal in-memory implementation (registry + simple event listeners) so the API is usable at runtime and the returned Disposable actually unregisters the provider. [possible bug] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ Extension editor registration completely broken at runtime. - ❌ EditorHost cannot resolve extension-provided editors. - ⚠️ SQL Lab editor switching fails for extensions. - ⚠️ Any extension relying on these APIs crashes. ``` </details> ```suggestion // In-memory provider registry and simple event/listener implementation. const _providers: EditorProvider[] = []; const _onDidRegisterListeners: Array<(e: EditorProviderRegisteredEvent) => void> = []; const _onDidUnregisterListeners: Array<(e: EditorProviderUnregisteredEvent) => void> = []; export function registerEditorProvider( contribution: EditorContribution, component: EditorComponent, ): Disposable { const provider: EditorProvider = { contribution, component }; _providers.push(provider); // Notify listeners _onDidRegisterListeners.slice().forEach(l => l({ provider })); // Return a disposable which removes this provider and notifies unregistered listeners return { dispose() { const idx = _providers.indexOf(provider); if (idx >= 0) { _providers.splice(idx, 1); _onDidUnregisterListeners.slice().forEach(l => l({ contribution })); } }, } as Disposable; } /** * Get the editor provider for a specific language. * Returns the extension's editor if registered, otherwise undefined. */ export function getEditorProvider(language: EditorLanguage): EditorProvider | undefined { // Find last-registered provider that supports the language (last wins) for (let i = _providers.length - 1; i >= 0; i -= 1) { const p = _providers[i]; // contributions typically declare `languages`; guard with any to avoid strict type assumptions const langs = (p.contribution as any).languages as EditorLanguage[] | undefined; if (langs && langs.includes(language)) { return p; } } return undefined; } export function hasEditorProvider(language: EditorLanguage): boolean { return !!getEditorProvider(language); } export function getAllEditorProviders(): EditorProvider[] { return _providers.slice(); } // Simple Event<T> implementation compatible with common patterns (subscribe returns Disposable) export const onDidRegisterEditorProvider: Event<EditorProviderRegisteredEvent> = (listener) => { _onDidRegisterListeners.push(listener); return { dispose() { const idx = _onDidRegisterListeners.indexOf(listener); if (idx >= 0) _onDidRegisterListeners.splice(idx, 1); }, } as Disposable; }; export const onDidUnregisterEditorProvider: Event<EditorProviderUnregisteredEvent> = (listener) => { _onDidUnregisterListeners.push(listener); return { dispose() { const idx = _onDidUnregisterListeners.indexOf(listener); if (idx >= 0) _onDidUnregisterListeners.splice(idx, 1); }, } as Disposable; }; ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Open the runtime API declarations at `superset-frontend/packages/superset-core/src/api/editors.ts:342-381`. These lines only use `export declare` signatures and do not emit runtime implementations. 2. Start the Superset frontend dev server (the app compiles and serves frontend bundles). At runtime the compiled JS for this module will not contain functions because `export declare` produces no implementation. 3. Install or enable an extension that calls `registerEditorProvider(...)` during activation (the example Editors Bundle referenced in PR description registers providers). When that extension's initialization calls `registerEditorProvider(...)` at runtime, the imported symbol resolves to undefined (no JS function), producing a TypeError like "registerEditorProvider is not a function". 4. The EditorHost / host code that tries to call `getEditorProvider(language)` will likewise find no runtime implementation and fail to resolve providers. Because the declarations are present but no implementation exists, extension registration and provider lookup will not function at runtime. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/packages/superset-core/src/api/editors.ts **Line:** 342:381 **Comment:** *Possible Bug: The file only declares the editor API functions and events (using `export declare`) but does not provide any runtime implementation; importing and calling these functions at runtime will be undefined and cause TypeError exceptions. Provide a minimal in-memory implementation (registry + simple event listeners) so the API is usable at runtime and the returned Disposable actually unregisters the provider. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> ########## superset-frontend/src/core/editors/EditorHost.test.tsx: ########## @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { render, screen, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import type { editors } from '@apache-superset/core'; + +afterEach(() => { + cleanup(); +}); + +type EditorProps = editors.EditorProps; + +// Mock the AceEditorProvider to avoid loading the full Ace editor in tests +jest.mock('./AceEditorProvider', () => ({ + __esModule: true, + default: ({ id, value, language }: EditorProps) => ( + <div data-test="ace-editor-provider"> + <span data-test="ace-editor-id">{id}</span> + <span data-test="ace-editor-value">{value}</span> + <span data-test="ace-editor-language">{language}</span> Review Comment: **Suggestion:** Test failure: The mocked Ace editor component uses `data-test` attributes but the tests call `getByTestId`, which queries `data-testid`. This mismatch will make the queries fail and break the tests; change the attributes to `data-testid`. [possible bug] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ EditorHost unit test fails in EditorHost.test.tsx. - ❌ CI job running unit tests will fail. - ⚠️ Local developer test runs break until fixed. ``` </details> ```suggestion <div data-testid="ace-editor-provider"> <span data-testid="ace-editor-id">{id}</span> <span data-testid="ace-editor-value">{value}</span> <span data-testid="ace-editor-language">{language}</span> ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Run the test file: execute Jest for this test at superset-frontend/src/core/editors/EditorHost.test.tsx. The file registers mocks before importing EditorHost (see mock declaration at lines 30-39). 2. During module setup the Ace editor mock is defined at superset-frontend/src/core/editors/EditorHost.test.tsx:31-38 and renders JSX with attributes data-test="ace-editor-provider" and similar (lines 33-36). 3. The assertion in the test at lines 64-72 calls screen.getByTestId('ace-editor-provider') (see expectation at line 67) which queries data-testid attributes. 4. Because the mock renders data-test (not data-testid) getByTestId will not find the element and the expectation throws a TestingLibrary error, failing the test and surfacing a clear mismatch between mock attributes and test queries. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/core/editors/EditorHost.test.tsx **Line:** 33:36 **Comment:** *Possible Bug: Test failure: The mocked Ace editor component uses `data-test` attributes but the tests call `getByTestId`, which queries `data-testid`. This mismatch will make the queries fail and break the tests; change the attributes to `data-testid`. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> ########## superset-frontend/src/core/editors/index.ts: ########## @@ -0,0 +1,128 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * @fileoverview Implementation of the editors API for Superset. + * + * This module provides the runtime implementation of the editor registration + * and resolution functions declared in the API types. + */ + +import type { editors, contributions } from '@apache-superset/core'; +import { Disposable } from '../models'; +import EditorProviders from './EditorProviders'; + +type EditorLanguage = contributions.EditorLanguage; +type EditorProvider = editors.EditorProvider; +type EditorContribution = editors.EditorContribution; +type EditorComponent = editors.EditorComponent; +type EditorProviderRegisteredEvent = editors.EditorProviderRegisteredEvent; +type EditorProviderUnregisteredEvent = editors.EditorProviderUnregisteredEvent; + +/** + * Register an editor provider for specific languages. + * When an extension registers an editor, it replaces the default for those languages. + * + * @param contribution The editor contribution metadata from extension.json + * @param component The React component implementing EditorProps + * @returns A Disposable to unregister the provider + */ +export const registerEditorProvider = ( + contribution: EditorContribution, + component: EditorComponent, +): Disposable => { + const manager = EditorProviders.getInstance(); + return manager.registerProvider(contribution, component); +}; + +/** + * Get the editor provider for a specific language. + * Returns the extension's editor if registered, otherwise undefined. + * + * @param language The language to get an editor for + * @returns The editor provider or undefined if no extension provides one + */ +export const getEditorProvider = ( + language: EditorLanguage, +): EditorProvider | undefined => { + const manager = EditorProviders.getInstance(); + return manager.getProvider(language); +}; + +/** + * Check if an extension has registered an editor for a language. + * + * @param language The language to check + * @returns True if an extension provides an editor for this language + */ +export const hasEditorProvider = (language: EditorLanguage): boolean => { + const manager = EditorProviders.getInstance(); + return manager.hasProvider(language); +}; + +/** + * Get all registered editor providers. + * + * @returns Array of all registered editor providers + */ +export const getAllEditorProviders = (): EditorProvider[] => { + const manager = EditorProviders.getInstance(); + return manager.getAllProviders(); +}; + +/** + * Event fired when an editor provider is registered. + * Subscribe to this event to react when extensions register new editors. + */ +export const onDidRegisterEditorProvider = ( + listener: (e: EditorProviderRegisteredEvent) => void, +): Disposable => { + const manager = EditorProviders.getInstance(); + return manager.onDidRegister(listener); +}; + +/** + * Event fired when an editor provider is unregistered. + * Subscribe to this event to react when extensions unregister editors. + */ +export const onDidUnregisterEditorProvider = ( + listener: (e: EditorProviderUnregisteredEvent) => void, +): Disposable => { + const manager = EditorProviders.getInstance(); + return manager.onDidUnregister(listener); +}; + +/** + * Editors API object for use in the extension system. + */ +export const editorsApi: typeof editors = { Review Comment: **Suggestion:** The `editorsApi` constant is annotated with `typeof editors`, but `editors` is imported using `import type` (a type-only import). Using `typeof` requires a value-level identifier and will cause a TypeScript error; the correct approach is to type the API with the `editors` type directly (or omit the annotation). Change the type annotation to `: editors` (the type) or remove it to avoid the incorrect `typeof` usage. [type error] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ Frontend TypeScript build fails blocking CI. - ❌ Extension API export fails to compile for consumers. - ⚠️ Local development dev-server prevented from starting. ``` </details> ```suggestion export const editorsApi: editors = { ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Open the file superset-frontend/src/core/editors/index.ts and locate the export at lines 114-121 where `editorsApi` is declared with `typeof editors`. 2. Run the TypeScript build for the frontend package (e.g., from the repo root run the project's build script that invokes tsc for the frontend). The compiler will type-check superset-frontend/src/core/editors/index.ts during the build. 3. TypeScript will report an error because `editors` is imported using `import type { editors }` (a type-only import) and therefore has no runtime value; using `typeof editors` expects a value-level identifier. This manifests as a compile-time type error originating from superset-frontend/src/core/editors/index.ts:114-121 and will fail the frontend build. 4. Result: build stops with a TypeScript error preventing local development build and CI frontend bundle generation. This is a reproducible, deterministic compile-time failure (no runtime execution required). ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/core/editors/index.ts **Line:** 114:114 **Comment:** *Type Error: The `editorsApi` constant is annotated with `typeof editors`, but `editors` is imported using `import type` (a type-only import). Using `typeof` requires a value-level identifier and will cause a TypeScript error; the correct approach is to type the API with the `editors` type directly (or omit the annotation). Change the type annotation to `: editors` (the type) or remove it to avoid the incorrect `typeof` usage. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> ########## superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.tsx: ########## @@ -21,6 +21,7 @@ import { PureComponent, createRef } from 'react'; import PropTypes from 'prop-types'; import { isDefined, ensureIsArray, DatasourceType } from '@superset-ui/core'; import { t } from '@apache-superset/core'; +import type { editors } from '@apache-superset/core'; Review Comment: **Suggestion:** The type-only import uses an incorrect/ambiguous name `editors`. Import the concrete exported type used for the editor ref (for example `EditorHandle`) so TypeScript references the actual exported type instead of an undefined/incorrect namespace; this prevents type mismatches and makes the subsequent ref typing accurate. [type error] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ Frontend build fails blocking CI runs. - ❌ Dev server fails to compile component. - ❌ SQL Lab editor component compilation error. ``` </details> ```suggestion import type { EditorHandle } from '@apache-superset/core'; ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Open the frontend build for the PR in a dev or CI environment. 2. The TypeScript compiler/ts-loader processes files including superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.tsx. The import at index.tsx:24 is `import type { editors } from '@apache-superset/core';`. 3. The file declares aceEditorRef with the imported name at index.tsx:151: `aceEditorRef: RefObject<editors.EditorHandle>;` (see class declaration area). 4. If `@apache-superset/core` does not export a named type `editors`, TypeScript emits an error (e.g., "Module '@apache-superset/core' has no exported member 'editors'"), causing the compilation to fail during dev-server startup or CI build. Replacing the incorrect type with the actual exported type (for example `EditorHandle`) resolves the type lookup failure. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.tsx **Line:** 24:24 **Comment:** *Type Error: The type-only import uses an incorrect/ambiguous name `editors`. Import the concrete exported type used for the editor ref (for example `EditorHandle`) so TypeScript references the actual exported type instead of an undefined/incorrect namespace; this prevents type mismatches and makes the subsequent ref typing accurate. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> ########## superset-frontend/src/core/index.ts: ########## @@ -40,6 +40,7 @@ export const core: typeof coreType = { export * from './authentication'; export * from './commands'; +export * from './editors'; Review Comment: **Suggestion:** A flat `export * from './editors'` can cause consumers to import editor symbols directly into the top-level namespace and can trigger circular import initialization problems if `./editors` imports from this module; re-export the module as a namespaced object to keep editor APIs grouped and reduce risk of symbol collisions and accidental early access. [possible bug] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ Breaking change to extension API import shape. - ❌ SQL Lab editor integrations fail to import API. - ⚠️ Requires changes across many consumer files. ``` </details> ```suggestion export * as editors from './editors'; ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Modify `superset-frontend/src/core/index.ts` replacing line 43 (`export * from './editors';`) with `export * as editors from './editors';`. 2. Attempt to compile the frontend or build an extension that consumes the editors API using the previously-documented named imports (PR description exposes `registerEditorProvider`, `getEditorProvider`, `hasEditorProvider` from `@apache-superset/core`). 3. Consumer code that currently does `import { registerEditorProvider } from '@apache-superset/core'` (extensions and internal providers per PR description) will now fail to resolve `registerEditorProvider` as a top-level named export because it's been moved under the `editors` namespace. 4. The TypeScript compiler / bundler reports missing export or type errors in consumer modules, breaking extension registration and editor provider usage at compile-time. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/core/index.ts **Line:** 43:43 **Comment:** *Possible Bug: A flat `export * from './editors'` can cause consumers to import editor symbols directly into the top-level namespace and can trigger circular import initialization problems if `./editors` imports from this module; re-export the module as a namespaced object to keep editor APIs grouped and reduce risk of symbol collisions and accidental early access. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
