-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: elem id add type #296
The head ref may contain hidden characters: "25-\u529F\u80FD\u63D0\u4F9B\u66F4\u7075\u6D3B\u7684\u7F16\u8F91\u533A\u6837\u5F0F"
Conversation
|
Warning Rate limit exceeded@cycleccc has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 6 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes introduce a new feature that adds an Element ID type to the Wangeditor Next editor and core packages, enhancing element identification functionality. Modifications include updates to test cases, import reorganization, and improvements in code readability across various files. Key functions such as Changes
Possibly related PRs
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (4)
.changeset/loud-dogs-fly.md (1)
6-6
: Enhance the commit message clarity.While the message follows conventional commit format with "feat:", it could be more descriptive about what the change accomplishes.
Consider updating to something like:
-feat: elem id add type +feat: add type parameter to element ID generationpackages/core/src/render/helper.ts (1)
6-7
: Add JSDoc documentation for the new parameter.The function's documentation should be updated to describe the new type parameter and provide examples of valid values.
/** * @description formats helper * @author wangfupeng + * @param type The type of element (e.g., 'paragraph', 'header') + * @param id Unique identifier for the element + * @returns Formatted element ID string */packages/core/src/render/element/renderElement.tsx (1)
50-53
: LGTM! Code style improvements enhance readability.Good improvements:
- Using
const
forrenderElem
improves code safety- Block syntax for conditionals enhances readability
Consider adding a type annotation for
renderElem
to improve code clarity:- const renderElem = getRenderElem(type) + const renderElem: RenderElementFunction = getRenderElem(type)Also applies to: 107-121
packages/core/src/editor/plugins/with-content.ts (1)
Line range hint
1-452
: Consider implementing a more robust error handling strategy.While the changes successfully implement element ID type support, consider enhancing the error handling strategy across the editor operations. Some suggestions:
- Implement a centralized error handling mechanism for editor operations
- Add error boundaries for critical operations like HTML parsing and content transformation
- Consider adding logging for debugging purposes
This would improve the maintainability and reliability of the editor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- .changeset/loud-dogs-fly.md (1 hunks)
- packages/core/tests/editor/dom-editor.test.ts (2 hunks)
- packages/core/src/editor/plugins/with-content.ts (15 hunks)
- packages/core/src/render/element/renderElement.tsx (5 hunks)
- packages/core/src/render/helper.ts (1 hunks)
🔇 Additional comments (8)
packages/core/src/render/helper.ts (1)
6-7
: Verify all callers of genElemId are updated.This is a breaking change as the function now requires a type parameter. We need to ensure all existing calls are updated.
✅ Verification successful
No issues found - function usage is consistent
The verification shows that:
- The
genElemId
inupdate-view.ts
is a different function with its own implementation for textarea elements- The imported
genElemId
fromhelper.ts
is correctly used with both type and id parameters in:
renderElement.tsx
:genElemId(elemNode.type, key.id)
with-content.ts
:genElemId(node.type, key.id)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for existing calls to genElemId to verify they've been updated with the type parameter rg "genElemId\(" --type tsLength of output: 474
Script:
#!/bin/bash # Get more context around the genElemId function in update-view.ts to understand if it's a different function rg "genElemId" packages/core/src/text-area/update-view.ts -B 2 -A 2 # Get more context around the usage in renderElement.tsx and with-content.ts rg "genElemId" packages/core/src/render/element/renderElement.tsx -B 2 -A 2 rg "genElemId" packages/core/src/editor/plugins/with-content.ts -B 2 -A 2Length of output: 1300
packages/core/src/render/element/renderElement.tsx (2)
6-22
: LGTM! Import organization improvements.The consolidation and reordering of imports enhance code organization and readability.
40-40
: Verify element type availability and usage consistency.The addition of
elemNode.type
togenElemId
aligns with the PR objective. However, we should verify:
- The type parameter is always available
- All other callers of
genElemId
have been updated✅ Verification successful
Based on the shell script results, I can now generate the final response:
genElemId
usage is consistent with type parameterThe verification shows that:
- The
genElemId
function inpackages/core/src/render/helper.ts
is correctly defined to accept bothtype
andid
parameters- All usages of
genElemId
in the codebase pass both parameters:
- In
renderElement.tsx
:genElemId(elemNode.type, key.id)
- In
with-content.ts
:genElemId(node.type, key.id)
The element type is consistently available through the
type
property of element nodes, as evidenced by numerous usages throughout the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all genElemId calls to ensure they've been updated with type parameter rg "genElemId\(" --type ts --type tsx # Search for potential element type definitions to verify type availability ast-grep --pattern "interface Element { type: $_ $$$ }"Length of output: 133
Script:
#!/bin/bash # Search for all genElemId calls with ripgrep, fixing the file type flags rg "genElemId\(" -t typescript -t tsx # Search for element type definitions using a more specific pattern ast-grep --pattern 'interface $_ { type: $_ }' # Also search for type in element node references ast-grep --pattern '$_.type'Length of output: 8398
Script:
#!/bin/bash # Search for genElemId function definition and usages with plain grep rg "genElemId" -l # Then use cat to see the content of files containing genElemId rg "genElemId" -l | xargs cat # Search for Element interface or type definition ast-grep --pattern 'type Element = { type: $_ $$$ }'Length of output: 18695
packages/core/__tests__/editor/dom-editor.test.ts (2)
8-8
: LGTM: Import statement aligns with type requirements.The addition of CustomElement type import supports the enhanced element identification feature.
152-159
: LGTM: Test updates properly verify the new element ID format.The changes correctly test the new element identification system by:
- Properly type-casting the element
- Verifying the updated ID format that includes the element type
Let's verify the consistency of this ID format across the codebase:
✅ Verification successful
Let me gather more information about the toDOMNode implementation since the previous pattern didn't match exactly.
Let me search specifically for the DomEditor implementation since that's what we're verifying.
Let me try one final search to locate the DomEditor implementation and element ID generation.
Based on the evidence gathered, I can now provide the final response:
✓ Element ID format is consistently implemented across the codebase
The test correctly verifies the element ID format which is defined in
packages/core/src/render/helper.ts
. ThegenElemId
helper function uses the exact same formatw-e-element-${type}-${id}
that's being tested, confirming that the test is properly aligned with the implementation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent element ID generation pattern # Expected format: w-e-element-${type}-${id} # Search for element ID generation patterns echo "Checking ID generation patterns:" rg -n "w-e-element-" --type ts # Search for toDOMNode implementations echo -e "\nChecking toDOMNode implementations:" ast-grep --pattern 'toDOMNode($$$) { $$$}'Length of output: 428
Script:
#!/bin/bash # Search for toDOMNode with class/object context ast-grep --pattern 'class $_ { $$$ toDOMNode($$$) { $$$ } $$$ }' # Also check for any DOM node ID related code rg -n "createElement|getElementById" --type ts -A 3Length of output: 15845
Script:
#!/bin/bash # Search for DomEditor class implementation ast-grep --pattern 'class DomEditor { $$$ }' # Also search for element ID generation related code rg -n "getElementId|generateId" --type ts -A 3Length of output: 97
Script:
#!/bin/bash # Search for files containing DomEditor rg -l "DomEditor" --type ts # Search for element ID generation in render/helper.ts which we found earlier cat packages/core/src/render/helper.tsLength of output: 11770
packages/core/src/editor/plugins/with-content.ts (3)
6-21
: LGTM! Import statements are well organized.The imports are logically grouped and the addition of
genElemId
aligns with the PR's objective of adding element ID type support.
35-35
: LGTM! Improved code clarity with explicit braces.The addition of braces to the if statement follows best practices and prevents potential bugs from implicit statement blocks.
430-430
: LGTM! Improved code formatting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (6)
packages/core/src/parse-html/parse-elem-html.ts (1)
Line range hint
65-65
: Consider replacing nodeType comparison with isDOMText.For consistency with the earlier changes, consider replacing the direct
nodeType !== 3
comparison with!isDOMText()
.- if ($elem[0].childNodes.length > 0 && $elem[0].childNodes[0].nodeType !== 3) { + if ($elem[0].childNodes.length > 0 && !isDOMText($elem[0].childNodes[0])) {packages/core/src/editor/plugins/with-max-length.ts (1)
Line range hint
74-136
: Consider enhancing error handling and user feedback.While the maxLength functionality works, it silently fails when limits are exceeded. Consider adding error events or callbacks to notify users when content is truncated or rejected.
Example implementation:
interface MaxLengthOptions { onLimitExceeded?: (remaining: number) => void; } // Add to config and emit events when content is truncated if (leftLength < text.length) { editor.config.onLimitExceeded?.(leftLength); // ... existing logic }packages/core/src/parse-html/parse-common-elem-html.ts (1)
Line range hint
164-174
: Consider extracting nested conditions into a separate function.The nested conditions for handling void elements and children could be more readable if extracted into a separate function.
Consider refactoring like this:
- if (!isVoid) { - // 非 void ,如果没有 children ,则取纯文本 - if (children.length === 0) { - elem.children = [{ text: $elem.text().replace(/\s+/gm, ' ') }] - } - - // 处理 style - PARSE_STYLE_HTML_FN_LIST.forEach(fn => { - elem = fn($elem[0], elem, editor) as Element - }) - } + function processNonVoidElement(elem: Element, $elem: Dom7Array, children: Descendant[], editor: IDomEditor): Element { + if (children.length === 0) { + elem.children = [{ text: $elem.text().replace(/\s+/gm, ' ') }] + } + + return PARSE_STYLE_HTML_FN_LIST.reduce((acc, fn) => + fn($elem[0], acc, editor) as Element, elem) + } + + if (!isVoid) { + elem = processNonVoidElement(elem, $elem, children, editor) + }packages/core/src/utils/dom.ts (3)
87-96
: Well-structured enum for DOM node types!Good practice replacing magic numbers with named constants. This improves code readability and maintainability.
Consider documenting the enum with JSDoc comments to describe each node type's purpose, especially for less common ones like
CDATA_SECTION_NODE
andPROCESSING_INSTRUCTION_NODE
. This would help other developers understand when to use each type.
345-345
: Consider enhancing void element detection.While the update to use NodeType.ELEMENT_NODE is good, consider these improvements:
- Extract the additional void elements (iframe, video) into a constant array alongside htmlVoidElements
- Consider making the iteration limit (10000) configurable or documenting why this specific number was chosen
+ const ADDITIONAL_VOID_ELEMENTS = ['iframe', 'video'] + const MAX_DEPTH_ITERATIONS = 10000 // Prevent infinite loops in malformed DOM export function getFirstVoidChild(elem: DOMElement): DOMElement | null { // ... num += 1 - if (num > 10000) { break } + if (num > MAX_DEPTH_ITERATIONS) { break } // ... if ( - htmlVoidElements.includes(name) - // 补充一些 - || name === 'iframe' - || name === 'video' + htmlVoidElements.includes(name) || ADDITIONAL_VOID_ELEMENTS.includes(name) ) {
391-394
: Improve readability of the loop condition.While the update to use NodeType enum values is good, the loop condition contains an assignment which could be confusing.
Consider restructuring the loop to make the assignment more explicit:
- for (let nodes = elem.childNodes, i = nodes.length; i -= 1;) { + const nodes = elem.childNodes + for (let i = nodes.length - 1; i >= 0; i--) {🧰 Tools
🪛 Biome
[error] 394-394: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- packages/core/src/editor/plugins/with-content.ts (15 hunks)
- packages/core/src/editor/plugins/with-max-length.ts (4 hunks)
- packages/core/src/parse-html/parse-common-elem-html.ts (9 hunks)
- packages/core/src/parse-html/parse-elem-html.ts (2 hunks)
- packages/core/src/utils/dom.ts (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/editor/plugins/with-content.ts
🧰 Additional context used
🪛 Biome
packages/core/src/utils/dom.ts
[error] 394-394: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (9)
packages/core/src/parse-html/parse-elem-html.ts (1)
11-11
: LGTM! Good refactoring of imports.The change from
NodeType
toisDOMText
import reflects a move towards more semantic utility functions, which improves code maintainability.packages/core/src/editor/plugins/with-max-length.ts (2)
6-12
: LGTM! Import organization and documentation improvements.The added comment about IME composition handling is valuable documentation, and the import reorganization improves code readability.
Line range hint
74-98
: Consider enhancing fragment insertion with partial content support.The current implementation handles the first node specially but rejects subsequent nodes entirely if they exceed the limit. Consider implementing a more graceful degradation that preserves as much content as possible within the maxLength constraint.
Let's verify if this pattern exists in other editor implementations:
packages/core/src/parse-html/parse-common-elem-html.ts (4)
7-14
: LGTM! Well-organized imports.The imports are now logically grouped, improving code readability and dependency management.
71-71
: LGTM! Improved type checking with utility functions.Replacing direct
nodeType
checks withisDOMElement
andisDOMText
utility functions enhances type safety and code readability.Also applies to: 95-95
127-127
: LGTM! Clear indication of unused parameters.Using underscore prefix for unused parameters follows TypeScript conventions and makes the code intent clearer.
160-160
: LGTM! Consistent use of curly braces.Adding curly braces to the if statement improves code consistency and reduces potential errors in future modifications.
packages/core/src/utils/dom.ts (2)
139-139
: Consistent use of NodeType enum across node type checks!The changes improve code readability by replacing magic numbers with meaningful enum values.
Also applies to: 146-146, 160-160
Line range hint
408-408
: Clean usage of NodeType enum!The function properly handles edge cases and consistently uses the NodeType enum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
scripts/build-all.sh (2)
3-4
: Translate comments to English for better maintainabilityConsider translating the Chinese comment to English to maintain consistency and improve accessibility for all contributors.
-## 一键打包所有 package +## Build all packages in one command
1-47
: Add script documentation and package dependency documentationThe script would benefit from better documentation about the build order requirements and package dependencies.
Add this documentation at the start of the file:
#!/bin/bash +# Build All Packages Script +# ------------------------ +# This script builds all packages in the correct dependency order: +# +# Build Order and Dependencies: +# 1. core (required by all) +# 2. basic-modules +# 3. code-highlight (depends on basic-modules/code-block) +# 4. list-module +# 5. table-module +# 6. upload-image-module (depends on basic-modules/image) +# 7. video-module +# 8. editor (depends on core and all modules) +# +# Usage: ./build-all.sh [dev|build] +# - dev: Development build +# - build: Production build (default)🧰 Tools
🪛 Shellcheck
[warning] 11-11: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 14-14: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 18-18: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 23-23: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 27-27: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 31-31: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 36-36: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 40-40: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 45-45: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
packages/core/src/utils/dom.ts (1)
112-114
: Add JSDoc documentation for the new function.The
isUnprocessedListElement
function is well-implemented, but would benefit from JSDoc documentation explaining its purpose, parameters, and return value.Add documentation like this:
+/** + * Checks if an element is an unprocessed list element (ul/ol without data-w-e-type attribute) + * @param el - The element to check + * @returns boolean - True if the element is an unprocessed list element + */ export const isUnprocessedListElement = (el: Element): boolean => { return 'matches' in el && el.matches('ul:not([data-w-e-type]),ol:not([data-w-e-type])') }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
- package.json (1 hunks)
- packages/core/src/editor/dom-editor.ts (2 hunks)
- packages/core/src/editor/plugins/with-content.ts (15 hunks)
- packages/core/src/render/helper.ts (1 hunks)
- packages/core/src/utils/dom.ts (6 hunks)
- packages/custom-types.d.ts (1 hunks)
- scripts/build-all.sh (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/editor/plugins/with-content.ts
🧰 Additional context used
🪛 Biome
packages/core/src/utils/dom.ts
[error] 397-397: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
🪛 Shellcheck
scripts/build-all.sh
[warning] 11-11: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 14-14: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 18-18: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 23-23: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 27-27: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 31-31: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 36-36: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 40-40: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 45-45: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (9)
packages/core/src/render/helper.ts (3)
6-6
: LGTM! Good use of shared types.The import of ElementType from a shared package promotes consistency across the codebase.
8-9
: LGTM! Well-structured type-safe implementation.The function changes look good:
- Uses ElementType for type safety (addressing the previous review comment)
- Maintains a clear and consistent ID format
- Keeps the function simple and focused
8-9
: Verify all callers have been updated.Since this is a core utility function with a modified signature, let's verify all callers have been updated correctly.
packages/custom-types.d.ts (1)
68-69
: LGTM! Good addition for type safety.The
ElementType
type alias provides proper type safety for element type strings, which is particularly useful for the updatedgenElemId
function.Let's verify the usage of this type in the codebase:
✅ Verification successful
ElementType is correctly used and integrated
The verification shows that:
ElementType
is properly imported and used ingenElemId
function in core package- The table module has its own scoped
ElementType
definition which is independent- The implementation aligns with the type safety goals
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the ElementType usage in genElemId function and other potential consumers # Test 1: Check if genElemId is using the new ElementType ast-grep --pattern 'function genElemId($_: $_, type: ElementType)' # Test 2: Look for other potential consumers of ElementType rg -l "ElementType" --type tsLength of output: 203
Script:
#!/bin/bash # Let's check the actual implementation and usage of ElementType in the found files # Check the implementation in helper.ts rg -A 5 "ElementType" packages/core/src/render/helper.ts # Check the implementation in options.ts rg -A 5 "ElementType" packages/table-module/src/utils/options.ts # Look for genElemId function implementation ast-grep --pattern 'function genElemId($$$)'Length of output: 738
package.json (1)
13-15
: Clarify the build strategy with parallel scripts.The repository now has two development scripts:
"dev": "turbo run dev"
"dev:sh": "sh scripts/build-all.sh dev"
Please document the differences and when to use each one.
#!/bin/bash # Check if turbo.json exists and examine its configuration if [ -f "turbo.json" ]; then echo "Turbo configuration:" cat turbo.json fipackages/core/src/utils/dom.ts (2)
87-96
: Well-structured enum for DOM node types!Good addition of the
NodeType
enum to replace magic numbers with named constants. The values correctly match the W3C DOM specification, improving code readability and maintainability.
142-142
: Good replacement of magic numbers with NodeType enum!The consistent use of the NodeType enum across node type checking functions improves code maintainability.
Also applies to: 149-149, 163-163, 348-348
packages/core/src/editor/dom-editor.ts (2)
25-25
: LGTM! Import addition enhances code consistency.The addition of
isDOMText
import aligns with the codebase's pattern of using dedicated DOM utility functions.
Line range hint
737-741
: LGTM! Improved text node check using utility function.The replacement of direct
nodeType
check withisDOMText
utility:
- Enhances code readability
- Maintains consistency with other DOM checks
- Makes the code more maintainable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/core/src/utils/dom.ts (6 hunks)
🔇 Additional comments (3)
packages/core/src/utils/dom.ts (3)
87-96
: Well-structured enum for DOM node types!Good addition of the
NodeType
enum to replace magic numbers. This improves code readability and maintainability by providing meaningful constants for DOM node types.
142-142
: LGTM! Good use of NodeType enum.The updates to use
NodeType
enum constants in type checking functions improve code clarity while maintaining the same functionality.Also applies to: 149-149, 163-163
394-397
: LGTM! Good implementation of the suggested changes.The node type checking has been simplified and improved by:
- Using
isDOMText
helper for text node check- Using array includes with
NodeType
enum for other node types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
scripts/build-base.sh (2)
15-21
: Improve variable initialization order and remove extra whitespace.
The build start time should be captured before the directory change to ensure it's always recorded, even if the script fails early.
Apply these changes:
+# Initialize build status tracking
+declare -A build_status
+build_start_time=$(date +%s)
+
cd ./packages || { echo "Error: packages directory not found"; exit 1; }
-
-
-# Initialize build status tracking
-declare -A build_status
-build_start_time=$(date +%s)
63-74
: Improve package build configuration and flexibility.
The current implementation has several limitations:
- Package dependencies are implied by build order but not verified
- No ability to build specific packages
- Package list is hard-coded
Consider these improvements:
- Add a configuration file for package definitions:
{
"packages": {
"core": { "dependencies": [] },
"basic-modules": { "dependencies": ["core"] },
"editor": { "dependencies": ["core", "basic-modules"] }
}
}
- Modify the script to read from this configuration:
+# Read package configuration
+config_file="package-build-config.json"
+if [ -f "$config_file" ]; then
+ packages=$(jq -r '.packages | keys[]' "$config_file")
+else
+ echo "Error: Missing $config_file"
+ exit 1
+fi
+
+# Allow building specific packages
+if [ -n "$2" ]; then
+ requested_package=$2
+ if echo "$packages" | grep -q "^$requested_package$"; then
+ build_package "$requested_package"
+ exit $?
+ else
+ echo "Error: Package $requested_package not found"
+ exit 1
+ fi
+fi
+
-# Build packages in dependency order
-build_package "core"
-build_package "basic-modules"
-build_package "code-highlight"
-build_package "list-module"
-build_package "table-module"
-build_package "upload-image-module"
-build_package "video-module"
-build_package "editor"
+# Build all packages in dependency order
+for package in $packages; do
+ build_package "$package"
+done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- package.json (1 hunks)
- packages/core/src/utils/dom.ts (6 hunks)
- scripts/build-base.sh (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- packages/core/src/utils/dom.ts
b1f8219
to
425aac8
Compare
Changes Overview
Implementation Approach
Testing Done
Verification Steps
Additional Notes
Checklist
Related Issues
Summary by CodeRabbit
Release Notes
New Features
isDOMText
to improve text node handling within the editor.ElementType
for better type safety.Improvements
Development Tools
"dev:sh"
for streamlined development builds.build-base.sh
for automating the packaging of multiple software packages.