korbit-ai[bot] commented on code in PR #35749:
URL: https://github.com/apache/superset/pull/35749#discussion_r2444780313
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/types/external.d.ts:
##########
@@ -17,6 +17,109 @@
* under the License.
*/
+// Type definitions for missing luma.gl modules
+declare module '@luma.gl/webgl' {
+ export interface WebGLDevice {
+ gl: WebGL2RenderingContext | WebGLRenderingContext;
+ canvasContext?: any;
+ info?: any;
+ features?: any;
+ limits?: any;
+ }
+
+ export interface WebGLBuffer {
+ handle: WebGLBuffer | null;
+ byteLength: number;
+ usage: number;
+ }
+
+ export interface WebGLTexture {
+ handle: WebGLTexture | null;
+ width: number;
+ height: number;
+ format: number;
+ type: number;
+ }
+
+ export interface DeviceProps {
+ canvas?: HTMLCanvasElement | OffscreenCanvas;
+ gl?: WebGL2RenderingContext | WebGLRenderingContext;
+ debug?: boolean;
+ width?: number;
+ height?: number;
+ createCanvasContext?: boolean;
+ type?: 'webgl' | 'webgl2';
+ }
+
+ export class WebGLDevice {
+ constructor(props?: DeviceProps);
+ createBuffer(props?: any): WebGLBuffer;
+ createTexture(props?: any): WebGLTexture;
+ destroy(): void;
+ }
+
+ export function createDevice(props?: DeviceProps): WebGLDevice;
+}
+
+declare module '@luma.gl/core' {
+ export interface Device {
+ info?: any;
+ limits?: any;
+ features?: any;
+ }
+
+ export interface Buffer {
+ byteLength: number;
+ }
+
+ export interface Texture {
+ width: number;
+ height: number;
+ }
+
+ export class Device {
+ constructor(props?: any);
+ createBuffer(props?: any): Buffer;
+ createTexture(props?: any): Texture;
+ }
+}
+
+declare module '@luma.gl/engine' {
+ export class Model {
+ constructor(device: any, props?: any);
+ draw(props?: any): void;
+ destroy(): void;
+ }
+
+ export class Geometry {
+ constructor(props?: any);
+ }
+
+ export class Transform {
+ constructor(device: any, props?: any);
+ run(props?: any): void;
+ destroy(): void;
+ }
+}
+
+declare module '@luma.gl/shadertools' {
+ export interface ShaderModule {
+ name: string;
+ vs?: string;
+ fs?: string;
+ uniforms?: any;
+ getUniforms?: (opts?: any) => any;
+ }
+
+ export function assembleShaders(gl: any, props: any): any;
+}
+
+declare module '@luma.gl/constants' {
+ export const GL: {
+ [key: string]: number;
+ };
Review Comment:
### Generic GL constants definition <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The GL constants object uses a generic index signature that doesn't leverage
TypeScript's ability to provide specific constant names.
###### Why this matters
This prevents tree-shaking of unused GL constants and eliminates
compile-time verification of valid GL constant names, potentially including
unused constants in the final bundle.
###### Suggested change ∙ *Feature Preview*
Define specific GL constants or use a more restrictive type:
```typescript
export const GL: {
readonly TRIANGLES: number;
readonly TRIANGLE_STRIP: number;
readonly ARRAY_BUFFER: number;
readonly ELEMENT_ARRAY_BUFFER: number;
// ... other commonly used constants
} & Record<string, number>;
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a3778de-dc47-416e-a3d6-a3fa4515ca4f/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a3778de-dc47-416e-a3d6-a3fa4515ca4f?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a3778de-dc47-416e-a3d6-a3fa4515ca4f?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a3778de-dc47-416e-a3d6-a3fa4515ca4f?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a3778de-dc47-416e-a3d6-a3fa4515ca4f)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:0d21b59c-757e-4934-90c3-ec4ee682b434 -->
[](0d21b59c-757e-4934-90c3-ec4ee682b434)
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/types/external.d.ts:
##########
@@ -17,6 +17,109 @@
* under the License.
*/
+// Type definitions for missing luma.gl modules
+declare module '@luma.gl/webgl' {
+ export interface WebGLDevice {
+ gl: WebGL2RenderingContext | WebGLRenderingContext;
+ canvasContext?: any;
+ info?: any;
+ features?: any;
+ limits?: any;
+ }
+
+ export interface WebGLBuffer {
+ handle: WebGLBuffer | null;
+ byteLength: number;
+ usage: number;
+ }
+
+ export interface WebGLTexture {
+ handle: WebGLTexture | null;
+ width: number;
+ height: number;
+ format: number;
+ type: number;
+ }
+
+ export interface DeviceProps {
+ canvas?: HTMLCanvasElement | OffscreenCanvas;
+ gl?: WebGL2RenderingContext | WebGLRenderingContext;
+ debug?: boolean;
+ width?: number;
+ height?: number;
+ createCanvasContext?: boolean;
+ type?: 'webgl' | 'webgl2';
+ }
+
+ export class WebGLDevice {
+ constructor(props?: DeviceProps);
+ createBuffer(props?: any): WebGLBuffer;
+ createTexture(props?: any): WebGLTexture;
+ destroy(): void;
+ }
+
+ export function createDevice(props?: DeviceProps): WebGLDevice;
+}
+
+declare module '@luma.gl/core' {
+ export interface Device {
+ info?: any;
+ limits?: any;
+ features?: any;
+ }
Review Comment:
### Loose Interface Property Types <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Interface properties are loosely typed with 'any', making the interface less
useful for type checking and documentation.
###### Why this matters
Loose typing reduces the ability to catch type-related errors at compile
time and makes the code less self-documenting.
###### Suggested change ∙ *Feature Preview*
Define specific types for interface properties. For example:
```typescript
interface DeviceInfo {
vendor: string;
renderer: string;
version: string;
}
export interface Device {
info?: DeviceInfo;
limits?: Record<string, number>;
features?: Record<string, boolean>;
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c14592e-87e6-425f-9416-c1e32014d7c7/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c14592e-87e6-425f-9416-c1e32014d7c7?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c14592e-87e6-425f-9416-c1e32014d7c7?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c14592e-87e6-425f-9416-c1e32014d7c7?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c14592e-87e6-425f-9416-c1e32014d7c7)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:9e3bd492-355e-4694-a4c5-d8578059f3f7 -->
[](9e3bd492-355e-4694-a4c5-d8578059f3f7)
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/types/external.d.ts:
##########
@@ -17,6 +17,109 @@
* under the License.
*/
+// Type definitions for missing luma.gl modules
+declare module '@luma.gl/webgl' {
+ export interface WebGLDevice {
+ gl: WebGL2RenderingContext | WebGLRenderingContext;
+ canvasContext?: any;
+ info?: any;
+ features?: any;
+ limits?: any;
+ }
+
+ export interface WebGLBuffer {
+ handle: WebGLBuffer | null;
+ byteLength: number;
+ usage: number;
+ }
+
+ export interface WebGLTexture {
+ handle: WebGLTexture | null;
+ width: number;
+ height: number;
+ format: number;
+ type: number;
+ }
+
+ export interface DeviceProps {
+ canvas?: HTMLCanvasElement | OffscreenCanvas;
+ gl?: WebGL2RenderingContext | WebGLRenderingContext;
+ debug?: boolean;
+ width?: number;
+ height?: number;
+ createCanvasContext?: boolean;
+ type?: 'webgl' | 'webgl2';
+ }
+
+ export class WebGLDevice {
+ constructor(props?: DeviceProps);
+ createBuffer(props?: any): WebGLBuffer;
+ createTexture(props?: any): WebGLTexture;
+ destroy(): void;
+ }
+
+ export function createDevice(props?: DeviceProps): WebGLDevice;
+}
+
+declare module '@luma.gl/core' {
+ export interface Device {
+ info?: any;
+ limits?: any;
+ features?: any;
+ }
+
+ export interface Buffer {
+ byteLength: number;
+ }
+
+ export interface Texture {
+ width: number;
+ height: number;
+ }
+
+ export class Device {
+ constructor(props?: any);
+ createBuffer(props?: any): Buffer;
+ createTexture(props?: any): Texture;
+ }
+}
+
+declare module '@luma.gl/engine' {
+ export class Model {
+ constructor(device: any, props?: any);
+ draw(props?: any): void;
+ destroy(): void;
+ }
+
+ export class Geometry {
+ constructor(props?: any);
+ }
+
+ export class Transform {
+ constructor(device: any, props?: any);
+ run(props?: any): void;
+ destroy(): void;
+ }
+}
+
+declare module '@luma.gl/shadertools' {
+ export interface ShaderModule {
+ name: string;
+ vs?: string;
+ fs?: string;
+ uniforms?: any;
+ getUniforms?: (opts?: any) => any;
+ }
+
+ export function assembleShaders(gl: any, props: any): any;
+}
+
+declare module '@luma.gl/constants' {
+ export const GL: {
+ [key: string]: number;
+ };
Review Comment:
### Generic GL constants definition <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The GL constants object uses a generic index signature that doesn't leverage
TypeScript's ability to provide specific constant names.
###### Why this matters
This prevents tree-shaking of unused GL constants and eliminates
compile-time verification of valid GL constant names, potentially including
unused constants in the final bundle.
###### Suggested change ∙ *Feature Preview*
Define specific GL constants or use a more restrictive type:
```typescript
export const GL: {
readonly TRIANGLES: number;
readonly TRIANGLE_STRIP: number;
readonly ARRAY_BUFFER: number;
readonly ELEMENT_ARRAY_BUFFER: number;
// ... other commonly used constants
} & Record<string, number>;
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a3778de-dc47-416e-a3d6-a3fa4515ca4f/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a3778de-dc47-416e-a3d6-a3fa4515ca4f?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a3778de-dc47-416e-a3d6-a3fa4515ca4f?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a3778de-dc47-416e-a3d6-a3fa4515ca4f?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8a3778de-dc47-416e-a3d6-a3fa4515ca4f)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:0d21b59c-757e-4934-90c3-ec4ee682b434 -->
[](0d21b59c-757e-4934-90c3-ec4ee682b434)
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/types/external.d.ts:
##########
@@ -17,6 +17,109 @@
* under the License.
*/
+// Type definitions for missing luma.gl modules
+declare module '@luma.gl/webgl' {
+ export interface WebGLDevice {
+ gl: WebGL2RenderingContext | WebGLRenderingContext;
+ canvasContext?: any;
+ info?: any;
+ features?: any;
+ limits?: any;
+ }
+
+ export interface WebGLBuffer {
+ handle: WebGLBuffer | null;
+ byteLength: number;
+ usage: number;
+ }
+
+ export interface WebGLTexture {
+ handle: WebGLTexture | null;
+ width: number;
+ height: number;
+ format: number;
+ type: number;
+ }
+
+ export interface DeviceProps {
+ canvas?: HTMLCanvasElement | OffscreenCanvas;
+ gl?: WebGL2RenderingContext | WebGLRenderingContext;
+ debug?: boolean;
+ width?: number;
+ height?: number;
+ createCanvasContext?: boolean;
+ type?: 'webgl' | 'webgl2';
+ }
+
+ export class WebGLDevice {
+ constructor(props?: DeviceProps);
+ createBuffer(props?: any): WebGLBuffer;
+ createTexture(props?: any): WebGLTexture;
+ destroy(): void;
+ }
+
+ export function createDevice(props?: DeviceProps): WebGLDevice;
+}
+
+declare module '@luma.gl/core' {
+ export interface Device {
+ info?: any;
+ limits?: any;
+ features?: any;
+ }
Review Comment:
### Loose Interface Property Types <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Interface properties are loosely typed with 'any', making the interface less
useful for type checking and documentation.
###### Why this matters
Loose typing reduces the ability to catch type-related errors at compile
time and makes the code less self-documenting.
###### Suggested change ∙ *Feature Preview*
Define specific types for interface properties. For example:
```typescript
interface DeviceInfo {
vendor: string;
renderer: string;
version: string;
}
export interface Device {
info?: DeviceInfo;
limits?: Record<string, number>;
features?: Record<string, boolean>;
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c14592e-87e6-425f-9416-c1e32014d7c7/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c14592e-87e6-425f-9416-c1e32014d7c7?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c14592e-87e6-425f-9416-c1e32014d7c7?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c14592e-87e6-425f-9416-c1e32014d7c7?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/7c14592e-87e6-425f-9416-c1e32014d7c7)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:9e3bd492-355e-4694-a4c5-d8578059f3f7 -->
[](9e3bd492-355e-4694-a4c5-d8578059f3f7)
--
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]
