You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "michael-s-molina (via GitHub)" <gi...@apache.org> on 2025/09/16 13:32:31 UTC

[PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

michael-s-molina opened a new pull request, #35159:
URL: https://github.com/apache/superset/pull/35159

   ### SUMMARY
   Modernized and streamlined TypeScript configuration across Superset's monorepo, consolidating redundant configurations and implementing a paths-only module resolution approach.
   
   This PR also updates files to use `GenericDataType` from `@apache-superset/core`.
   
   ## Key Changes
   
   ### 1. Root Configuration Modernization
   - **Consolidated** `tsconfig.base.json` into main `tsconfig.json` to eliminate redundancy
   - **Updated** module resolution from `"node"` to `"bundler"` (modern recommended approach)
   - **Updated** target from `"esnext"` to `"es2020"` for better compatibility
   - **Enhanced** include patterns to cover packages and plugins test files
   - **Added** echarts internal types path mapping to resolve bundler resolution conflicts
   
   ### 2. Package/Plugin tsconfig Cleanup (27 files affected)
   - **Removed** all project references (`"references"` fields) that caused build conflicts
   - **Simplified** configurations to minimal extends-only pattern
   - **Eliminated** redundant compiler options already defined in root config
   - **Standardized** inheritance pattern across all packages and plugins
   
   ### 3. Module Resolution Strategy
   - **Implemented** paths-only approach instead of hybrid project references + paths
   - **Centralized** all path mappings in root `tsconfig.json` for better maintainability
   - **Fixed** ECharts internal type imports blocked by bundler resolution
   
   ## Technical Benefits
   
   - ✅ **Faster type checking** - No more incremental builds dependency chain
   - ✅ **Simplified configuration** - Reduced from complex multi-file setup to clean inheritance
   - ✅ **Better IDE support** - Modern bundler resolution works better with IDEs
   - ✅ **Eliminated build conflicts** - No more conflicts between path mappings and project references
   - ✅ **Maintained compatibility** - All existing imports continue working
   
   ## Files Changed
   - **Root config**: `tsconfig.json` (consolidated and modernized)
   - **Packages**: 4 package tsconfig files cleaned up
   - **Plugins**: 23 plugin tsconfig files cleaned up
   - **Total**: 96 lines removed, cleaner configuration maintained
   
   ### TESTING INSTRUCTIONS
     - `npm run type` passes without errors
     - `npm run eslint` continues working
     - All internal module resolution working correctly
     - External library types (ECharts) resolved properly
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina merged PR #35159:
URL: https://github.com/apache/superset/pull/35159


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #35159:
URL: https://github.com/apache/superset/pull/35159#issuecomment-3320036223

   > Good to know! Is `plugins:build` required every time a plugin code changes or just as a one-off?
   
   I think it depends on a type definition change.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2371236751


##########
.github/workflows/superset-frontend.yml:
##########
@@ -143,7 +143,7 @@ jobs:
       - name: tsc
         run: |
           docker run --rm $TAG bash -c \
-          "npm run type"
+          "npm run plugins:build && npm run type"

Review Comment:
   Oh I think my confusion was a bit subtle, I wasn't running `npm run dev` (which wasn't required before for `npm run type`) but is required now. Touched up some things here https://github.com/apache/superset/pull/35240, running the type checker inline by default as part of `npm run dev`



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #35159:
URL: https://github.com/apache/superset/pull/35159#issuecomment-3298827018

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/35159?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 50.81%. Comparing base ([`76d897e`](https://app.codecov.io/gh/apache/superset/commit/76d897eaa2f9e137102bc194c2e3109c29d0348f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`9f8851d`](https://app.codecov.io/gh/apache/superset/commit/9f8851d85e5cc733b25d7676a535b0f4c3ad1c6b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 2529 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #35159      +/-   ##
   ==========================================
   - Coverage   60.48%   50.81%   -9.68%     
   ==========================================
     Files        1931      590    -1341     
     Lines       76236    43255   -32981     
     Branches     8568     4554    -4014     
   ==========================================
   - Hits        46114    21978   -24136     
   + Misses      28017    20660    -7357     
   + Partials     2105      617    -1488     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/35159/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/35159/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.59% <ø> (-2.56%)` | :arrow_down: |
   | [javascript](https://app.codecov.io/gh/apache/superset/pull/35159/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/35159/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `50.29% <ø> (-3.51%)` | :arrow_down: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/35159/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `50.50% <ø> (-13.00%)` | :arrow_down: |
   | [superset-extensions-cli](https://app.codecov.io/gh/apache/superset/pull/35159/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `94.52% <ø> (?)` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/35159/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/35159?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2360320449


##########
superset-frontend/src/types/files.d.ts:
##########
@@ -19,3 +19,6 @@
 
 declare module '*.svg';
 declare module '*.gif';
+declare module '*.png';
+declare module '*.jpg';
+declare module '*.jpeg';

Review Comment:
   @korbit-ai TypeScript's module declaration syntax doesn't actually support regex patterns like that in the declare module statement.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #35159:
URL: https://github.com/apache/superset/pull/35159#issuecomment-3317544419

   My envs are having troubles after rebasing and wondering if it's related to this PR. Are there new commands required to run in order to set up plugins after this PR? 


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2360321362


##########
superset-frontend/src/types/files.d.ts:
##########
@@ -19,3 +19,6 @@
 
 declare module '*.svg';
 declare module '*.gif';
+declare module '*.png';
+declare module '*.jpg';
+declare module '*.jpeg';

Review Comment:
   You're right, I apologize for the incorrect suggestion. TypeScript indeed doesn't support regex in module declarations. A more appropriate approach would be to keep the separate declarations as they are, which is actually the standard practice for TypeScript projects. The current implementation is correct and widely used.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2369556253


##########
.github/workflows/superset-frontend.yml:
##########
@@ -143,7 +143,7 @@ jobs:
       - name: tsc
         run: |
           docker run --rm $TAG bash -c \
-          "npm run type"
+          "npm run plugins:build && npm run type"

Review Comment:
   @rusackas I think correctness beats slowness in this case as if a plugin type changed, we need to rebuild them in order for `npm run type` to work. If you know a faster way of doing that while generating the types when needed, it would be awesome.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #35159:
URL: https://github.com/apache/superset/pull/35159#issuecomment-3318543467

   Exactly @msyavuz! The plugins need to be updated @mistercrunch.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2371236751


##########
.github/workflows/superset-frontend.yml:
##########
@@ -143,7 +143,7 @@ jobs:
       - name: tsc
         run: |
           docker run --rm $TAG bash -c \
-          "npm run type"
+          "npm run plugins:build && npm run type"

Review Comment:
   Touched up some things here https://github.com/apache/superset/pull/35240 to improve devex, I think I got webpack to watch/auto-compile `plugins/` and `packages/` and now running the type checker inline by default as part of `npm run dev`



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2372018035


##########
.github/workflows/superset-frontend.yml:
##########
@@ -143,7 +143,7 @@ jobs:
       - name: tsc
         run: |
           docker run --rm $TAG bash -c \
-          "npm run type"
+          "npm run plugins:build && npm run type"

Review Comment:
   Great! Thanks @mistercrunch!



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #35159:
URL: https://github.com/apache/superset/pull/35159#issuecomment-3320027876

   Good to know! Is `plugins:build` required every time a plugin code changes or just as a one-off?


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2369553637


##########
.github/workflows/superset-frontend.yml:
##########
@@ -143,7 +143,7 @@ jobs:
       - name: tsc
         run: |
           docker run --rm $TAG bash -c \
-          "npm run type"
+          "npm run plugins:build && npm run type"

Review Comment:
   Sorry to keep rambing, but if we DO need it, we might want to try pnpm/bun here to speed things up quite a bit. 



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2369546292


##########
.github/workflows/superset-frontend.yml:
##########
@@ -143,7 +143,7 @@ jobs:
       - name: tsc
         run: |
           docker run --rm $TAG bash -c \
-          "npm run type"
+          "npm run plugins:build && npm run type"

Review Comment:
   @michael-s-molina ^^ just for an @-mention :D



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on PR #35159:
URL: https://github.com/apache/superset/pull/35159#issuecomment-3298797913

   Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment `/korbit-review`.
   
   > Your admin can change your review schedule in the [Korbit Console](https://app.korbit.ai/settings)
   
   
   
   [](b16b3fc1-c722-4cf5-b04e-35a2d34070fa)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2360478120


##########
superset-frontend/src/pages/AlertReportList/index.tsx:
##########
@@ -271,8 +271,7 @@ function AlertList({
           },
         }: any) =>
           lastEvalDttm
-            ? extendedDayjs
-                .utc(lastEvalDttm)
+            ? (extendedDayjs.utc(lastEvalDttm) as any)

Review Comment:
   Is this necessary? It should work without `as any`



##########
superset-frontend/src/pages/QueryHistoryList/index.tsx:
##########
@@ -239,7 +239,7 @@ function QueryList({ addDangerToast }: QueryListProps) {
             original: { start_time },
           },
         }: any) => {
-          const start = extendedDayjs.utc(start_time).local();
+          const start = ((extendedDayjs as any).utc(start_time) as any).local();

Review Comment:
   This too



##########
superset-frontend/src/pages/QueryHistoryList/index.tsx:
##########
@@ -267,9 +267,9 @@ function QueryList({ addDangerToast }: QueryListProps) {
           const startTime = start_running_time || start_time;
           const timerTime =
             end_time && startTime
-              ? extendedDayjs(extendedDayjs.utc(end_time - startTime)).format(
-                  TIME_WITH_MS,
-                )
+              ? extendedDayjs(
+                  (extendedDayjs as any).utc(end_time - startTime),
+                ).format(TIME_WITH_MS)

Review Comment:
   i think these might be leftovers from a debugging step earlier?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2360283453


##########
superset-frontend/src/types/files.d.ts:
##########
@@ -19,3 +19,6 @@
 
 declare module '*.svg';
 declare module '*.gif';
+declare module '*.png';
+declare module '*.jpg';
+declare module '*.jpeg';

Review Comment:
   ### Redundant Image Module Declarations <sub>![category Design](https://img.shields.io/badge/Design-0d9488)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Multiple similar image module declarations are defined separately, violating the DRY principle and making the code less maintainable.
   
   
   ###### Why this matters
   If new image formats need to be supported in the future, developers would need to add new declarations one by one, increasing the chance of inconsistencies and making maintenance more difficult.
   
   ###### Suggested change ∙ *Feature Preview*
   Combine all image declarations into a single, more maintainable declaration using a union type:
   ```typescript
   declare module '*.(svg|gif|png|jpg|jpeg)' {
     const content: string;
     export default content;
   }
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c8ed481e-3172-459a-8dca-4af6019c7ca1/upvote) [![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c8ed481e-3172-459a-8dca-4af6019c7ca1?what_not_true=true)  [![Not in Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c8ed481e-3172-459a-8dca-4af6019c7ca1?what_out_of_scope=true) [![Not in coding standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c8ed481e-3172-459a-8dca-4af6019c7ca1?what_not_in_standard=true) [![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c8ed481e-3172-459a-8dca-4af6019c7ca1)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:29c8a2f3-255b-4fa3-90b9-b1b7a55f9896 -->
   
   
   [](29c8a2f3-255b-4fa3-90b9-b1b7a55f9896)



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #35159:
URL: https://github.com/apache/superset/pull/35159#issuecomment-3309310854

   YES!


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2369552367


##########
.github/workflows/superset-frontend.yml:
##########
@@ -143,7 +143,7 @@ jobs:
       - name: tsc
         run: |
           docker run --rm $TAG bash -c \
-          "npm run type"
+          "npm run plugins:build && npm run type"

Review Comment:
   We're hoping that `tsc` is sufficient for the plugins/packages :D 



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2371114081


##########
.github/workflows/superset-frontend.yml:
##########
@@ -143,7 +143,7 @@ jobs:
       - name: tsc
         run: |
           docker run --rm $TAG bash -c \
-          "npm run type"
+          "npm run plugins:build && npm run type"

Review Comment:
   Seems we need to adjust pre-commit and other things, or is there any way to get webpack do also rebuilt this automatically the same way it rebuilds everything else?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on PR #35159:
URL: https://github.com/apache/superset/pull/35159#issuecomment-3317707622

   I had to run build plugins command before running npm run type.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] chore: TypeScript Configuration Modernization and Cleanup [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #35159:
URL: https://github.com/apache/superset/pull/35159#discussion_r2369541744


##########
.github/workflows/superset-frontend.yml:
##########
@@ -143,7 +143,7 @@ jobs:
       - name: tsc
         run: |
           docker run --rm $TAG bash -c \
-          "npm run type"
+          "npm run plugins:build && npm run type"

Review Comment:
   Do we NEED to do the plugins:build here? I think it's good hygeine, and we should be doing this in pre-commit as well, but there are some concerns about it being too slow for pre-commit. 



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org