feat: use @coze-infra/rslib-config #41
Conversation
WalkthroughThis pull request introduces two new configuration packages, Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Config as @coze-infra/rslib-config
participant Build as Build System
Dev->>Config: Import configuration
Config-->>Dev: Provide standardized config
Dev->>Build: Execute build/lint
Build->>Config: Retrieve build settings
Config-->>Build: Return configuration details
Possibly related PRs
Suggested Reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
config/rslib-config/index.ts (2)
32-34: Consider making DTS configuration customizableThe DTS output path is currently hardcoded. Consider making it configurable through the Options interface for better flexibility.
interface Options { format?: LibFormat[]; bundle?: BundleType; tsconfigPath?: string; umdName?: string; + dtsPath?: string; } // In getRslibConfig: libs[0].dts = { - distPath: './dist/types', + distPath: options.dtsPath ?? './dist/types', };
44-57: Consider adding JSDoc documentation for the helper functionThe
getLibSharedfunction is well-implemented but lacks documentation. Consider adding JSDoc to describe the parameters and return value.+/** + * Creates a shared library configuration with specified format and bundle settings + * @param format - The output format (esm, cjs, or umd) + * @param bundleType - Bundle configuration (true, false, or 'excludeExternal') + * @returns LibConfig object with shared configuration + */ function getLibShared(format: LibFormat, bundleType: BundleType)packages/coze-js/rslib.config.ts (1)
1-10: Add documentation for bundle configurationWhile the configuration is correct, consider adding a comment explaining why
excludeExternalis used for this package specifically.export default defineConfig( getRslibConfig({ + // Exclude external dependencies from the bundle to reduce size + // and allow consumers to manage their own dependency versions format: ['esm', 'cjs', 'umd'], bundle: 'excludeExternal', umdName: 'CozeJs', tsconfigPath: './tsconfig.build.json', }), );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
common/config/subspaces/default/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
config/rslib-config/eslint.config.js(1 hunks)config/rslib-config/index.ts(1 hunks)config/rslib-config/package.json(1 hunks)packages/coze-js/package.json(1 hunks)packages/coze-js/rslib.config.ts(1 hunks)packages/realtime-api/package.json(1 hunks)packages/realtime-api/rslib.config.ts(1 hunks)rush.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- config/rslib-config/eslint.config.js
- config/rslib-config/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
rush.json
[error] 384-387: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 387-387: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
config/rslib-config/index.ts
[error] 4-4: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
🔇 Additional comments (5)
config/rslib-config/index.ts (1)
6-11: LGTM: Well-structured configuration interface with sensible defaults
The Options interface and defaultOptions are well-designed with:
- Optional parameters for flexibility
- Type safety through LibFormat
- Reasonable defaults for common use cases
Also applies to: 12-15
packages/realtime-api/rslib.config.ts (1)
1-9: LGTM: Clean migration to centralized configuration
The configuration is correctly migrated to use @coze-infra/rslib-config with appropriate settings for the realtime API package.
Let's verify the UMD name consistency across the codebase:
✅ Verification successful
UMD name 'CozeRealtimeApi' is uniquely defined and consistent
The UMD name is only used in the configuration file and not referenced elsewhere in the TypeScript codebase, which is the expected behavior as this name is primarily used for the UMD bundle generation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any other occurrences of CozeRealtimeApi to ensure naming consistency
rg "CozeRealtimeApi" --type ts
Length of output: 100
packages/coze-js/package.json (1)
59-59: LGTM!
The addition of @coze-infra/rslib-config as a workspace dependency is correctly implemented and aligns with the project's dependency management pattern.
packages/realtime-api/package.json (1)
57-57: LGTM!
The addition of @coze-infra/rslib-config as a workspace dependency is correctly implemented and aligns with the project's dependency management pattern.
rush.json (1)
384-387: LGTM!
The addition of @coze-infra/rslib-config project entry is correctly implemented, following the same pattern as other infrastructure packages in the config directory.
🧰 Tools
🪛 Biome (1.9.4)
[error] 384-387: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 387-387: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
wfc-web
force-pushed
the
feat-rslib-share
branch
from
17de69d to
69d92c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
config/rslib-config/index.ts (1)
32-34: Document DTS configuration behaviorThe DTS configuration is only set for the first library in the array. This behavior should be documented to explain why only the first library receives the DTS configuration.
+ // DTS is only generated once and shared across all formats libs[0].dts = { distPath: './dist/types', };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
common/config/subspaces/default/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
config/rslib-config/eslint.config.js(1 hunks)config/rslib-config/index.ts(1 hunks)config/rslib-config/package.json(1 hunks)packages/coze-js/package.json(1 hunks)packages/coze-js/rslib.config.ts(1 hunks)packages/realtime-api/package.json(1 hunks)packages/realtime-api/rslib.config.ts(1 hunks)rush.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- config/rslib-config/eslint.config.js
- packages/coze-js/package.json
- packages/realtime-api/package.json
- packages/coze-js/rslib.config.ts
- packages/realtime-api/rslib.config.ts
- config/rslib-config/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
config/rslib-config/index.ts
[error] 4-4: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
rush.json
[error] 401-404: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 404-404: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (3)
config/rslib-config/index.ts (2)
4-4: Use lowercase 'boolean' type instead of 'Boolean'
The BundleType type uses the Boolean type which is not recommended. Use the lowercase primitive boolean for consistency with TypeScript best practices.
🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
44-57: LGTM! Clean and well-structured implementation
The function follows good practices:
- Pure function with no side effects
- Clear handling of bundle and autoExternal settings
- Consistent configuration structure
rush.json (1)
401-404: LGTM! Project entry follows established patterns
The new project entry for @coze-infra/rslib-config is correctly structured and placed appropriately within the config projects section.
🧰 Tools
🪛 Biome (1.9.4)
[error] 401-404: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 404-404: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
config/rslib-config/index.ts (3)
12-15: Add JSDoc comment for defaultOptionsConsider adding documentation to explain the default configuration choices.
+/** + * Default configuration options for library builds. + * - format: Outputs both ESM and CommonJS modules + * - bundle: Enables bundling by default + */ const defaultOptions = { format: ['esm', 'cjs'] as LibFormat[], bundle: true, };
37-39: Document DTS configuration behaviorThe DTS (TypeScript declaration) configuration is only set for the first format in the array. This behavior should be documented to prevent confusion.
+ // TypeScript declarations are only generated once and shared across all formats libs[0].dts = { distPath: './dist/types', };
49-62: Make bundle type conversion more explicitThe bundle type conversion logic could be more explicit for better maintainability.
function getLibShared(format: LibFormat, bundleType: BundleType) { + // Convert bundleType to specific configuration options + const shouldBundle = !!bundleType; + const shouldExcludeExternal = bundleType === 'excludeExternal'; + const shared: LibConfig = { output: { distPath: { root: `./dist/${format}`, }, }, format, syntax: 'es6', - bundle: !!bundleType, - autoExternal: bundleType === 'excludeExternal', + bundle: shouldBundle, + autoExternal: shouldExcludeExternal, }; return shared; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
config/rslib-config/index.ts(1 hunks)packages/coze-js/rslib.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/coze-js/rslib.config.ts
🔇 Additional comments (3)
config/rslib-config/index.ts (3)
1-11: LGTM! Well-structured type definitions
The type definitions are clean, properly exported, and follow TypeScript best practices. The Options interface provides a clear contract for the configuration function.
23-24: Remove unsafe non-null assertion
The bundle parameter could be undefined, making the non-null assertion unsafe.
1-64: Verify consistent usage across packages
As this is a new shared configuration module, we should verify its usage across different packages.
✅ Verification successful
Configuration usage is consistent across packages
The verification shows that getRslibConfig is being used consistently across packages:
- Both
coze-jsandrealtime-apipackages properly import and use the configuration - Configuration options align with the supported features:
- Both use all formats (esm, cjs, umd)
- UMD name is provided where required
- Bundle options are correctly specified
- Typescript config paths are properly set
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of getRslibConfig across packages
# Find all rslib config files
echo "Searching for rslib.config.ts files..."
fd -e ts -p "rslib.config.ts$"
# Check import patterns
echo -e "\nChecking import patterns..."
rg -A 2 "from ['|\"]@coze-infra/rslib-config" --type ts
# Check configuration usage
echo -e "\nChecking getRslibConfig usage patterns..."
ast-grep --pattern 'getRslibConfig({$$$})'
Length of output: 1531
wfc-web
force-pushed
the
feat-rslib-share
branch
from
1fd4cb6 to
2feb5e7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
config/rslib-config/index.ts (3)
12-15: Consider documenting the rationale for default optionsAdding JSDoc comments explaining why 'esm' and 'cjs' are the default formats would improve maintainability.
+/** + * Default configuration options. + * - format: Includes both ESM and CommonJS for maximum compatibility + * - bundle: Enabled by default for optimal package distribution + */ const defaultOptions = { format: ['esm', 'cjs'] as LibFormat[], bundle: true, };
17-47: Consider enhancing error handling and configuration flexibility
- The error message could be more descriptive about the UMD requirements
- The DTS configuration could be made configurable through options
- Consider adding validation for required parameters
interface Options { format?: LibFormat[]; bundle?: BundleType; tsconfigPath?: string; umdName?: string; + dts?: { + distPath: string; + }; } function getRslibConfig(options: Options) { - const { format, bundle, umdName, tsconfigPath } = { + const { format, bundle, umdName, tsconfigPath, dts } = { ...defaultOptions, ...options, }; + if (!format?.length) { + throw new Error('getRslibConfig: At least one format must be specified'); + } const libs = format.map(libFormat => { const lib = getLibShared(libFormat, bundle); if (libFormat === 'umd') { if (!umdName) { throw new Error( - 'getRslibConfig: umdName is required when using UMD format', + 'getRslibConfig: umdName is required when using UMD format. This name will be used as the global variable name in the UMD bundle.', ); } lib.umdName = umdName; lib.bundle = true; } return lib; }); - libs[0].dts = { - distPath: './dist/types', - }; + if (libs.length > 0) { + libs[0].dts = dts ?? { + distPath: './dist/types', + }; + } return defineConfig({ source: { tsconfigPath, }, lib: libs, }); }
49-62: Add JSDoc documentation for the helper functionAdding documentation would improve maintainability by explaining the purpose and configuration options.
+/** + * Creates a shared library configuration for a specific format. + * @param format - The output format (e.g., 'esm', 'cjs', 'umd') + * @param bundleType - Controls bundling behavior: + * - true: Bundle all dependencies + * - false: No bundling + * - 'excludeExternal': Bundle but exclude external dependencies + * @returns LibConfig object with the specified format and bundling settings + */ function getLibShared(format: LibFormat, bundleType: BundleType) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
common/config/subspaces/default/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
config/rslib-config/eslint.config.js(1 hunks)config/rslib-config/index.ts(1 hunks)config/rslib-config/package.json(1 hunks)packages/coze-js/package.json(1 hunks)packages/coze-js/rslib.config.ts(1 hunks)packages/realtime-api/package.json(1 hunks)packages/realtime-api/rslib.config.ts(1 hunks)rush.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- config/rslib-config/eslint.config.js
- packages/coze-js/package.json
- packages/realtime-api/package.json
- packages/realtime-api/rslib.config.ts
- packages/coze-js/rslib.config.ts
- config/rslib-config/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
rush.json
[error] 401-404: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 404-404: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (3)
config/rslib-config/index.ts (2)
1-11: LGTM! Well-structured type definitions
The type definitions are clear, properly typed, and provide good type safety for the configuration parameters.
64-64: LGTM! Proper module export
The default export is appropriate for this configuration module.
rush.json (1)
401-404: LGTM! Proper project configuration
The new project entry follows the established pattern and includes all required fields.
🧰 Tools
🪛 Biome (1.9.4)
[error] 401-404: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 404-404: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
config/rslib-config/src/index.ts (4)
14-15: Consider adding validation for target valuesThe
targetoption accepts any value ofRsbuildConfigOutputTargettype, but there's no runtime validation to ensure the provided value is valid.Consider adding a type guard or validation:
const validTargets = ['web', 'node'] as const; type ValidTarget = typeof validTargets[number]; function isValidTarget(target: unknown): target is ValidTarget { return typeof target === 'string' && validTargets.includes(target as ValidTarget); }
17-21: Use const assertions for better type inferenceThe current type assertion could be replaced with a const assertion for better type safety.
- format: ['esm', 'cjs'] as LibFormat[], + format: ['esm', 'cjs'] as const,
58-71: Enhance type safety and validation in shared configurationThe shared configuration could benefit from more explicit type handling and validation.
Consider these improvements:
type SharedConfig = Pick<LibConfig, 'output' | 'format' | 'syntax' | 'bundle' | 'autoExternal'>; function getLibShared(format: LibFormat, bundleType: BundleType): SharedConfig { if (!format) { throw new Error('Format is required for shared configuration'); } const bundle = bundleType === 'excludeExternal' ? true : !!bundleType; return { output: { distPath: { root: `./dist/${format}`, }, }, format, syntax: 'es6', bundle, autoExternal: bundleType === 'excludeExternal', }; }
73-73: Consider using named exports for better tree-shakingWhile default export works, named exports can provide better tree-shaking opportunities and explicit imports.
-export default getRslibConfig; +export { getRslibConfig };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
config/rslib-config/src/index.ts(1 hunks)config/rslib-config/tsconfig.json(1 hunks)packages/coze-js/rslib.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- config/rslib-config/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/coze-js/rslib.config.ts
🔇 Additional comments (1)
config/rslib-config/src/index.ts (1)
1-73: Verify integration with existing configurations
As this is a new shared configuration module, we should verify its integration with existing configurations and the build system.
Consider adding integration tests and documentation for common configuration patterns to help other teams adopt this shared configuration.
✅ Verification successful
Let me gather more information about the existing configuration usage to make a more informed decision.
Let me check the content of the existing rslib.config.ts files to understand their current configuration patterns.
Integration with existing configurations verified successfully
The new shared configuration module is already being used correctly in existing packages (realtime-api and coze-js) with proper integration:
- Both packages import and use
getRslibConfigfrom@coze-infra/rslib-config - The configuration options match the new API (format, bundle, umdName, tsconfigPath)
- All packages are using compatible versions of
@rslib/core(0.0.18) - The implementation maintains backward compatibility as evidenced by the existing usage patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check existing rslib configurations that might need updates
echo "Checking existing rslib configurations..."
rg -l "rslib\.config\.(ts|js)"
# Check package dependencies
echo "Checking package dependencies..."
fd -g "package.json" -x jq -r '.dependencies["@rslib/core"] // .devDependencies["@rslib/core"] // empty'
Length of output: 399
Script:
#!/bin/bash
# Check the content of existing config files to understand usage patterns
echo "Content of vitest.config.ts files:"
cat packages/realtime-api/vitest.config.ts packages/coze-js/vitest.config.ts
# Check if there are any other build configurations that might be affected
echo -e "\nChecking for other build configurations:"
rg -l "defineConfig|LibConfig" --type ts
Length of output: 1145
Script:
#!/bin/bash
# Check the content of existing rslib config files
echo "Content of existing rslib.config.ts files:"
cat packages/realtime-api/rslib.config.ts packages/coze-js/rslib.config.ts
# Check if there are any imports of @rslib/core in these files
echo -e "\nChecking @rslib/core imports in these files:"
rg "@rslib/core" packages/realtime-api/rslib.config.ts packages/coze-js/rslib.config.ts
Length of output: 1077
wfc-web
force-pushed
the
feat-rslib-share
branch
from
0867f52 to
4b61a2a
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 (1)
common/config/subspaces/default/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
config/rslib-config/eslint.config.js(1 hunks)config/rslib-config/package.json(1 hunks)config/rslib-config/src/index.ts(1 hunks)config/rslib-config/tsconfig.json(1 hunks)packages/coze-js/package.json(1 hunks)packages/coze-js/rslib.config.ts(1 hunks)packages/realtime-api/package.json(1 hunks)packages/realtime-api/rslib.config.ts(1 hunks)rush.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/coze-js/package.json
- config/rslib-config/eslint.config.js
- config/rslib-config/tsconfig.json
- packages/realtime-api/package.json
- packages/realtime-api/rslib.config.ts
- packages/coze-js/rslib.config.ts
- config/rslib-config/package.json
- config/rslib-config/src/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
rush.json
[error] 405-408: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 408-408: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (1)
rush.json (1)
405-408: LGTM! Project configuration follows established patterns.
The new project entry for @coze-infra/rslib-config is well-structured and consistent with other configuration packages in the monorepo:
- Follows the established naming convention
@coze-infra/* - Placed in the
config/directory alongside other configuration packages - Maintains consistent structure with other project entries
🧰 Tools
🪛 Biome (1.9.4)
[error] 405-408: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 408-408: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
feat: use @coze-infra/rslib-config feat: rush update feat: comments feat: tsconfig.json feat: src feat: add target web feat: output target feat: default tsconfig path feat: rush update
wfc-web
force-pushed
the
feat-rslib-share
branch
from
4b61a2a to
9f43aed
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
config/rsbuild-config/src/index.ts (1)
1-16: Consider adding documentation and type exports.As this is a shared configuration package, it would benefit from:
- JSDoc comments explaining the purpose and usage of the configuration
- Type exports for custom configuration options
- A README.md file with setup instructions and configuration options
Example documentation:
+/** + * Base Rsbuild configuration for Coze projects + * @description Provides common build settings including React support and browser polyfills + * @example + * ```ts + * import baseRsbuildConfig from '@coze-infra/rsbuild-config'; + * export default { + * ...baseRsbuildConfig, + * // Add your project-specific settings + * }; + * ``` + */ const baseRsbuildConfig = defineConfig({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
common/config/subspaces/default/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
config/rsbuild-config/eslint.config.js(1 hunks)config/rsbuild-config/package.json(1 hunks)config/rsbuild-config/src/index.ts(1 hunks)config/rsbuild-config/tsconfig.json(1 hunks)examples/coze-js-web/package.json(2 hunks)examples/coze-js-web/rsbuild.config.ts(1 hunks)examples/coze-js-web/src/App.tsx(1 hunks)examples/coze-js-web/tsconfig.json(1 hunks)rush.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- config/rsbuild-config/eslint.config.js
- config/rsbuild-config/tsconfig.json
- examples/coze-js-web/rsbuild.config.ts
- config/rsbuild-config/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
rush.json
[error] 405-408: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 408-408: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 409-412: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 412-412: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (8)
examples/coze-js-web/tsconfig.json (2)
19-19: LGTM! Including rsbuild.config.ts for type checking.
The addition of "rsbuild.config.ts" to the includes array is appropriate as it ensures TypeScript will type-check the build configuration file.
19-19: Verify rsbuild.config.ts setup
Let's verify that the rsbuild.config.ts file exists and is properly configured with the new @coze-infra packages.
✅ Verification successful
rsbuild.config.ts is properly configured with @coze-infra/rsbuild-config
The file exists and correctly imports the base configuration from @coze-infra/rsbuild-config/src, extending it using defineConfig from @rsbuild/core. The setup follows the expected pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and configuration of rsbuild.config.ts
# Check if the file exists and show its content
echo "Checking rsbuild.config.ts existence and content:"
cat examples/coze-js-web/rsbuild.config.ts
# Verify imports of @coze-infra packages
echo -e "\nChecking @coze-infra package imports:"
rg -l "@coze-infra/(rslib|rsbuild)-config" examples/coze-js-web/rsbuild.config.ts
Length of output: 525
rush.json (1)
405-412: LGTM! Project configuration follows established patterns.
The new packages @coze-infra/rslib-config and @coze-infra/rsbuild-config are correctly configured following the monorepo's conventions:
- Package naming follows the
@coze-infra/*scope - Project folders are placed in the
config/directory alongside other configuration packages
Let's verify the package dependencies are properly configured:
✅ Verification successful
Package configurations are properly set up and dependencies are correctly configured
Both new packages are well-configured with:
- Correct package names and versions
- Proper main and types entry points
- Required dependencies:
rslib-configdepends on@rslib/corersbuild-configdepends on@rsbuild/coreand related plugins
- Workspace dependencies for shared configs (
eslint-config,ts-config) - Consistent dev tooling setup
Dependencies are properly configured:
@coze/apiand@coze/realtime-apicorrectly use workspace dependencies forrslib-configexamples/coze-js-webis the only package depending onrsbuild-config, which is appropriate for a build configuration package
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify package configuration and dependencies for the new packages
echo "Checking rslib-config package.json..."
cat config/rslib-config/package.json 2>/dev/null | jq -r '.name' || echo "package.json not found"
echo "Checking rsbuild-config package.json..."
cat config/rsbuild-config/package.json 2>/dev/null | jq -r '.name' || echo "package.json not found"
echo "Checking dependent packages..."
echo "Checking @coze/api dependencies:"
cat packages/coze-js/package.json | jq '.dependencies."@coze-infra/rslib-config" + .devDependencies."@coze-infra/rslib-config"'
echo "Checking @coze/realtime-api dependencies:"
cat packages/realtime-api/package.json | jq '.dependencies."@coze-infra/rslib-config" + .devDependencies."@coze-infra/rslib-config"'
Length of output: 865
Script:
#!/bin/bash
# Let's verify the package.json contents of both new packages to ensure they have proper configuration
echo "Checking rslib-config package.json full content:"
cat config/rslib-config/package.json | jq '.'
echo -e "\nChecking rsbuild-config package.json full content:"
cat config/rsbuild-config/package.json | jq '.'
# Also check if any other packages depend on rsbuild-config
echo -e "\nChecking packages depending on rsbuild-config:"
fd package.json --type f | xargs grep -l "@coze-infra/rsbuild-config"
Length of output: 1962
🧰 Tools
🪛 Biome (1.9.4)
[error] 405-408: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 408-408: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 409-412: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 412-412: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
config/rsbuild-config/src/index.ts (3)
1-2: LGTM! Imports are correctly defined.
The necessary imports from @rsbuild/core and @rsbuild/plugin-react are properly defined.
16-16: LGTM! Export is properly defined.
The base configuration is correctly exported as the default export.
4-15: Verify the necessity of the process polyfill.
The configuration includes a polyfill for the process object. This is commonly needed when code relies on Node.js-specific APIs in the browser environment.
Consider environment-specific configurations.
The current configuration might benefit from environment-specific settings (development vs production).
Consider extending the configuration with environment checks:
const baseRsbuildConfig = defineConfig({
plugins: [pluginReact()],
tools: {
rspack: (_, { rspack, appendPlugins }) => {
appendPlugins([
new rspack.ProvidePlugin({
process: [require.resolve('process/browser')],
}),
]);
+ // Add environment-specific optimizations
+ if (process.env.NODE_ENV === 'production') {
+ appendPlugins([
+ // Add production-specific plugins
+ ]);
+ }
},
},
});✅ Verification successful
Process polyfill is necessary for browser environment
The process polyfill is required as the codebase shows active usage of process.env in browser-based applications:
- Web applications use
process.env.PUBLIC_URLfor routing inexamples/coze-js-web/src/App.tsxandexamples/realtime-console/src/App.tsx - Environment checks with
process.env.NODE_ENVare used for configuration - The applications are browser-based (evidenced by
windowusage) where Node.jsprocessobject isn't natively available
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of process.* in the codebase to verify if the polyfill is necessary
rg "process\." --type ts --type tsx -l
Length of output: 68
Script:
#!/bin/bash
# Search for usage of process.* in the codebase with ripgrep, including all possible file extensions
rg "process\." --type-add 'ts:*.{ts,tsx}' --type ts -l
# Also check for any imports or requires of process-related modules
rg "(?:import|require).+process" --type-add 'ts:*.{ts,tsx}' --type ts -l
# Look for environment checks that might indicate process usage
rg "process\.env" --type-add 'ts:*.{ts,tsx}' --type ts -l
Length of output: 3018
Script:
#!/bin/bash
# Let's check the specific examples and web-related files for process usage patterns
rg "process\." -A 3 -B 3 examples/coze-js-web/src/App.tsx examples/realtime-console/src/App.tsx
# Check if there are any browser-specific environment checks
rg "typeof window|window\." --type-add 'ts:*.{ts,tsx}' --type ts examples/coze-js-web/ examples/realtime-console/
# Look for any Node.js specific API usage that might require the process polyfill
rg "process\.env\.NODE_ENV|process\.env\.PROD|process\.env\.DEV" --type-add 'ts:*.{ts,tsx}' --type ts examples/
Length of output: 2311
examples/coze-js-web/src/App.tsx (1)
10-10: Verify routing behavior in different environments
The addition of the empty string fallback for PUBLIC_URL is a good defensive programming practice. However, we should verify that this change doesn't affect routing behavior in various deployment scenarios.
✅ Verification successful
Router basename change is consistent with project configuration
The verification shows that the addition of the empty string fallback for PUBLIC_URL in examples/coze-js-web/src/App.tsx is safe and consistent with the project's setup:
- The
PUBLIC_URLenvironment variable is properly configured in the build system (webpack configs and env.js) - Other examples in the codebase (realtime-console) also use
PUBLIC_URLfor router basename - The routes defined in App.tsx are simple paths (/chat, /voice, /chat-x) that will work correctly with or without a basename
- The test environment explicitly sets
PUBLIC_URL = '', confirming that empty string is a valid fallback
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other routing-related configurations that might be affected
# Look for other instances of PUBLIC_URL usage
rg "PUBLIC_URL" --type ts --type tsx --type js --type jsx
# Check for any route definitions that might be affected
rg "Route.*path=" --type ts --type tsx
Length of output: 154
Script:
#!/bin/bash
# Description: Check for other routing-related configurations that might be affected
# Look for other instances of PUBLIC_URL usage without file type restrictions
rg "PUBLIC_URL"
# Check for any route definitions that might be affected
rg "Route.*path="
# Find all router-related files
fd -e tsx -e ts -e jsx -e js | rg -i "route|nav"
Length of output: 7165
examples/coze-js-web/package.json (1)
6-6: LGTM: Build script migration to rsbuild
The migration from custom scripts to rsbuild commands simplifies the build configuration.
Also applies to: 8-8
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
config/rslib-config/src/index.ts (2)
21-26: Improve type inference and configuration flexibilityConsider these improvements:
- Remove unnecessary type assertions as TypeScript can infer these types
- Make tsconfig path more flexible by supporting environment-based configuration
Consider this implementation:
const defaultOptions = { - format: ['esm', 'cjs'] as LibFormat[], - bundle: true, - target: 'web' as RsbuildConfigOutputTarget, - tsconfigPath: './tsconfig.build.json', + format: ['esm', 'cjs'], + bundle: true, + target: 'web', + tsconfigPath: process.env.RSLIB_TSCONFIG_PATH ?? './tsconfig.build.json', } satisfies Required<Pick<Options, 'format' | 'bundle' | 'target' | 'tsconfigPath'>>;
63-76: Improve bundle type handlingThe bundle type handling could be more explicit and type-safe.
Consider this implementation:
function getLibShared(format: LibFormat, bundleType: BundleType) { + const getBundleConfig = (type: BundleType): Pick<LibConfig, 'bundle' | 'autoExternal'> => { + switch (type) { + case 'excludeExternal': + return { bundle: true, autoExternal: true }; + case true: + return { bundle: true, autoExternal: false }; + case false: + return { bundle: false, autoExternal: false }; + } + }; + + const { bundle, autoExternal } = getBundleConfig(bundleType); + const shared: LibConfig = { output: { distPath: { root: `./dist/${format}`, }, }, format, syntax: 'es6', - bundle: !!bundleType, - autoExternal: bundleType === 'excludeExternal', + bundle, + autoExternal, }; return shared; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
common/config/subspaces/default/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
config/rslib-config/bin/index.js(1 hunks)config/rslib-config/eslint.config.cjs(1 hunks)config/rslib-config/package.json(1 hunks)config/rslib-config/src/index.ts(1 hunks)packages/coze-js/package.json(1 hunks)packages/coze-js/rslib.config.ts(1 hunks)packages/realtime-api/package.json(1 hunks)packages/realtime-api/rslib.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- config/rslib-config/eslint.config.cjs
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/coze-js/package.json
- packages/realtime-api/package.json
- packages/coze-js/rslib.config.ts
- packages/realtime-api/rslib.config.ts
🔇 Additional comments (5)
config/rslib-config/package.json (1)
1-33: Verify monorepo integration
The package is set up as part of a monorepo using workspace dependencies. Ensure proper integration with the monorepo tooling and verify that other packages can correctly resolve the workspace dependencies.
✅ Verification successful
Workspace dependencies are properly configured
The package @coze-infra/rslib-config is correctly integrated within the monorepo:
- Referenced as a workspace dependency in
packages/realtime-apiandpackages/coze-js - Properly registered in
rush.json
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for workspace protocol usage in dependent packages
echo "Checking workspace dependency references:"
rg '"@coze-infra/rslib-config": "workspace:\*"' -g 'package.json'
# Verify package resolution in rush.json
echo "Checking rush.json configuration:"
rg '"@coze-infra/rslib-config"' rush.json
Length of output: 487
config/rslib-config/bin/index.js (1)
1-4: LGTM! Following Node.js best practices.
The shebang is correctly set for Node.js CLI and the imports follow best practices by using the node: protocol for built-in modules.
config/rslib-config/src/index.ts (3)
1-20: LGTM! Well-structured type definitions.
The imports, type aliases, and Options interface are well-defined and provide good type safety.
48-50: DTS configuration should be format-independent
The DTS configuration is arbitrarily tied to the first format, which could lead to maintenance issues if format orders are changed.
34-46:
Add validation for empty format array
The function should validate that the format array is not empty before processing.
Consider this implementation:
+ if (!format.length) {
+ throw new Error('getRslibConfig: format array cannot be empty');
+ }
+
const libs = format.map(libFormat => {
const lib = getLibShared(libFormat, bundle);
if (libFormat === 'umd') {Likely invalid or redundant comment.
feat: rush update
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
common/config/subspaces/default/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.gitignore(1 hunks)config/rsbuild-config/eslint.config.js(1 hunks)config/rsbuild-config/package.json(1 hunks)config/rsbuild-config/tsconfig.build.json(1 hunks)config/rslib-config/eslint.config.cjs(1 hunks)config/rslib-config/package.json(1 hunks)config/rslib-config/tsconfig.build.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- config/rsbuild-config/tsconfig.build.json
- config/rslib-config/tsconfig.build.json
🚧 Files skipped from review as they are similar to previous changes (2)
- config/rslib-config/eslint.config.cjs
- config/rslib-config/package.json
🔇 Additional comments (6)
config/rsbuild-config/eslint.config.js (2)
1-2: LGTM! Clean and straightforward import.
The use of CommonJS require syntax is appropriate for ESLint configuration files, as ESLint configs traditionally use CommonJS modules.
3-7: Verify the minimal ESLint configuration coverage.
The configuration looks clean but minimal. Let's verify if this configuration is sufficient for the build system's needs:
- The
nodepreset is appropriate for a build configuration package - The
packageRootis correctly set to the configuration directory - The
ignoresarray excludes thelib-tsdirectory, which is likely the TypeScript output
Let's verify the configuration coverage and usage:
✅ Verification successful
ESLint configuration is appropriate and well-structured
The ESLint configuration for the rsbuild-config package is verified to be sufficient based on the following findings:
-
The configuration correctly uses the
nodepreset which is appropriate as:- The package contains Node.js build configuration code
- The base node configuration properly includes Node.js globals and security plugin
-
The
packageRootis correctly set to__dirnamewhich properly scopes the linting to the rsbuild-config package -
The
ignorespattern forlib-tsis appropriate because:- The package uses TypeScript (has .ts files in src/)
- The
buildscript in package.json indicates TypeScript compilation - The
lib-tsdirectory would contain generated code
-
The package has proper ESLint dependencies:
@coze-infra/eslint-configis included in devDependencies- The configuration extends the workspace's base ESLint setup
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify ESLint configuration coverage and dependencies
# Check if there are any other ESLint configs that might conflict
echo "Checking for other ESLint configs..."
fd -e js -e cjs -e json -g '*eslint*'
# Check if the node preset exists in dependencies
echo "Checking package.json for ESLint dependencies..."
fd package.json --exec jq -r '.dependencies + .devDependencies | keys[] | select(contains("eslint"))' 2>/dev/null
# Check for any TypeScript files in lib-ts to confirm if ignore pattern is correct
echo "Checking for any files in lib-ts directory..."
fd . -t f -p 'lib-ts/**'
# Look for any ESLint overrides or extends in the codebase
echo "Checking for ESLint configuration patterns..."
rg -g '!node_modules' -g '!dist' -A 5 'eslintConfig|extends.*eslint|rules:' package.json
Length of output: 2707
Script:
#!/bin/bash
# Let's verify the specific configuration in the context of the rsbuild-config package
# Check the content of the ESLint config file
echo "Checking the content of rsbuild-config ESLint config..."
cat config/rsbuild-config/eslint.config.js
# Check if there are any TypeScript files in the rsbuild-config directory
echo "Checking for TypeScript files in rsbuild-config..."
fd -e ts -e tsx . config/rsbuild-config/
# Check the package.json of rsbuild-config
echo "Checking rsbuild-config package.json..."
cat config/rsbuild-config/package.json
# Check the base ESLint config that might be extended
echo "Checking base ESLint config..."
cat config/eslint-config/eslint.config.node.js
Length of output: 1750
.gitignore (2)
162-162: Verify handling of configuration file
The ignored configuration file examples/coze-js-node/src/config/config.ts might contain important settings. Ensure that:
- No critical configuration is lost
- A template or example configuration file is provided
- Documentation exists for setting up this configuration
Also applies to: 166-167
✅ Verification successful
Let me check the content of the README to verify if it properly documents the configuration setup.
Let me verify the existence of the default configuration file mentioned in the README.
Configuration handling is properly implemented
The configuration setup follows best practices:
- A default configuration template (
config.default.ts) is provided with masked sensitive values - Clear documentation in README.md explains the setup process
- The actual config file (
config.ts) is correctly gitignored to prevent committing sensitive credentials - The setup process is straightforward: copy
config.default.tstoconfig.tsand update with real values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if a template or example config exists
fd "config.example.ts|config.template.ts" "examples/coze-js-node/src/config"
# Check if setup is documented in README
fd "README.md" "examples/coze-js-node" --exec grep -l "config.ts"
Length of output: 170
Script:
#!/bin/bash
# Check README content for configuration setup instructions
cat examples/coze-js-node/README.md
# Also check if there's a default/sample config we can examine
rg -l "export (type|interface|const).*[Cc]onfig" examples/coze-js-node/src/config/
Length of output: 870
Script:
#!/bin/bash
# Check if config.default.ts exists and examine its content
fd "config.default.ts" examples/coze-js-node/src/config/ --exec cat {}
Length of output: 2183
Line range hint 169-169: Reconsider tracking dist directory
The exception !examples/coze-js-web/src/lib/wavtools/dist goes against the common practice of ignoring build outputs. This could lead to:
- Bloated repository size
- Merge conflicts
- Inconsistencies between source and distribution files
Consider removing this exception and properly managing the wavtools distribution through package management.
config/rsbuild-config/package.json (2)
8-11: Enhance package scripts for better development workflow
Add the following scripts for a more complete setup:
"scripts": {
"build": "npx tsc -b tsconfig.build.json",
- "lint": "eslint ./ --cache --quiet"
+ "lint": "eslint ./ --cache --quiet",
+ "clean": "rimraf dist",
+ "prepare": "npm run clean && npm run build",
+ "prepack": "npm run prepare"
},Also, consider adding bin forwarding for consistency with other packages, as mentioned in the previous review.
12-16: Verify the necessity of the process package
The process package might be unnecessary as it's a Node.js built-in module. If it's needed for browser compatibility, consider documenting the reason.
✅ Verification successful
Let me broaden the search to find any references to the process object and specifically look in configuration files where it's commonly used for environment variables and browser polyfills.
Let me check the specific usage in webpack configurations and the rsbuild config to understand if the process package is actually needed for browser compatibility or webpack configuration.
The process package is needed for browser compatibility in webpack builds
Based on the webpack configurations in the examples, the process package is necessary as it's used extensively for:
- Environment variable access (e.g.,
process.env.NODE_ENV,process.env.PUBLIC_URL) - Build configuration flags (e.g.,
process.env.GENERATE_SOURCEMAP,process.env.INLINE_RUNTIME_CHUNK) - Runtime checks in the browser environment
While process is indeed a Node.js built-in module, the package is required here to provide a browser-compatible polyfill for webpack builds, ensuring that process.env references work correctly in the browser environment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for process usage in the codebase
rg -l "require\(['\"]process['\"]\)|from ['\"]process['\"]\""
Length of output: 70
Script:
#!/bin/bash
# Search for process usage with broader patterns
rg -l "process\." --type js --type jsx --type ts --type tsx
# Also check for process in webpack/rsbuild configuration files
fd -e js -e ts -e json "config|webpack|rsbuild" --exec grep -l "process" {} \;
Length of output: 660
Script:
#!/bin/bash
# Let's check the actual usage in webpack configs to understand if it's needed
grep -A 5 -B 5 "process" examples/*/config/webpack.config.js
# Also check the rsbuild config to see if there's any process-related configuration
cat config/rsbuild-config/index.ts
Length of output: 30268
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
config/rsbuild-config/bin/index.sh (1)
1-2: Consider using POSIX-compliant path resolution.The current path handling only converts Windows-style backslashes. Consider using a more robust POSIX-compliant approach.
-BASE_DIR=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')") +BASE_DIR=$(cd "$(dirname "$0")" && pwd -P)config/rslib-config/bin/index.sh (2)
1-2: Add error handling flags and improve path resolution.The script initialization could be more robust with proper error handling flags and improved path resolution.
#!/usr/bin/env bash +set -euo pipefail +IFS=$'\n\t' -BASE_DIR=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')") +BASE_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)These changes:
- Add error handling flags to fail fast and catch undefined variables
- Use a more reliable method to determine the script's directory that handles symlinks correctly
1-13: Consider alternatives to shell script wrapper.While the shell script works, consider these alternatives for better maintainability and cross-platform support:
- Use Node.js CLI directly with a bin entry in package.json
- Use npm scripts with cross-platform commands
- Consider using a tool like
cross-envfor environment variablesExample package.json approach:
{ "bin": { "rslib": "./cli.js" } }With cli.js:
#!/usr/bin/env node require('../lib/rslib-cli.js');This would:
- Eliminate the need for shell script compatibility checks
- Work consistently across platforms
- Leverage npm's built-in binary linking
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
config/rsbuild-config/bin/index.sh(1 hunks)config/rsbuild-config/package.json(1 hunks)config/rsbuild-config/src/index.ts(1 hunks)config/rsbuild-config/tsconfig.build.json(1 hunks)config/rslib-config/bin/index.sh(1 hunks)config/rslib-config/package.json(1 hunks)config/rslib-config/tsconfig.build.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- config/rslib-config/tsconfig.build.json
- config/rsbuild-config/tsconfig.build.json
- config/rsbuild-config/src/index.ts
- config/rslib-config/package.json
- config/rsbuild-config/package.json
🔇 Additional comments (1)
config/rsbuild-config/bin/index.sh (1)
11-13: 🛠️ Refactor suggestion
Add validation checks and improve argument handling.
The script should validate paths and handle arguments more safely.
-ROOT_DIR=$(realpath "$BASE_DIR/../")
+ROOT_DIR=$($RESOLVE_PATH "$BASE_DIR/../")
+
+if [ ! -d "$ROOT_DIR" ]; then
+ echo "Error: Root directory not found: $ROOT_DIR"
+ exit 1
+fi
+
+RSBUILD_BIN="$ROOT_DIR/node_modules/.bin/rsbuild"
+if [ ! -f "$RSBUILD_BIN" ]; then
+ echo "Error: rsbuild not found. Please ensure @coze-infra/rsbuild-config is installed"
+ exit 1
+fi
-bash "$ROOT_DIR/node_modules/.bin/rsbuild" "$@"
+exec "$RSBUILD_BIN" "$@"The changes:
- Use the detected path resolution command
- Validate root directory existence
- Check for rsbuild binary
- Use
execto replace the shell process instead of spawning a new one
feat: use @coze-infra/rslib-config
Summary by CodeRabbit
Release Notes
New Features
@coze-infra/rslib-configand@coze-infra/rsbuild-config.rsbuildtool.Bug Fixes
basenameprop in the Router component to ensure proper routing whenPUBLIC_URLis undefined.Documentation
rslib-configandrsbuild-config.Chores
.gitignoreto refine ignored files and directories.