korbit-ai[bot] commented on code in PR #35240:
URL: https://github.com/apache/superset/pull/35240#discussion_r2371227750
##########
superset-frontend/webpack.config.js:
##########
@@ -538,6 +589,16 @@ const config = {
},
plugins,
devtool: isDevMode ? 'eval-cheap-module-source-map' : false,
+ watchOptions: isDevMode
+ ? {
+ // Watch all plugin and package source directories
+ ignored: ['**/node_modules', '**/.git', '**/lib', '**/esm', '**/dist'],
+ // Poll less frequently to reduce file handles
+ poll: 2000,
Review Comment:
### Inefficient polling-based file watching <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The watchOptions.poll is set to 2000ms which enables polling mode for file
watching, but this can cause excessive CPU usage and file system load,
especially in large codebases with many files.
###### Why this matters
Polling mode continuously checks files for changes at the specified
interval, which can significantly impact system performance. Modern file
systems support native file watching events that are much more efficient than
polling.
###### Suggested change ∙ *Feature Preview*
Remove the poll option to use native file system events, or make it
conditional based on environment needs:
```javascript
watchOptions: isDevMode
? {
// Watch all plugin and package source directories
ignored: ['**/node_modules', '**/.git', '**/lib', '**/esm', '**/dist'],
// Only use polling if native watching fails (e.g., in Docker/VM
environments)
// poll: process.env.WEBPACK_POLL ? 2000 : false,
// Aggregate changes for 500ms before rebuilding
aggregateTimeout: 500,
}
: undefined,
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a57a4752-b44b-4e17-af3b-ba1986b861a9/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a57a4752-b44b-4e17-af3b-ba1986b861a9?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a57a4752-b44b-4e17-af3b-ba1986b861a9?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a57a4752-b44b-4e17-af3b-ba1986b861a9?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a57a4752-b44b-4e17-af3b-ba1986b861a9)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:f0cf614b-941a-414f-983b-d5d35eff5eb9 -->
[](f0cf614b-941a-414f-983b-d5d35eff5eb9)
##########
superset-frontend/webpack.config.js:
##########
@@ -184,22 +184,68 @@ if (!isDevMode) {
chunkFilename: '[name].[chunkhash].chunk.css',
}),
);
+}
- // Runs type checking on a separate process to speed up the build
+// Type checking for both dev and production
+// In dev mode, this provides real-time type checking and builds .d.ts files
for plugins
+// Can be disabled with DISABLE_TYPE_CHECK=true npm run dev
+if (isDevMode) {
+ if (process.env.DISABLE_TYPE_CHECK) {
+ console.log('⚡ Type checking disabled (DISABLE_TYPE_CHECK=true)');
+ } else {
+ console.log(
+ '✅ Type checking enabled (disable with DISABLE_TYPE_CHECK=true npm run
dev)',
+ );
+ // Optimized configuration for development - much faster type checking
+ plugins.push(
+ new ForkTsCheckerWebpackPlugin({
+ typescript: {
+ memoryLimit: 8192,
Review Comment:
### Excessive memory allocation for TypeScript checking <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The TypeScript checker memory limit is set to 8GB in development mode, which
is excessive and may cause memory pressure on developer machines.
###### Why this matters
Allocating 8GB for type checking can lead to system slowdowns, especially on
machines with limited RAM, and may cause other applications to swap to disk.
###### Suggested change ∙ *Feature Preview*
Reduce the memory limit to a more reasonable value like 4096MB (4GB) or
2048MB (2GB) for development:
```javascript
typescript: {
memoryLimit: 4096, // 4GB should be sufficient for most projects
build: true,
mode: 'write-references',
// ...
},
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/938c0628-44b3-4e18-b2e5-82120bf45470/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/938c0628-44b3-4e18-b2e5-82120bf45470?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/938c0628-44b3-4e18-b2e5-82120bf45470?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/938c0628-44b3-4e18-b2e5-82120bf45470?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/938c0628-44b3-4e18-b2e5-82120bf45470)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:343bbd95-344f-4ae3-8d89-e3db92c449f6 -->
[](343bbd95-344f-4ae3-8d89-e3db92c449f6)
##########
superset-frontend/src/explore/components/controls/MetricControl/savedMetricType.ts:
##########
@@ -16,7 +16,16 @@
* specific language governing permissions and limitations
* under the License.
*/
-export { savedMetricType } from './types';
+import PropTypes from 'prop-types';
-// For backward compatibility with PropTypes usage
-export { savedMetricType as default } from './types';
+export type { savedMetricType } from './types';
+
+// PropTypes definition for JavaScript files
+const savedMetricTypePropTypes = PropTypes.shape({
Review Comment:
### Duplicate Type Definitions <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code maintains two parallel type definitions - one in TypeScript
(savedMetricType) and one in PropTypes - which violates the DRY principle and
increases maintenance burden.
###### Why this matters
Having duplicate type definitions increases the risk of inconsistencies when
one definition is updated but the other isn't, leading to potential runtime
type mismatches.
###### Suggested change ∙ *Feature Preview*
Consider generating PropTypes from TypeScript types using a tool like
'typescript-to-proptypes' or gradually phase out PropTypes in favor of
TypeScript types:
```typescript
import { TypeFromProps } from 'typescript-to-proptypes';
export type { savedMetricType } from './types';
export default TypeFromProps<savedMetricType>();
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/73cf289a-370b-47e8-8124-bd55341d2a6f/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/73cf289a-370b-47e8-8124-bd55341d2a6f?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/73cf289a-370b-47e8-8124-bd55341d2a6f?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/73cf289a-370b-47e8-8124-bd55341d2a6f?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/73cf289a-370b-47e8-8124-bd55341d2a6f)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:1c01f482-77fe-4009-b3a3-e68ac73f52da -->
[](1c01f482-77fe-4009-b3a3-e68ac73f52da)
--
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]
