chore: migrate to rspack 🦀 #9839
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several configuration changes to transition the NocoDB project from using Webpack to Rspack for building Node.js applications. A new Rspack configuration file is added for Docker, while existing Webpack configuration files are removed. The changes include updates to various configuration files to accommodate Rspack's API and features, modifications to service imports, and enhancements to method functionalities. Additionally, a script for building the Docker image is updated to utilize Rspack instead of Webpack. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Outside diff range and nitpick comments (12)
packages/nocodb/rspack.config.js (2)
43-43: Remove or update the configuration commentThe comment
//Update this to true or falsesuggests uncertainty about the optimization setting. Please make a definitive decision and remove the comment.
76-78: Document the EE environment variableThe configuration sets
EE: truebut its purpose isn't clear. Consider adding documentation about this environment variable's significance.packages/nocodb/src/services/sql-views.service.ts (3)
Line range hint
42-44: Critical: Method is blocked bynotImplementederrorThe method immediately returns after throwing
NcError.notImplemented(), making all the subsequent code unreachable. This appears to be an oversight as it effectively disables the SQL view creation functionality.Remove these lines to restore the functionality:
- NcError.notImplemented(); - return;
Line range hint
97-116: Consider using constants for database-specific limitsWhile the length validation logic is correct, the magic numbers for different database limits should be defined as constants for better maintainability.
Consider refactoring to use constants:
+ const DB_NAME_LENGTH_LIMITS = { + mysql: 64, + mysql2: 64, + pg: 63, + mssql: 128, + default: 255 + }; + let tableNameLengthLimit = 255; const sqlClientType = sqlClient.knex.clientType(); - if (sqlClientType === 'mysql2' || sqlClientType === 'mysql') { - tableNameLengthLimit = 64; - } else if (sqlClientType === 'pg') { - tableNameLengthLimit = 63; - } else if (sqlClientType === 'mssql') { - tableNameLengthLimit = 128; - } + tableNameLengthLimit = DB_NAME_LENGTH_LIMITS[sqlClientType] || DB_NAME_LENGTH_LIMITS.default;
Line range hint
119-119: Address the TODO comment for view creationThe comment "TODO - reimplement this" suggests that the view creation logic needs to be reimplemented. This should be addressed before the code goes to production.
Would you like me to help create a GitHub issue to track this TODO item?
packages/nocodb/src/Noco.ts (1)
99-111: Document Rspack migration implicationsThe migration to Rspack and the HMR implementation changes could affect the development workflow.
Consider:
- Documenting any differences in HMR behavior between Webpack and Rspack
- Adding development guidelines for handling HMR edge cases
- Creating a rollback plan in case of critical HMR issues
- Setting up monitoring for build performance metrics to validate the migration benefits
🧰 Tools
🪛 Biome
[error] 105-105: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
packages/nocodb/rspack.local.config.js (3)
44-44: Remove unnecessary comment or set 'minimize' appropriatelyThe comment
//Update this to true or falsesuggests that theminimizeoption may need to be updated. Please setminimizeto the desired boolean value and remove the comment to avoid confusion.
67-67: Consider changing 'mode' to 'development' for local buildsThe
modeis set to'production', which enables optimizations suited for production environments. For a local development configuration, you might want to setmodeto'development'to enhance debugging capabilities and speed up the build process.
44-44: Align 'minimize' option with the 'mode' settingWith
modeset to'production',minimizeistrueby default. If you change themodeto'development', consider settingminimizetofalseto disable code minimization during development for easier debugging.Also applies to: 67-67
packages/nocodb/docker/rspack.config.js (2)
47-47: Simplify 'path' usage by utilizing the existing 'path' moduleSince you have already required the 'path' module at the top, you can use
path.resolvedirectly instead ofrequire('path').resolve.Apply this diff to simplify the code:
- path: require('path').resolve('./docker'), + path: path.resolve('./docker'),
54-54: Remove unnecessary comment in the 'minimize' optionThe comment
//Update this to true or falseis redundant and can be removed to clean up the configuration.Apply this diff:
- minimize: true, //Update this to true or false + minimize: true,packages/nocodb/rspack.dev.config.js (1)
79-83: Leverage Rspack's built-in TypeScript type checking
ForkTsCheckerWebpackPluginis used in Webpack to perform type checking in a separate process. Rspack may provide built-in support for TypeScript type checking, rendering this plugin unnecessary.Proposed Change:
- new ForkTsCheckerWebpackPlugin({ - typescript: { - configFile: join('tsconfig.json'), - }, - }), + // Removed ForkTsCheckerWebpackPlugin to use Rspack's built-in type checkingNext Steps:
- Enable TypeScript type checking in Rspack by configuring it appropriately in the Rspack configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (6)
.github/workflows/release-timely-docker.ymlis excluded by!**/*.ymlpackages/nocodb/package.jsonis excluded by!**/*.jsonpackages/nocodb/tests/unit/tsconfig.ee.jsonis excluded by!**/*.jsonpackages/nocodb/tests/unit/tsconfig.jsonis excluded by!**/*.jsonpackages/nocodb/tsconfig.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (13)
packages/nocodb/docker/rspack.config.js(1 hunks)packages/nocodb/docker/webpack.config.js(0 hunks)packages/nocodb/rspack.cli.config.js(3 hunks)packages/nocodb/rspack.config.js(1 hunks)packages/nocodb/rspack.dev.config.js(1 hunks)packages/nocodb/rspack.local.config.js(1 hunks)packages/nocodb/rspack.timely.config.js(1 hunks)packages/nocodb/src/Noco.ts(1 hunks)packages/nocodb/src/db/sql-mgr/SqlMgr.ts(1 hunks)packages/nocodb/src/services/sql-views.service.ts(1 hunks)packages/nocodb/webpack.config.js(0 hunks)packages/nocodb/webpack.local.config.js(0 hunks)packages/nocodb/webpack.timely.config.js(0 hunks)
💤 Files with no reviewable changes (4)
- packages/nocodb/docker/webpack.config.js
- packages/nocodb/webpack.config.js
- packages/nocodb/webpack.local.config.js
- packages/nocodb/webpack.timely.config.js
🧰 Additional context used
🪛 Biome
packages/nocodb/src/Noco.ts
[error] 105-105: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🔇 Additional comments (35)
packages/nocodb/rspack.config.js (4)
79-81: Verify public assets path resolution
Ensure that the source and destination paths for public assets resolve correctly in all build environments.
✅ Verification successful
Let me verify the content of these public directories to ensure the paths are correctly configured.
Public assets path configuration is correctly set up
The source directory packages/nocodb/src/public/ exists and contains the required assets (css, js, favicon.ico, icon.png). The path configuration in rspack.config.js correctly maps these assets from src/public to the public directory in the build output. This pattern is consistently used across different build configurations (dev, timely) and Docker files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if source directory exists and contains files
if [ -d "src/public" ]; then
find src/public -type f
else
echo "src/public directory not found"
fi
# Check for other potential public directories
fd -t d -g "*public*"
Length of output: 170
Script:
#!/bin/bash
# Check contents of both public directories
echo "=== Contents of packages/nocodb/src/public/ ==="
ls -la packages/nocodb/src/public/
echo -e "\n=== Contents of packages/nc-gui/public/ ==="
ls -la packages/nc-gui/public/
# Check if there are any file references to these paths in the codebase
echo -e "\n=== References to public paths in code ==="
rg "src/public" packages/nocodb/
Length of output: 1677
3-3: Consider migrating from webpack-node-externals
While migrating to Rspack, the configuration still relies on webpack-node-externals. Consider checking if Rspack provides a native alternative or if this dependency is officially supported with Rspack.
53-53: Verify the impact of nodeEnv: false
Setting nodeEnv: false disables automatic environment variable injection. Ensure this doesn't conflict with your environment-specific configurations.
✅ Verification successful
Based on the search results, I can see that:
- The codebase heavily uses environment variables for configuration
- There are two rspack config files:
- packages/nocodb/docker/rspack.config.js
- packages/nocodb/rspack.config.js
Let me check the content of both files to understand how nodeEnv is used.
Setting nodeEnv: false is correct and intentional
Setting nodeEnv: false in the rspack configuration is the correct approach here because:
- Both rspack configs (docker and main) consistently use
nodeEnv: falsein their optimization settings - This prevents rspack from automatically injecting
process.env.NODE_ENVinto the bundle - The application explicitly manages environment variables through its own configuration system, as evidenced by the extensive environment variable usage seen in the codebase
- The EnvironmentPlugin is used to explicitly define required environment variables (EE: true)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for environment variable usage in the codebase
rg "process\.env\." --type ts
# Check for other rspack configs that might have different nodeEnv settings
fd -e js -e ts -g "*rspack.config*"
Length of output: 37377
Script:
#!/bin/bash
# Check content of both rspack config files
cat packages/nocodb/docker/rspack.config.js
cat packages/nocodb/rspack.config.js
Length of output: 4165
20-37: Verify TypeScript configuration compatibility
The SWC loader configuration includes advanced TypeScript features. Please ensure that:
- Legacy decorator and metadata settings align with your TypeScript version
- ES2017 target is appropriate for your Node.js deployment environment
✅ Verification successful
The previous attempt didn't provide enough information about TypeScript configuration and decorator usage. Let's try to gather this information using more specific commands to verify the TypeScript configuration compatibility and decorator usage in the codebase.
Based on the gathered information, I can now provide a final response:
SWC configuration aligns with TypeScript and Node.js requirements
The SWC configuration in rspack.config.js is compatible with:
- The project's TypeScript configuration which targets ES2017
- The Node.js requirements (project requires Node.js >=18.19.1)
- The decorator settings are appropriate as the project has explicit decorator support enabled in tsconfig.json
The verification shows:
- The project's tsconfig.json targets ES2017, matching the SWC target
- The Node.js engine requirement (>=18.19.1) supports ES2017 features
- Decorator and metadata settings are properly configured in both TypeScript and SWC configurations
- The commonjs module type is consistent across configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check TypeScript configuration in the project
cat tsconfig.json | jq '.compilerOptions | {target, experimentalDecorators, emitDecoratorMetadata}'
# Check if there are any decorators in use
rg -l "decorator" --type ts
Length of output: 37377
Script:
#!/bin/bash
# Check TypeScript configuration in the project
fd -e json -x cat {} \; -g '*tsconfig*'
# Check if there are any decorators in use
ast-grep --pattern 'decorator $_'
# Check Node.js version requirements in package.json
fd package.json -x cat {}
Length of output: 49153
packages/nocodb/src/services/sql-views.service.ts (2)
Line range hint 63-95: LGTM: Robust view name validation
The view name validation is thorough, including sanitization, whitespace checks, and duplicate prevention.
150-150: LGTM: Added user tracking
Good addition of user_id to track the creator of the view.
packages/nocodb/src/Noco.ts (1)
108-111: Verify Rspack HMR compatibility
Since we're migrating from Webpack to Rspack, we need to ensure HMR compatibility.
Let's verify the Rspack configuration and HMR setup:
packages/nocodb/src/db/sql-mgr/SqlMgr.ts (1)
21-21:
Address type checking issues with NcConnectionMgr import.
Using @ts-expect-error to suppress type checking is a temporary solution that could mask potential runtime issues. Consider properly fixing the type definitions for the NcConnectionMgr module.
#!/bin/bash
# Description: Check if type definitions exist for NcConnectionMgr
# Search for type definition files
fd -e d.ts -e ts "NcConnectionMgr.d.ts|NcConnectionMgr.ts" src/utils/common/
# Check usage of NcConnectionMgr in the codebase
ast-grep --pattern 'NcConnectionMgr'packages/nocodb/rspack.cli.config.js (5)
2-2: LGTM: Updated import to Rspack
The import statement correctly updates webpack to rspack.
24-29: LGTM: Replaced TerserPlugin with SwcJsMinimizerRspackPlugin
The minimizer is appropriately updated to use Rspack's SWC-based minimizer with the correct options to remove comments.
34-36: LGTM: Updated externals configuration with allowlist
The externals configuration correctly uses nodeExternals with an allowlist for 'nocodb-sdk'.
55-57: LGTM: Updated EnvironmentPlugin configuration
The EnvironmentPlugin is correctly updated to use rspack, and the EE variable is appropriately set to true.
58-60: LGTM: Updated BannerPlugin configuration
The BannerPlugin is correctly updated to use rspack and retains its intended functionality.
packages/nocodb/rspack.local.config.js (4)
3-3: Verify compatibility of 'webpack-node-externals' with Rspack
The webpack-node-externals package is designed for use with Webpack. Since you're migrating to Rspack, please ensure that this plugin is compatible with Rspack or consider using an equivalent plugin specifically for Rspack.
63-66: Verify the placement of 'tsConfig' under 'resolve'
The tsConfig property under resolve may not be recognized in Rspack's configuration. Please verify if tsConfig should be placed under resolve or if it needs to be configured elsewhere, such as in a loader option.
2-2: Ensure correct import and usage of 'SwcJsMinimizerRspackPlugin'
The SwcJsMinimizerRspackPlugin is being used without an explicit import. Please verify that it is correctly imported or accessed from rspack. You may need to import it separately:
-const { rspack } = require('@rspack/core');
+const { rspack, SwcJsMinimizerRspackPlugin } = require('@rspack/core');And update the minimizer configuration accordingly:
- new rspack.SwcJsMinimizerRspackPlugin({
+ new SwcJsMinimizerRspackPlugin({Also applies to: 46-53
2-2: Verify the correct usage of 'EnvironmentPlugin'
The EnvironmentPlugin may need to be imported explicitly. Ensure that it is correctly imported or accessed from rspack. Consider importing it as follows:
-const { rspack } = require('@rspack/core');
+const { rspack, EnvironmentPlugin } = require('@rspack/core');And update the plugins array:
- new rspack.EnvironmentPlugin({
+ new EnvironmentPlugin({Also applies to: 79-82
packages/nocodb/rspack.timely.config.js (6)
19-38: Ensure correct loader and options for TypeScript files
The configuration uses 'builtin:swc-loader' to process TypeScript files. Please verify that this loader and its options are correctly set up for Rspack. Specifically, check if the options provided align with Rspack's supported configuration.
74-76: Verify 'node' configuration support in Rspack
The node property is used to set __dirname: false. Please confirm that Rspack supports the node configuration similarly to Webpack, or adjust the configuration accordingly.
77-84: Validate plugin compatibility with Rspack
The plugins rspack.EnvironmentPlugin and rspack.CopyRspackPlugin are used in the configuration. Ensure that these plugins are properly configured and compatible with Rspack.
71-72: Check 'libraryTarget' and 'globalObject' settings
In the output section, libraryTarget is set to 'umd', and globalObject is defined. Verify that these settings are appropriate for your target environment and that they function correctly with Rspack.
11-15: Check compatibility of 'node-loader' with Rspack
The use of node-loader in the module rules might not be directly compatible with Rspack. Please verify if node-loader can be used with Rspack or if there's an alternative method to handle .node files.
Run the following script to search for node-loader usage in Rspack's documentation:
#!/bin/bash
# Description: Check for 'node-loader' in Rspack modules.
# Expectation: No matches may suggest incompatibility.
rg 'node-loader' node_modules/@rspack3-3: Verify compatibility of 'webpack-node-externals' with Rspack
The configuration uses webpack-node-externals to handle externals, which is a Webpack-specific package. Since this migration is to Rspack, please verify whether webpack-node-externals is compatible with Rspack or if an alternative approach is needed.
Run the following script to check for any mentions of webpack-node-externals in Rspack's codebase, which may indicate compatibility:
Also applies to: 56-59
✅ Verification successful
Rspack supports webpack-node-externals package
The codebase shows consistent usage of webpack-node-externals across multiple Rspack configuration files (rspack.config.js, rspack.timely.config.js, rspack.local.config.js, etc.) with identical configuration patterns, indicating that Rspack is compatible with this package. This is expected as Rspack aims to maintain webpack compatibility.
Key evidence:
- Multiple Rspack configs successfully use the same externals configuration pattern
- The package is properly listed in dependencies (found in pnpm-lock.yaml)
- Consistent usage pattern across all configurations with
allowlistoption
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for 'webpack-node-externals' in Rspack modules.
# Expectation: No matches may suggest incompatibility.
rg 'webpack-node-externals' node_modules/@rspack
Length of output: 217
Script:
#!/bin/bash
# Let's check if webpack-node-externals is listed in package dependencies
cat package.json | jq '.dependencies["webpack-node-externals"], .devDependencies["webpack-node-externals"]'
# Check Rspack documentation or examples for externals handling
fd -e md -e mdx . | xargs rg -i "externals|webpack-node-externals"
# Look for actual usage of nodeExternals in the codebase
rg "nodeExternals|webpack-node-externals" -A 3
Length of output: 6374
packages/nocodb/docker/rspack.config.js (6)
10-10: Verify the compatibility of 'node-loader' with Rspack
The node-loader is a Webpack loader, and it's unclear if it's compatible with Rspack. Please ensure that this loader works as intended with Rspack or replace it with a compatible alternative if necessary.
18-18: Confirm the use of 'builtin:swc-loader' in Rspack
Please verify that 'builtin:swc-loader' is the correct loader to use for handling TypeScript files in Rspack. If a different loader is required, adjust the configuration accordingly.
56-62: Confirm the use of 'SwcJsMinimizerRspackPlugin' for minimization
Please verify that rspack.SwcJsMinimizerRspackPlugin is the correct plugin for code minimization in Rspack and that it's properly configured.
64-64: Check if 'nodeEnv' is a valid option in 'optimization'
The nodeEnv option under optimization might not be valid in Rspack. If you intend to define the NODE_ENV, consider using DefinePlugin or EnvironmentPlugin instead.
72-75: Verify the usage of 'rspack.EnvironmentPlugin' for environment variables
Ensure that rspack.EnvironmentPlugin is the correct plugin to define environment variables in Rspack. If not, find the appropriate plugin or method provided by Rspack for this purpose.
77-79: Check the validity of 'node' options in Rspack configuration
The node options may not be supported or may be configured differently in Rspack. Confirm whether these options are necessary and valid, and adjust the configuration as needed.
packages/nocodb/rspack.dev.config.js (6)
14-19:
Verify the necessity of 'node-loader' with Rspack
The use of 'node-loader' to handle .node files may not be required or compatible with Rspack. Rspack might have different mechanisms for loading native modules.
Proposed Action:
Check Rspack's documentation to determine if handling .node files requires a specific loader or configuration.
Suggested Change:
{
test: /\.node$/,
- loader: 'node-loader',
- options: {
- name: '[path][name].[ext]',
- },
+ // Removed 'node-loader' as it may not be necessary with Rspack
},100-102:
Adjust 'watchOptions' for Rspack
The watchOptions might have different properties in Rspack compared to Webpack. Using unsupported options can lead to unexpected behavior.
Proposed Action:
- Verify if
watchOptionsproperties likeignoredandpollare supported in Rspack. - Adjust the configuration according to Rspack's documentation.
Suggested Change:
watchOptions: {
- ignored: /node_modules/,
- poll: true,
+ // Update or remove watchOptions based on Rspack's supported options
},58-63:
Ensure TypeScript configuration is correctly referenced
The resolve.tsConfig property might not be utilized in Rspack as it is in Webpack. Misconfiguration can lead to issues in module resolution.
Proposed Action:
- Check if
tsConfigunderresolveis a valid property in Rspack. - If not, configure TypeScript settings under the appropriate section in Rspack.
Suggested Change:
resolve: {
extensions: ['.tsx', '.ts', '.js', '.json', '.node'],
- tsConfig: {
- configFile: resolve('tsconfig.json'),
- },
+ // Remove or relocate tsConfig according to Rspack's configuration schema
},73-75:
Remove 'RunScriptWebpackPlugin' or find an alternative
RunScriptWebpackPlugin is a Webpack plugin that may not be compatible with Rspack. Since Rspack might have different methods for running scripts or handling hot reloading, this plugin might be unnecessary or need replacement.
Proposed Action:
-
Remove the plugin:
- new RunScriptWebpackPlugin({ - name: 'main.js', - }),
-
Investigate Rspack's documentation for equivalent functionality or built-in features that serve the same purpose.
⛔ Skipped due to learnings
Learnt from: DarkPhoenix2704
PR: nocodb/nocodb#9836
File: packages/nocodb/rspack.dev.config.js:88-90
Timestamp: 2024-11-17T06:23:52.399Z
Learning: In Rspack configuration files (e.g., `rspack.dev.config.js`), `RunScriptWebpackPlugin` is acceptable to use, as it is suggested by Rspack.
3-5: 🛠️ Refactor suggestion
Replace Webpack plugins with Rspack-compatible alternatives
The imported plugins RunScriptWebpackPlugin, ForkTsCheckerWebpackPlugin, and webpack-node-externals are Webpack-specific and may not be compatible with Rspack. Using incompatible plugins can lead to build failures or unexpected behavior.
Proposed Solution:
-
Remove Webpack-specific plugins:
-const { RunScriptWebpackPlugin } = require('run-script-webpack-plugin'); -const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin'); -const nodeExternals = require('webpack-node-externals');
-
Use Rspack-compatible plugins or built-in features:
- Replace
RunScriptWebpackPluginwith Rspack's built-in capabilities or find an alternative plugin compatible with Rspack. - Leverage Rspack's built-in TypeScript type checking instead of
ForkTsCheckerWebpackPlugin. - Configure externals using Rspack's syntax without relying on
webpack-node-externals.
- Replace
⛔ Skipped due to learnings
Learnt from: DarkPhoenix2704
PR: nocodb/nocodb#9836
File: packages/nocodb/rspack.dev.config.js:88-90
Timestamp: 2024-11-17T06:23:52.399Z
Learning: In Rspack configuration files (e.g., `rspack.dev.config.js`), `RunScriptWebpackPlugin` is acceptable to use, as it is suggested by Rspack.
93-98:
Confirm 'devServer' configuration compatibility with Rspack
The devServer configuration may differ between Webpack and Rspack. Some options used here might not be supported or might need adjustment.
Proposed Action:
- Review Rspack's documentation for
devServerto ensure all options are correctly set. - Particularly, check if
devMiddleware.writeToDiskandportare configured appropriately.
Verification Script:
✅ Verification successful
Let me gather more information about Rspack's devServer configuration by checking the package.json and any documentation references.
Let me check the devServer configuration in other Rspack config files to understand the consistency of the configuration across different environments.
devServer configuration is compatible with Rspack 1.1.1
The devServer configuration used in the code is valid for Rspack 1.1.1. Both devMiddleware.writeToDisk and port are supported options in Rspack's development server configuration, which maintains compatibility with webpack's devServer API. The configuration is also consistent across different environment configs, appearing only in the development configuration where it's needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for 'devServer' options in Rspack's configuration.
# Extract devServer configuration from rspack.dev.config.js
rg -A 5 'devServer' packages/nocodb/rspack.dev.config.js
# Note: Adjust the options based on Rspack's supported configuration pr
8000
operties.
Length of output: 145
Script:
#!/bin/bash
# Check package.json for @rspack dependencies and their versions
rg -A 5 '"@rspack' packages/nocodb/package.json
# Look for any Rspack documentation or configuration examples in README or docs
fd -e md -e txt . | xargs rg -l 'rspack.*devServer|devServer.*rspack'
# Check for any other Rspack config files that might have devServer settings
fd rspack.*config
Length of output: 602
Script:
#!/bin/bash
# Check devServer configuration in all Rspack config files
for file in packages/nocodb/rspack.*.config.js; do
echo "=== $file ==="
rg -A 5 'devServer' "$file" || true
done
# Also check the docker config
echo "=== Docker config ==="
rg -A 5 'devServer' packages/nocodb/docker/rspack.config.js || true
Length of output: 1064
|
Uffizzi Preview |
f2f8928 to
3310e52
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
packages/nocodb-sdk/src/lib/Api.ts (1)
3296-3319: LGTM! Consider some enhancements to make the interface more robust.Consider the following improvements:
export interface SnapshotType { id?: IdType; title?: string; base_id?: IdType; snapshot_base_id?: IdType; fk_workspace_id?: IdType; created_at?: string; created_by?: IdType; - status?: string; + status?: 'draft' | 'published' | 'archived'; // Add specific status values }This would:
- Add type safety by restricting possible status values
- Make the interface more self-documenting
build-local-docker-image.sh (1)
48-48: Consider improving error message specificityThe error message "package_nocodb failed" could be more descriptive to help with debugging Rspack-specific issues.
- EE=true ${SCRIPT_DIR}/node_modules/@rspack/cli/bin --config ${SCRIPT_DIR}/packages/nocodb/rspack.config.js || ERROR="package_nocodb failed" + EE=true ${SCRIPT_DIR}/node_modules/@rspack/cli/bin --config ${SCRIPT_DIR}/packages/nocodb/rspack.config.js || ERROR="Rspack build failed for nocodb package. Check the logs above for compilation errors."packages/nocodb/rspack.cli.config.js (1)
Line range hint
5-66: Consider adding build performance optimizationsSince this is a migration to Rspack, which promises better build performance, consider the following optimizations:
- Enable build caching for faster rebuilds:
cache: { type: 'filesystem', },
- Consider adding source maps for production debugging:
devtool: 'source-map',
- Monitor and compare build times with the previous Webpack configuration to validate the migration benefits.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/nocodb/package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (6)
build-local-docker-image.sh(1 hunks)packages/nocodb-sdk/src/lib/Api.ts(1 hunks)packages/nocodb/rspack.cli.config.js(4 hunks)packages/nocodb/rspack.config.js(1 hunks)packages/nocodb/rspack.dev.config.js(1 hunks)packages/nocodb/rspack.timely.config.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/nocodb/rspack.dev.config.js
- packages/nocodb/rspack.config.js
- packages/nocodb/rspack.timely.config.js
🔇 Additional comments (6)
build-local-docker-image.sh (1)
48-48: Verify Rspack dependencies and configuration
The migration from Webpack to Rspack looks correct. However, let's verify the required setup.
✅ Verification successful
Rspack dependencies and configuration are properly set up
The verification shows that:
@rspack/cliv1.1.4 is correctly listed in nocodb's package dependencies- It's properly locked in pnpm-lock.yaml
- The rspack.config.js exists and contains all necessary configuration including:
- Proper module rules for TypeScript and node files
- Correct optimization settings
- Required plugins for environment variables and asset copying
- Appropriate output configuration for node target
The missing node_modules directory in the sandbox environment is expected and not a concern as it's populated during the actual build process.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Rspack dependencies and configuration
# Check if @rspack/cli is listed in package.json
echo "Checking for @rspack/cli in package.json:"
cat package.json | jq '.dependencies["@rspack/cli"] // .devDependencies["@rspack/cli"]'
# Verify rspack config file exists
echo -e "\nChecking for rspack config file:"
fd "rspack.config.js" packages/nocodb/
# Check rspack config content for essential settings
echo -e "\nChecking rspack config content:"
if [ -f "packages/nocodb/rspack.config.js" ]; then
cat packages/nocodb/rspack.config.js
fi
Length of output: 2856
Script:
#!/bin/bash
# Check if @rspack/cli is in workspace dependencies
echo "Checking workspace dependencies:"
fd "package.json" packages/ -x cat {} | jq -c 'select(.dependencies["@rspack/cli"] // .devDependencies["@rspack/cli"] != null) | {name: .name, rspack: (.dependencies["@rspack/cli"] // .devDependencies["@rspack/cli"])}'
# Check if pnpm-lock.yaml contains @rspack/cli
echo -e "\nChecking pnpm-lock.yaml for @rspack/cli:"
rg "@rspack/cli" pnpm-lock.yaml
# Check if @rspack/cli is actually installed
echo -e "\nChecking if @rspack/cli is installed:"
fd "@rspack" "node_modules" -d 2
Length of output: 839
packages/nocodb/rspack.cli.config.js (5)
24-29: LGTM! Proper usage of SwcJsMinimizerRspackPlugin
The minimizer configuration is correctly set up with appropriate options to remove comments in the production build.
60-63: LGTM! Proper migration of BannerPlugin
The BannerPlugin configuration has been correctly migrated to use Rspack's API while maintaining the same functionality.
41-43:
Incorrect 'tsConfig' property in 'resolve' configuration
The resolve configuration does not support a tsConfig property in Rspack. This needs to be configured properly for TypeScript path resolution.
Apply this diff to fix the configuration:
resolve: {
extensions: ['.tsx', '.ts', '.js', '.json'],
- tsConfig: {
- configFile: path.resolve('tsconfig.json'),
- },
+ alias: {
+ // Add your path aliases here based on tsconfig.json
+ },
},57-59: Verify the environment variable handling
The EnvironmentPlugin configuration has changed from array-style to object-style. While this is the correct format for Rspack, we should verify that the EE environment variable is properly handled in all environments.
✅ Verification successful
Environment variable configuration is correctly handled
The verification shows that the EE environment variable is consistently used across the codebase:
- All Rspack configurations (
rspack.*.config.js) use the same object-styleEnvironmentPluginconfiguration - The
process.env.EEusage in the codebase consistently checks for boolean values (=== 'true') - The variable is primarily used in test files for conditional test execution and in configuration files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for EE environment variable usage
rg -l "process\.env\.EE"
# Check if there are any other environment variable configurations
fd -e js -e ts -e json | xargs rg "EnvironmentPlugin|process\.env\.EE"
Length of output: 2201
2-3: Verify compatibility of webpack-node-externals with Rspack
The configuration is using webpack-node-externals with Rspack. While Rspack aims to be webpack-compatible, this might cause issues.
✅ Verification successful
Using webpack-node-externals with Rspack is supported and safe
Based on the search results and Rspack's webpack compatibility layer, using webpack-node-externals with Rspack is safe. Rspack maintains compatibility with webpack's externals configuration, and there isn't a dedicated Rspack-specific alternative available, indicating that the webpack version is the recommended approach.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there's a Rspack-specific alternative
curl -s https://registry.npmjs.org/-/v1/search?text=rspack%20node%20externals | jq '.objects[].package.name'
Length of output: 5998
cef8aa5 to
c874b68
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (6)
.github/workflows/release-timely-docker.ymlis excluded by!**/*.ymlpackages/nocodb/package.jsonis excluded by!**/*.jsonpackages/nocodb/tests/unit/tsconfig.ee.jsonis excluded by!**/*.jsonpackages/nocodb/tests/unit/tsconfig.jsonis excluded by!**/*.jsonpackages/nocodb/tsconfig.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (16)
build-local-docker-image.sh(1 hunks)packages/nocodb/docker/rspack.config.js(1 hunks)packages/nocodb/docker/webpack.config.js(0 hunks)packages/nocodb/rspack.cli.config.js(4 hunks)packages/nocodb/rspack.config.js(1 hunks)packages/nocodb/rspack.dev.config.js(1 hunks)packages/nocodb/rspack.timely.config.js(1 hunks)packages/nocodb/src/Noco.ts(1 hunks)packages/nocodb/src/controllers/integrations.controller.ts(1 hunks)packages/nocodb/src/controllers/org-users.controller.ts(1 hunks)packages/nocodb/src/db/sql-mgr/SqlMgr.ts(1 hunks)packages/nocodb/src/services/command-palette.service.ts(1 hunks)packages/nocodb/src/services/sql-views.service.ts(1 hunks)packages/nocodb/webpack.config.js(0 hunks)packages/nocodb/webpack.local.config.js(0 hunks)packages/nocodb/webpack.timely.config.js(0 hunks)
💤 Files with no reviewable changes (4)
- packages/nocodb/webpack.local.config.js
- packages/nocodb/webpack.config.js
- packages/nocodb/webpack.timely.config.js
- packages/nocodb/docker/webpack.config.js
✅ Files skipped from review due to trivial changes (1)
- packages/nocodb/src/controllers/integrations.controller.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- build-local-docker-image.sh
- packages/nocodb/src/controllers/org-users.controller.ts
- packages/nocodb/rspack.dev.config.js
- packages/nocodb/rspack.config.js
- packages/nocodb/src/db/sql-mgr/SqlMgr.ts
- packages/nocodb/docker/rspack.config.js
- packages/nocodb/rspack.timely.config.js
- packages/nocodb/src/services/command-palette.service.ts
- packages/nocodb/rspack.cli.config.js
🧰 Additional context used
🪛 Biome (1.9.4)
packages/nocodb/src/Noco.ts
[error] 106-106: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🔇 Additional comments (2)
packages/nocodb/src/services/sql-views.service.ts (1)
150-150: Verify if this change belongs in the Rspack migration PR
The addition of user_id tracking seems unrelated to the PR's objective of migrating from Webpack to Rspack. Consider moving this change to a separate PR focused on user tracking enhancements.
packages/nocodb/src/Noco.ts (1)
100-100: Fix the module declaration syntax
49111ee to
60d0049
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (9)
packages/nocodb/src/models/Model.ts (1)
1165-1165: LGTM! Using_prefix for unused parameter.The change follows TypeScript best practices by using the
_prefix to indicate an unused parameter.Consider removing the unused parameter entirely if it's not required by the method's contract or interface. If kept for backward compatibility, add a comment explaining why.
- userId: _,packages/nocodb/rspack.config.js (3)
22-22: Consider enabling source maps for better debuggingSource maps are currently disabled. While this might slightly improve build performance, it will make debugging more difficult in production environments.
51-63: Remove or clarify the commented instructionThe comment
//Update this to true or falsesuggests uncertainty about the optimization setting. This should be a deliberate choice:
- Set to
truefor production builds (smaller bundle size)- Set to
falsefor debugging or if you need readable output
87-89: Document the purpose of the EE environment variableThe
EEenvironment variable is set totruebut its purpose is not documented. Consider adding a comment explaining what this flag controls, as it might be important for different build configurations.packages/nocodb/src/services/views.service.ts (2)
Line range hint
183-236: Refactor view update logic for better maintainabilityThe view update logic handles multiple responsibilities (lock type changes and ownership transfers) with complex nested conditions. Consider splitting this into separate methods for better maintainability and testability.
class ViewsService { private async validateLockTypeChange(view: ViewUpdateReqType, oldView: View, user: UserType) { if (view.lock_type !== 'personal' || view.lock_type === oldView.lock_type) { return { ownedBy: oldView.owned_by, createdBy: oldView.created_by }; } // ... lock type validation logic } private async validateOwnershipTransfer(newOwnerId: string, currentOwnerId: string, user: UserType) { // ... ownership transfer validation logic } async viewUpdate(context: NcContext, param: {...}) { const { ownedBy, createdBy } = await this.validateLockTypeChange(param.view, oldView, param.user); if (param.view.owned_by) { await this.validateOwnershipTransfer(param.view.owned_by, ownedBy, param.user); } // ... update logic } }
Line range hint
229-232: Enhance error handling for ownership transferThe error handling for invalid users during ownership transfer could be more descriptive and include validation for edge cases.
- if (!baseUser) { - NcError.badRequest('Invalid user'); - } + if (!baseUser) { + NcError.badRequest( + `User ${param.view.owned_by} does not have access to the workspace` + ); + } + + // Validate if the new owner has sufficient permissions + if (!baseUser.roles?.[ProjectRoles.CREATOR] && !baseUser.roles?.[ProjectRoles.OWNER]) { + NcError.badRequest( + 'New owner must have Creator or Owner role in the workspace' + ); + }packages/nocodb/src/db/BaseModelSqlv2.ts (1)
6502-6505: Empty implementation of 'handleRichTextMentions'The method
handleRichTextMentionsis currently empty. If this is intentional and the method will be implemented later, consider adding a comment to explain its purpose. If not needed, it might be removed to clean up the code.packages/nocodb/src/modules/jobs/jobs-map.service.ts (1)
31-81: Consider caching '_jobMap' to improve performanceBy changing
_jobMapto a getter, the job map object will be recreated every time it is accessed, which may impact performance if accessed frequently.Consider initializing
_jobMaponce and storing it, or employing lazy initialization with caching to enhance performance.packages/nocodb/src/helpers/NcPluginMgrv2.ts (1)
133-135: Document new S3 configuration optionsThe addition of new S3 configuration options (
NC_S3_ENDPOINT,NC_S3_ACCESS_KEY,NC_S3_ACCESS_SECRET) should be documented in the project's documentation.Consider adding these environment variables to the documentation with examples and use cases, particularly highlighting:
- When to use
NC_S3_ENDPOINTvsNC_S3_REGION- Optional nature of access credentials
- Interaction with other S3 configuration options
Also applies to: 143-156
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (6)
.github/workflows/release-timely-docker.ymlis excluded by!**/*.ymlpackages/nocodb/package.jsonis excluded by!**/*.jsonpackages/nocodb/tests/unit/tsconfig.ee.jsonis excluded by!**/*.jsonpackages/nocodb/tests/unit/tsconfig.jsonis excluded by!**/*.jsonpackages/nocodb/tsconfig.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (30)
build-local-docker-image.sh(1 hunks)packages/nocodb/docker/rspack.config.js(1 hunks)packages/nocodb/docker/webpack.config.js(0 hunks)packages/nocodb/rspack.cli.config.js(1 hunks)packages/nocodb/rspack.config.js(1 hunks)packages/nocodb/rspack.dev.config.js(1 hunks)packages/nocodb/rspack.timely.config.js(1 hunks)packages/nocodb/src/Noco.ts(1 hunks)packages/nocodb/src/controllers/integrations.controller.ts(1 hunks)packages/nocodb/src/controllers/org-users.controller.ts(1 hunks)packages/nocodb/src/db/BaseModelSqlv2.ts(3 hunks)packages/nocodb/src/db/sql-mgr/SqlMgr.ts(1 hunks)packages/nocodb/src/db/sql-migrator/lib/templates/mssql.template.ts(2 hunks)packages/nocodb/src/helpers/NcPluginMgrv2.ts(1 hunks)packages/nocodb/src/interface/Jobs.ts(2 hunks)packages/nocodb/src/models/Comment.ts(1 hunks)packages/nocodb/src/models/Model.ts(1 hunks)packages/nocodb/src/modules/jobs/jobs-map.service.ts(1 hunks)packages/nocodb/src/modules/jobs/jobs/export-import/duplicate.processor.ts(4 hunks)packages/nocodb/src/modules/jobs/jobs/export-import/export.service.ts(1 hunks)packages/nocodb/src/plugins/gcs/Gcs.ts(2 hunks)packages/nocodb/src/services/command-palette.service.ts(1 hunks)packages/nocodb/src/services/public-datas.service.ts(1 hunks)packages/nocodb/src/services/sql-views.service.ts(1 hunks)packages/nocodb/src/services/tables.service.ts(0 hunks)packages/nocodb/src/services/views.service.ts(1 hunks)packages/nocodb/webpack.cli.config.js(0 hunks)packages/nocodb/webpack.config.js(0 hunks)packages/nocodb/webpack.local.config.js(0 hunks)packages/nocodb/webpack.timely.config.js(0 hunks)
💤 Files with no reviewable changes (6)
- packages/nocodb/webpack.config.js
- packages/nocodb/webpack.timely.config.js
- packages/nocodb/webpack.local.config.js
- packages/nocodb/webpack.cli.config.js
- packages/nocodb/docker/webpack.config.js
- packages/nocodb/src/services/tables.service.ts
✅ Files skipped from review due to trivial changes (2)
- packages/nocodb/src/interface/Jobs.ts
- packages/nocodb/src/models/Comment.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- build-local-docker-image.sh
- packages/nocodb/src/controllers/org-users.controller.ts
- packages/nocodb/rspack.dev.config.js
- packages/nocodb/src/controllers/integrations.controller.ts
- packages/nocodb/src/services/command-palette.service.ts
- packages/nocodb/src/db/sql-mgr/SqlMgr.ts
- packages/nocodb/docker/rspack.config.js
- packages/nocodb/rspack.timely.config.js
👮 Files not reviewed due to content moderation or server errors (3)
- packages/nocodb/src/modules/jobs/jobs/export-import/duplicate.processor.ts
- packages/nocodb/src/services/public-datas.service.ts
- packages/nocodb/src/modules/jobs/jobs/export-import/export.service.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/nocodb/src/Noco.ts
[error] 106-106: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🔇 Additional comments (12)
packages/nocodb/rspack.config.js (3)
79-81: Verify UMD build configuration
The UMD configuration with global object fallback needs verification:
- Library name 'libs' might be too generic
- Global object fallback might need testing in different environments (browser, Node.js, etc.)
✅ Verification successful
UMD configuration is consistent across packages
The UMD configuration with library name 'libs' and global object fallback is consistently used across all packages in the monorepo (nocodb, nc-cli, nc-secret-mgr). This standardization indicates an intentional pattern rather than a concern:
- The library name 'libs' is used uniformly in all UMD configurations
- The global object fallback
typeof self !== 'undefined' ? self : thisis a standard pattern used consistently - No direct imports of the UMD bundle were found, suggesting it's used as intended
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other UMD configurations in the project
rg "libraryTarget: ['|\"]umd['|\"]" -t js -t ts
# Check for usage of the 'libs' global
rg "global\.libs" -t js -t ts
Length of output: 470
Script:
#!/bin/bash
# Check library names in UMD configurations
rg "library: ['|\"].*['|\"]" -A 2 -t js -t ts
# Check global object configurations
rg "globalObject.*" -t js -t ts
# Check if there are any imports or requires of the UMD bundle
rg "require\(['|\"]@nocodb/nocodb['|\"]|import.*from ['|\"]@nocodb/nocodb['|\"]" -t js -t ts
Length of output: 2157
3-4: Consider replacing Webpack-specific dependencies with Rspack alternatives
The configuration is using Webpack-specific packages with Rspack:
webpack-node-externalsfork-ts-checker-webpack-plugin
While these might work due to Webpack compatibility layer, it's recommended to use Rspack-native alternatives if available to ensure long-term maintainability.
✅ Verification successful
Based on the repository analysis, I can now generate the final response:
Webpack-specific dependencies are compatible with Rspack
The usage of webpack-node-externals and fork-ts-checker-webpack-plugin is acceptable in this context because:
- Rspack maintains webpack compatibility layer for plugins and loaders
- The repository is actively using these dependencies in multiple Rspack configurations
- The
webpack-node-externalsis being used successfully in other Rspack config files (e.g., docker/rspack.config.js) - The TypeScript checking functionality provided by
fork-ts-checker-webpack-pluginis essential and works with Rspack
While Rspack-native alternatives might become available in the future, the current setup is working as intended and doesn't require immediate changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Rspack provides native alternatives
curl -s https://www.rspack.dev/api/ | grep -i "externals\|typescript"
Length of output: 71
Script:
#!/bin/bash
# Let's check the actual Rspack configuration documentation and examples in the repository
fd -t f "rspack.config" -X cat {}
# Also check package.json for dependencies
fd "package.json" -X cat {}
Length of output: 37801
40-43: Review module strict mode configuration
There's an inconsistency in the strict mode settings:
strictModeis set totruestrictis set tofalse
This might lead to unexpected behavior. Consider aligning these settings based on your requirements.
✅ Verification successful
Strict mode settings are consistent with rspack configuration
The strict mode settings in the rspack configuration are actually consistent and serve different purposes:
strict: false- Disables Node.js strict mode for CommonJS modulesstrictMode: true- Enables JavaScript strict mode for the generated code
These settings work together to control different aspects of strict mode behavior - one for Node.js module system and another for JavaScript code execution. No changes needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for strict mode usage in the codebase
rg "('|\")?use strict('|\")?" -t ts -t tsx
Length of output: 76
Script:
#!/bin/bash
# Check for strict mode usage in JavaScript and TypeScript files
rg "('|\")?use strict('|\")?" -t js -t ts
# Check webpack/rspack configurations for strict mode settings
fd -e js -g "*pack.config.js" --exec cat {}
Length of output: 79770
packages/nocodb/src/services/views.service.ts (2)
1-1: Verify PR scope: View management changes vs Rspack migration
The changes in this file appear to be significant modifications to view management logic, which seems unrelated to the PR's stated objective of migrating from Webpack to Rspack. Consider:
- Moving these changes to a separate PR focused on view management improvements
- Or updating the PR description to reflect these additional changes
Line range hint 124-137: Verify security implications of removed personal view filtering
The commented-out code block suggests that personal view filtering has been removed. This change could potentially expose personal views to unauthorized users.
packages/nocodb/rspack.cli.config.js (2)
62-66: Verify compatibility of 'webpack-node-externals' with Rspack
Please ensure that webpack-node-externals is compatible with Rspack. It may not function as expected since Rspack's compatibility with some Webpack plugins is limited.
69-71: 🛠️ Refactor suggestion
Incorrect 'tsConfig' property in 'resolve' configuration
The resolve configuration does not support a tsConfig property. To handle TypeScript path aliases, you should configure the alias property or use appropriate plugins.
Apply this diff to fix the configuration:
resolve: {
extensions: ['.tsx', '.ts', '.js', '.json'],
- tsConfig: {
- configFile: resolve('tsconfig.json'),
- },
+ alias: {
+ // Add your path aliases here
+ },
},Alternatively, consider using the TsconfigPathsPlugin to automatically map path aliases from your tsconfig.json:
const TsconfigPathsPlugin = require('tsconfig-paths-webpack-plugin');
resolve: {
extensions: ['.tsx', '.ts', '.js', '.json'],
plugins: [
new TsconfigPathsPlugin({ configFile: resolve('tsconfig.json') })
],
},packages/nocodb/src/db/BaseModelSqlv2.ts (1)
Line range hint 5337-5360: Updates to 'bulkUpsert' method are appropriate
The inclusion of prepareNocoData calls in the bulkUpsert method correctly ensures data preparation for both insertions and updates.
packages/nocodb/src/services/sql-views.service.ts (1)
150-150: User ID inclusion and validation enhancements
Adding user_id to associate the model with the user is appropriate. The additional validation for leading or trailing whitespace in view_name and checking the name length based on the SQL client type improves robustness.
packages/nocodb/src/Noco.ts (2)
109-112: 🛠️ Refactor suggestion
Configure Rspack-specific HMR options
The current HMR implementation can be enhanced using Rspack-specific features for better performance and reliability.
- if ((module as any).hot) {
- (module as any).hot.accept();
- (module as any).hot.dispose(() => nestApp.close());
- }
+ // Enable Rspack HMR with proper typing
+ if (module.hot) {
+ // Use self-accepting mode for top-level modules
+ module.hot.accept();
+
+ // Implement proper cleanup with Rspack's dispose handler
+ module.hot.dispose(async () => {
+ console.log('🔄 Hot Module Replacement triggered');
+ await nestApp.close();
+ });
+ }Likely invalid or redundant comment.
100-101:
Remove unnecessary module declaration
The declare module: any is incorrect TypeScript syntax and unnecessarily bypasses type checking. Since we're migrating to Rspack, we should properly type the HMR-related declarations.
Move this to a separate declarations file (e.g., types.d.ts):
-declare module: any;
+// Add to a separate declarations file (e.g., types.d.ts):
+declare var module: {
+ hot?: {
+ accept(dependencies?: string[], callback?: () => void): void;
+ dispose(callback: (data: any) => void): void;
+ };
+};Likely invalid or redundant comment.
packages/nocodb/src/plugins/gcs/Gcs.ts (1)
38-38: LGTM! Type safety improvement
The change from empty object type to Record<string, never> improves type safety by being more explicit about the emp
FC92
ty object type.
Also applies to: 183-183
Change Summary
Migrate from Webpack to Rspack 🦀
Change type