MOC-294 flag for recording styles #193
Conversation
📝 WalkthroughWalkthroughThe changes introduce a new type, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Reactor
participant Main
User->>Reactor: exportDOM(element, options)
Reactor->>Main: htmlElementToJson(root, options)
Main->>Main: Process HTML element
alt exportStyles is true
Main->>Main: Retrieve computed styles
Main->>Main: Map styles to unique class names
end
Main-->>Reactor: Return DomJsonExportNode[]
Reactor-->>User: Return exported JSON
Poem
🪧 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: 12
🧹 Outside diff range and nitpick comments (5)
packages/reactor/interfaces.ts (2)
40-42: LGTM! Consider adding JSDoc comments.The new
DomJsonExportOptionstype is well-defined and follows TypeScript conventions. It provides a clear way to control style inclusion in DOM JSON exports.Consider adding JSDoc comments to improve documentation:
/** * Options for DOM JSON export. */ export type DomJsonExportOptions = { /** * Whether to include computed styles in the export. */ styles?: boolean; }
40-49: Summary: New export options and modified DOM node representationThe changes introduce a new
DomJsonExportOptionstype for controlling style inclusion in exports and modifyDomJsonExportNodeto require theattributesproperty. These changes enhance the flexibility of DOM JSON exports and ensure consistency in node representation.Key points:
- The new
DomJsonExportOptionsallows for optional inclusion of styles in exports.- The
attributesproperty inDomJsonExportNodeis now required, which may impact existing code.These changes appear to be part of a larger feature implementation related to MOC-294 for recording styles. Ensure that these changes are reflected in the implementation of the export functionality and that existing code is updated to accommodate the new required
attributesproperty.packages/reactor/reactor.ts (1)
Line range hint
126-137: LGTM: Method signature updated correctly.The
exportDOMmethod has been successfully updated to include the optionaloptionsparameter, and it's correctly passed to thehtmlElementToJsonfunction. This change enhances the method's flexibility while maintaining backward compatibility.Consider updating the method's JSDoc comment to include information about the new
optionsparameter. Here's a suggested addition:* @param {HTMLElement | null} element - The element to export. If not provided, the entire body of the attached document will be exported. + * @param {DomJsonExportOptions} [options] - Optional configuration for the DOM export process. * @throws {Error} If the reactor is not attached and no element is specified. * @return {DomJsonExportNode[]} An array of `DomJsonExportNode` objects representing the exported DOM. */packages/reactor/main.ts (1)
Line range hint
43-140: Add unit tests for the newexportStylesoptionThe introduction of the
exportStylesoption adds significant functionality. To ensure reliability and prevent future regressions, please consider adding unit tests that cover scenarios withexportStylesset totrueandfalse.Would you like me to help draft unit tests for this feature or open a GitHub issue to track this task?
packages/reactor/tests/main.test.ts (1)
2-4: Organize import statementsConsider organizing import statements for better readability. Group imports logically, such as placing all test-related imports together.
Apply this diff:
import { JSDOM } from "jsdom"; import { describe, expect, it, beforeEach } from "vitest"; +import type { AppliedModificationsImpl } from "../modifications"; import type { ModificationRequest } from "../interfaces"; -import { modifyDom, modifyHtml, htmlElementToJson } from "../main"; +import { modifyDom, modifyHtml, htmlElementToJson } from "../main";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- packages/reactor/interfaces.ts (1 hunks)
- packages/reactor/main.ts (5 hunks)
- packages/reactor/reactor.ts (3 hunks)
- packages/reactor/tests/main.test.ts (2 hunks)
🔇 Additional comments not posted (3)
packages/reactor/reactor.ts (2)
4-4: LGTM: New import added correctly.The new import for
DomJsonExportOptionsis correctly added and aligns with the changes in theexportDOMmethod.
Line range hint
1-224: Summary: Changes successfully implement the new flag for recording styles.The modifications to the
Reactorclass, specifically theexportDOMmethod, successfully implement the ability to include a flag for recording styles as described in the PR objectives (MOC-294). The changes are well-structured, maintain backward compatibility, and enhance the flexibility of the DOM export process.Key points:
- The new
DomJsonExportOptionstype is correctly imported.- The
exportDOMmethod signature is updated to include an optionaloptionsparameter.- The
optionsparameter is correctly passed to thehtmlElementToJsonfunction.These changes align with the PR objectives and the AI-generated summary, providing the desired functionality for including computed styles in the JSON output when exporting the DOM.
packages/reactor/tests/main.test.ts (1)
381-403: Verify if inline styles should be retained after consolidationPlease verify whether retaining the inline
styleattributes is intentional after consolidating styles into classes. This may lead to duplication and unnecessary data in the JSON output.Run the following script to check for elements that have both a
classand astyleattribute:Also applies to: 405-437
✅ Verification successful
Inline styles are correctly consolidated without duplication.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find elements with both 'class' and 'style' attributes in the JSON output. # Test: Search for patterns in test cases. Expect: Elements should not have both attributes unless intentional. grep -E '"attributes": \{[^}]*"class": "[^"]+",[^}]*"style": "[^"]+"' -r packages/reactor/tests/Length of output: 98
| export type DomJsonExportNode = { | ||
| tag: string; | ||
| visible: boolean; | ||
| text?: string; | ||
| attributes?: Record<string, string>; | ||
| attributes: Record<string, string>; | ||
| children?: DomJsonExportNode[]; |
There was a problem hiding this comment.
💡 Codebase verification
Issues Found with DomJsonExportNode Type Changes
The recent change making the attributes property required in DomJsonExportNode introduces inconsistencies in the codebase. Specifically, the following instance is missing the attributes property:
- File:
packages/reactor/main.ts- Location:
nodeToJsonfunction - Issue: The
DomJsonExportNodeobject is created without theattributesproperty.
- Location:
Please update the affected code to include the attributes property to ensure consistency and prevent potential runtime errors.
🔗 Analysis chain
LGTM! Verify impact on existing code.
The change to make attributes a required property in DomJsonExportNode is logical and aligns with the typical structure of DOM nodes. This ensures consistency in the exported JSON representation of DOM nodes.
However, this change might affect existing code that uses this type. Please run the following script to verify the impact:
Ensure that all usage of DomJsonExportNode includes the attributes property, and update any code that might assume attributes is optional.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for usage of DomJsonExportNode type and potential issues with the attributes property
# Test 1: Find all occurrences of DomJsonExportNode
echo "Occurrences of DomJsonExportNode:"
rg --type typescript "DomJsonExportNode"
# Test 2: Check for potential issues where attributes might be undefined
echo "\nPotential issues with attributes property:"
rg --type typescript "DomJsonExportNode.*\{(?!.*attributes)" -A 5
Length of output: 581
Script:
#!/bin/bash
# Description: Check for usage of DomJsonExportNode type and potential issues with the attributes property
# Test 1: Find all occurrences of DomJsonExportNode
echo "Occurrences of DomJsonExportNode:"
rg "DomJsonExportNode"
# Test 2: Check for potential issues where attributes might be undefined without using look-around
echo "\nPotential issues with attributes property:"
rg "DomJsonExportNode\s*{\s*(?!.*attributes)" -A 5 --pcre2
Length of output: 1669
| if (exportStyles) { | ||
| const styles = window.getComputedStyle(element); | ||
| if (styles.length > 0) { | ||
| const styleString = Array.from(styles) | ||
| .map((style) => `${style}: ${styles.getPropertyValue(style)}`) | ||
| .join('; '); | ||
| let styleClass = stylesMap[styleString]; | ||
| if (!styleClass) { | ||
| styleClass = `mocksi-${styleIndex.idx}`; | ||
| stylesMap[styleString] = styleClass; | ||
| styleIndex.idx += 1; | ||
| } | ||
|
|
||
| if (obj.attributes['class']) { | ||
| obj.attributes['class'] += ' ' + styleClass; | ||
| } else { | ||
| obj.attributes['class'] = styleClass; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize style extraction for performance
Using window.getComputedStyle for every element can be performance-intensive, especially for large DOM trees. Consider limiting the extracted styles to only those necessary for your application or implementing caching mechanisms.
| if (exportStyles) { | ||
| const stylesString = Object.entries(stylesMap).map(([styleString, clazz]) => `.${clazz} { ${styleString} }`).join('\n'); | ||
| json.push({ | ||
| tag: 'style', | ||
| visible: false, | ||
| text: stylesString, | ||
| attributes: {} | ||
| }) | ||
| } |
There was a problem hiding this comment.
Avoid adding empty <style> nodes
If no styles are collected (i.e., stylesMap is empty), the code still adds an empty <style> node to the JSON. This can be unnecessary and may introduce empty nodes in your output.
Apply this diff to add a check before adding the <style> node:
if (exportStyles && Object.keys(stylesMap).length > 0) {
const stylesString = Object.entries(stylesMap)
.map(([styleString, clazz]) => `.${clazz} { ${styleString} }`)
.join('\n');
json.push({
tag: 'style',
visible: false,
text: stylesString,
attributes: {}
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (exportStyles) { | |
| const stylesString = Object.entries(stylesMap).map(([styleString, clazz]) => `.${clazz} { ${styleString} }`).join('\n'); | |
| json.push({ | |
| tag: 'style', | |
| visible: false, | |
| text: stylesString, | |
| attributes: {} | |
| }) | |
| } | |
| if (exportStyles && Object.keys(stylesMap).length > 0) { | |
| const stylesString = Object.entries(stylesMap).map(([styleString, clazz]) => `.${clazz} { ${styleString} }`).join('\n'); | |
| json.push({ | |
| tag: 'style', | |
| visible: false, | |
| text: stylesString, | |
| attributes: {} | |
| }); | |
| } |
| if (obj.attributes['class']) { | ||
| obj.attributes['class'] += ' ' + styleClass; | ||
| } else { | ||
| obj.attributes['class'] = styleClass; | ||
| } |
There was a problem hiding this comment.
Handle existing class attributes safely
When appending the new style class to the element's class attribute, ensure that you're correctly handling existing classes and avoiding potential duplicates.
Apply this diff to improve class attribute handling:
- if (obj.attributes['class']) {
- obj.attributes['class'] += ' ' + styleClass;
- } else {
- obj.attributes['class'] = styleClass;
- }
+ const existingClass = obj.attributes['class'] || '';
+ const classes = new Set(existingClass.split(/\s+/).filter(Boolean));
+ classes.add(styleClass);
+ obj.attributes['class'] = Array.from(classes).join(' ');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (obj.attributes['class']) { | |
| obj.attributes['class'] += ' ' + styleClass; | |
| } else { | |
| obj.attributes['class'] = styleClass; | |
| } | |
| const existingClass = obj.attributes['class'] || ''; | |
| const classes = new Set(existingClass.split(/\s+/).filter(Boolean)); | |
| classes.add(styleClass); | |
| obj.attributes['class'] = Array.from(classes).join(' '); |
| @@ -39,7 +40,11 @@ | |||
| } | |||
| } | |||
|
|
|||
| export const htmlElementToJson = (root: HTMLElement): DomJsonExportNode[] => { | |||
| export const htmlElementToJson = (root: HTMLElement, options?: DomJsonExportOptions): DomJsonExportNode[] => { | |||
There was a problem hiding this comment.
💡 Codebase verification
htmlElementToJson is used outside of browser contexts
Using htmlElementToJson without ensuring window is available can lead to runtime errors in non-browser environments.
Affected file:
packages/reactor/reactor.ts
🔗 Analysis chain
Ensure window is available in the execution environment
The use of window.getComputedStyle relies on the availability of the window object. Please verify that this function is always executed in a browser environment. If the code can run in non-browser environments (like Node.js), this will cause a runtime error.
You can run the following script to check for non-browser usages of htmlElementToJson:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find usages of 'htmlElementToJson' outside browser contexts.
# Search for files importing 'htmlElementToJson' that don't reference 'window'.
rg --type ts --files-with-matches 'htmlElementToJson' | xargs grep -L 'window'
Length of output: 104
| doc.body.innerHTML = '<div id="test" style="color: red; font-size: 24px;">Hello World!</div><div id="test" style="color: red; font-size: 24px;">Hello World!</div>'; | ||
| const json = htmlElementToJson(doc.body, {styles: true}); |
There was a problem hiding this comment.
Duplicate id attributes in HTML elements
Both div elements have the same id="test", which violates HTML standards as id attributes should be unique within a document. This could lead to unexpected behavior during DOM manipulation.
Apply this diff to assign unique id values:
- doc.body.innerHTML = '<div id="test" style="color: red; font-size: 24px;">Hello World!</div><div id="test" style="color: red; font-size: 24px;">Hello World!</div>';
+ doc.body.innerHTML = '<div id="test1" style="color: red; font-size: 24px;">Hello World!</div><div id="test2" style="color: red; font-size: 24px;">Hello World!</div>';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| doc.body.innerHTML = '<div id="test" style="color: red; font-size: 24px;">Hello World!</div><div id="test" style="color: red; font-size: 24px;">Hello World!</div>'; | |
| const json = htmlElementToJson(doc.body, {styles: true}); | |
| doc.body.innerHTML = '<div id="test1" style="color: red; font-size: 24px;">Hello World!</div><div id="test2" style="color: red; font-size: 24px;">Hello World!</div>'; | |
| const json = htmlElementToJson(doc.body, {styles: true}); |
| it('should convert a simple HTML element to JSON', async () => { | ||
| doc.body.innerHTML = '<div id="test" class="example">Hello World!</div>'; | ||
| const json = htmlElementToJson(doc.body); | ||
|
|
||
| expect(json).toEqual([ | ||
| { | ||
| tag: 'div', | ||
| visible: false, | ||
| attributes: { | ||
| id: 'test', | ||
| class: 'example', | ||
| }, | ||
| text: 'Hello World!', | ||
| }, | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
Unnecessary 'async' keyword in test function declaration
The test function is declared as async, but it does not use the await keyword inside. Consider removing async for clarity.
Apply this diff to remove the async keyword:
- it('should convert a simple HTML element to JSON', async () => {
+ it('should convert a simple HTML element to JSON', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should convert a simple HTML element to JSON', async () => { | |
| doc.body.innerHTML = '<div id="test" class="example">Hello World!</div>'; | |
| const json = htmlElementToJson(doc.body); | |
| expect(json).toEqual([ | |
| { | |
| tag: 'div', | |
| visible: false, | |
| attributes: { | |
| id: 'test', | |
| class: 'example', | |
| }, | |
| text: 'Hello World!', | |
| }, | |
| ]); | |
| }); | |
| it('should convert a simple HTML element to JSON', () => { | |
| doc.body.innerHTML = '<div id="test" class="example">Hello World!</div>'; | |
| const json = htmlElementToJson(doc.body); | |
| expect(json).toEqual([ | |
| { | |
| tag: 'div', | |
| visible: false, | |
| attributes: { | |
| id: 'test', | |
| class: 'example', | |
| }, | |
| text: 'Hello World!', | |
| }, | |
| ]); | |
| }); |
| it('should handle nested HTML elements', async () => { | ||
| doc.body.innerHTML = '<div id="test"><p>Hello</p><span>World!</span></div>'; | ||
| const json = htmlElementToJson(doc.body); | ||
|
|
||
| expect(json).toEqual([ | ||
| { | ||
| tag: 'div', | ||
| visible: false, | ||
| attributes: { | ||
| id: 'test', | ||
| }, | ||
| children: [ | ||
| { | ||
| attributes: {}, | ||
| tag: 'p', | ||
| visible: false, | ||
| text: 'Hello', | ||
| }, | ||
| { | ||
| attributes: {}, | ||
| tag: 'span', | ||
| visible: false, | ||
| text: 'World!', | ||
| }, | ||
| ], | ||
| }, | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
Unnecessary 'async' keyword in test function declaration
Similar to the previous test, this function does not use await. Removing async improves readability.
Apply this diff:
- it('should handle nested HTML elements', async () => {
+ it('should handle nested HTML elements', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should handle nested HTML elements', async () => { | |
| doc.body.innerHTML = '<div id="test"><p>Hello</p><span>World!</span></div>'; | |
| const json = htmlElementToJson(doc.body); | |
| expect(json).toEqual([ | |
| { | |
| tag: 'div', | |
| visible: false, | |
| attributes: { | |
| id: 'test', | |
| }, | |
| children: [ | |
| { | |
| attributes: {}, | |
| tag: 'p', | |
| visible: false, | |
| text: 'Hello', | |
| }, | |
| { | |
| attributes: {}, | |
| tag: 'span', | |
| visible: false, | |
| text: 'World!', | |
| }, | |
| ], | |
| }, | |
| ]); | |
| }); | |
| it('should handle nested HTML elements', () => { | |
| doc.body.innerHTML = '<div id="test"><p>Hello</p><span>World!</span></div>'; | |
| const json = htmlElementToJson(doc.body); | |
| expect(json).toEqual([ | |
| { | |
| tag: 'div', | |
| visible: false, | |
| attributes: { | |
| id: 'test', | |
| }, | |
| children: [ | |
| { | |
| attributes: {}, | |
| tag: 'p', | |
| visible: false, | |
| text: 'Hello', | |
| }, | |
| { | |
| attributes: {}, | |
| tag: 'span', | |
| visible: false, | |
| text: 'World!', | |
| }, | |
| ], | |
| }, | |
| ]); | |
| }); |
| it('should export styles when the option is set', async () => { | ||
| doc.body.innerHTML = '<div id="test" style="color: red; font-size: 24px;">Hello World!</div>'; | ||
| const json = htmlElementToJson(doc.body, {styles: true}); | ||
|
|
||
| expect(json).toEqual([ | ||
| { | ||
| tag: 'div', | ||
| visible: false, | ||
| attributes: { | ||
| class: 'mocksi-1', | ||
| id: 'test', | ||
| style: 'color: red; font-size: 24px;', | ||
| }, | ||
| text: 'Hello World!', | ||
| }, | ||
| { | ||
| attributes: {}, | ||
| tag: "style", | ||
| text: ".mocksi-1 { display: block; color: rgb(255, 0, 0); font-size: 24px; visibility: visible; pointer-events: auto; background-color: rgba(0, 0, 0, 0); border-block-start-color: rgb(255, 0, 0); border-block-end-color: rgb(255, 0, 0); border-inline-start-color: rgb(255, 0, 0); border-inline-end-color: rgb(255, 0, 0); border-top-color: rgb(255, 0, 0); border-right-color: rgb(255, 0, 0); border-bottom-color: rgb(255, 0, 0); border-left-color: rgb(255, 0, 0); caret-color: auto }", | ||
| visible: false | ||
| } | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
Unnecessary 'async' keyword in test function declaration
The async keyword is unnecessary here as well.
Apply this diff:
- it('should export styles when the option is set', async () => {
+ it('should export styles when the option is set', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should export styles when the option is set', async () => { | |
| doc.body.innerHTML = '<div id="test" style="color: red; font-size: 24px;">Hello World!</div>'; | |
| const json = htmlElementToJson(doc.body, {styles: true}); | |
| expect(json).toEqual([ | |
| { | |
| tag: 'div', | |
| visible: false, | |
| attributes: { | |
| class: 'mocksi-1', | |
| id: 'test', | |
| style: 'color: red; font-size: 24px;', | |
| }, | |
| text: 'Hello World!', | |
| }, | |
| { | |
| attributes: {}, | |
| tag: "style", | |
| text: ".mocksi-1 { display: block; color: rgb(255, 0, 0); font-size: 24px; visibility: visible; pointer-events: auto; background-color: rgba(0, 0, 0, 0); border-block-start-color: rgb(255, 0, 0); border-block-end-color: rgb(255, 0, 0); border-inline-start-color: rgb(255, 0, 0); border-inline-end-color: rgb(255, 0, 0); border-top-color: rgb(255, 0, 0); border-right-color: rgb(255, 0, 0); border-bottom-color: rgb(255, 0, 0); border-left-color: rgb(255, 0, 0); caret-color: auto }", | |
| visible: false | |
| } | |
| ]); | |
| }); | |
| it('should export styles when the option is set', () => { | |
| doc.body.innerHTML = '<div id="test" style="color: red; font-size: 24px;">Hello World!</div>'; | |
| const json = htmlElementToJson(doc.body, {styles: true}); | |
| expect(json).toEqual([ | |
| { | |
| tag: 'div', | |
| visible: false, | |
| attributes: { | |
| class: 'mocksi-1', | |
| id: 'test', | |
| style: 'color: red; font-size: 24px;', | |
| }, | |
| text: 'Hello World!', | |
| }, | |
| { | |
| attributes: {}, | |
| tag: "style", | |
| text: ".mocksi-1 { display: block; color: rgb(255, 0, 0); font-size: 24px; visibility: visible; pointer-events: auto; background-color: rgba(0, 0, 0, 0); border-block-start-color: rgb(255, 0, 0); border-block-end-color: rgb(255, 0, 0); border-inline-start-color: rgb(255, 0, 0); border-inline-end-color: rgb(255, 0, 0); border-top-color: rgb(255, 0, 0); border-right-color: rgb(255, 0, 0); border-bottom-color: rgb(255, 0, 0); border-left-color: rgb(255, 0, 0); caret-color: auto }", | |
| visible: false | |
| } | |
| ]); | |
| }); |
| it('should consolidate styles when they are the same', async () => { | ||
| doc.body.innerHTML = '<div id="test" style="color: red; font-size: 24px;">Hello World!</div><div id="test" style="color: red; font-size: 24px;">Hello World!</div>'; | ||
| const json = htmlElementToJson(doc.body, {styles: true}); | ||
|
|
||
| expect(json).toEqual([ | ||
| { | ||
| tag: 'div', | ||
| visible: false, | ||
| attributes: { | ||
| class: 'mocksi-1', | ||
| id: 'test', | ||
| style: 'color: red; font-size: 24px;', | ||
| }, | ||
| text: 'Hello World!', | ||
| }, | ||
| { | ||
| tag: 'div', | ||
| visible: false, | ||
| attributes: { | ||
| class: 'mocksi-1', | ||
| id: 'test', | ||
| style: 'color: red; font-size: 24px;', | ||
| }, | ||
| text: 'Hello World!', | ||
| }, | ||
| { | ||
| attributes: {}, | ||
| tag: "style", | ||
| text: ".mocksi-1 { display: block; color: rgb(255, 0, 0); font-size: 24px; visibility: visible; pointer-events: auto; background-color: rgba(0, 0, 0, 0); border-block-start-color: rgb(255, 0, 0); border-block-end-color: rgb(255, 0, 0); border-inline-start-color: rgb(255, 0, 0); border-inline-end-color: rgb(255, 0, 0); border-top-color: rgb(255, 0, 0); border-right-color: rgb(255, 0, 0); border-bottom-color: rgb(255, 0, 0); border-left-color: rgb(255, 0, 0); caret-color: auto }", | ||
| visible: false | ||
| } | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
Unnecessary 'async' keyword in test function declaration
Again, remove async since there's no await used in the function.
Apply this diff:
- it('should consolidate styles when they are the same', async () => {
+ it('should consolidate styles when they are the same', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should consolidate styles when they are the same', async () => { | |
| doc.body.innerHTML = '<div id="test" style="color: red; font-size: 24px;">Hello World!</div><div id="test" style="color: red; font-size: 24px;">Hello World!</div>'; | |
| const json = htmlElementToJson(doc.body, {styles: true}); | |
| expect(json).toEqual([ | |
| { | |
| tag: 'div', | |
| visible: false, | |
| attributes: { | |
| class: 'mocksi-1', | |
| id: 'test', | |
| style: 'color: red; font-size: 24px;', | |
| }, | |
| text: 'Hello World!', | |
| }, | |
| { | |
| tag: 'div', | |
| visible: false, | |
| attributes: { | |
| class: 'mocksi-1', | |
| id: 'test', | |
| style: 'color: red; font-size: 24px;', | |
| }, | |
| text: 'Hello World!', | |
| }, | |
| { | |
| attributes: {}, | |
| tag: "style", | |
| text: ".mocksi-1 { display: block; color: rgb(255, 0, 0); font-size: 24px; visibility: visible; pointer-events: auto; background-color: rgba(0, 0, 0, 0); border-block-start-color: rgb(255, 0, 0); border-block-end-color: rgb(255, 0, 0); border-inline-start-color: rgb(255, 0, 0); border-inline-end-color: rgb(255, 0, 0); border-top-color: rgb(255, 0, 0); border-right-color: rgb(255, 0, 0); border-bottom-color: rgb(255, 0, 0); border-left-color: rgb(255, 0, 0); caret-color: auto }", | |
| visible: false | |
| } | |
| ]); | |
| }); | |
| it('should consolidate styles when they are the same', () => { | |
| doc.body.innerHTML = '<div id="test" style="color: red; font-size: 24px;">Hello World!</div><div id="test" style="color: red; font-size: 24px;">Hello World!</div>'; | |
| const json = htmlElementToJson(doc.body, {styles: true}); | |
| expect(json).toEqual([ | |
| { | |
| tag: 'div', | |
| visible: false, | |
| attributes: { | |
| class: 'mocksi-1', | |
| id: 'test', | |
| style: 'color: red; font-size: 24px;', | |
| }, | |
| text: 'Hello World!', | |
| }, | |
| { | |
| tag: 'div', | |
| visible: false, | |
| attributes: { | |
| class: 'mocksi-1', | |
| id: 'test', | |
| style: 'color: red; font-size: 24px;', | |
| }, | |
| text: 'Hello World!', | |
| }, | |
| { | |
| attributes: {}, | |
| tag: "style", | |
| text: ".mocksi-1 { display: block; color: rgb(255, 0, 0); font-size: 24px; visibility: visible; pointer-events: auto; background-color: rgba(0, 0, 0, 0); border-block-start-color: rgb(255, 0, 0); border-block-end-color: rgb(255, 0, 0); border-inline-start-color: rgb(255, 0, 0); border-inline-end-color: rgb(255, 0, 0); border-top-color: rgb(255, 0, 0); border-right-color: rgb(255, 0, 0); border-bottom-color: rgb(255, 0, 0); border-left-color: rgb(255, 0, 0); caret-color: auto }", | |
| visible: false | |
| } | |
| ]); | |
| }); |
Summary by CodeRabbit
New Features
DomJsonExportOptionstype to customize JSON export options, including style inclusion.htmlElementToJsonfunction to support exporting computed styles in JSON output.exportDOMmethod to accept additional options for improved DOM export functionality.Bug Fixes
DomJsonExportNodeto ensure required attributes are enforced.Tests
htmlElementToJsonto ensure accurate conversion of HTML elements to JSON, including style handling.