Skip to content

Conversation

@Ali-Zmn
Copy link
Collaborator

@Ali-Zmn Ali-Zmn commented Jul 16, 2025

PR Type

Enhancement, Bug fix


Description

  • Fixed hiding issue for nested properties in SolaceJsonSchemaForm component
  • Enhanced Hidden story in Storybook with a comprehensive schema to test all hiding scenarios
  • Added detailed play function to test various hiding cases in the Hidden story
  • Updated dependencies, including removal of react-diff-view and addition of diff package
  • Bumped @SolaceDev/maas-react-components version to 15.9.0

Changes walkthrough 📝

Relevant files
Enhancement
SolaceJsonSchemaForm.tsx
Improve hiding functionality for nested properties             

src/components/jsonschemaform/SolaceJsonSchemaForm.tsx

  • Added recursive processing for nested properties in the hideProperties
    function
  • Removed redundant call to hideProperties at the end of the function
  • +7/-3     
    Tests
    SolaceJsonSchemaForm.stories.tsx
    Enhance Hidden story with comprehensive schema and tests 

    storybook/src/stories/layout/SolaceJsonSchemaForm.stories.tsx

  • Added comprehensive schema for testing all hiding scenarios
  • Updated Hidden story to use the new comprehensive schema
  • Added detailed play function to test various hiding scenarios
  • Imported expect from Storybook Jest for assertions
  • +212/-5 
    Dependencies
    package-lock.json
    Update dependencies and package version                                   

    storybook/package-lock.json

  • Updated version of @SolaceDev/maas-react-components to 15.9.0
  • Removed react-diff-view dependency
  • Updated @types/diff to version 5.2.3
  • Added diff dependency version 8.0.2
  • +3/-3     

    @gitstream-cm
    Copy link

    gitstream-cm bot commented Jul 16, 2025

    Please mark whether you used Copilot to assist coding in this PR

    • Copilot Assisted

    @mre-leads mre-leads added enhancement New feature or request Bug fix labels Jul 16, 2025
    @mre-leads
    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Verify that the removal of 'react-diff-view' doesn't break existing functionality

    The removal of the 'react-diff-view' dependency might impact components or features that
    rely on it. Ensure that all code using this package has been refactored or removed to
    prevent runtime errors.

    storybook/package-lock.json [67-69]

    +"react-beautiful-dnd": "^13.1.0",
    +"react-codemirror2": "^7.2.1",
    +"react-split-pane": "^0.1.92",
     
    -
    Suggestion importance[1-10]: 9

    Why: Removing a dependency without ensuring all its usages are removed or replaced can lead to runtime errors, making this a critical suggestion.

    9
    Check for potential breaking changes in updated '@types/diff' package

    The '@types/diff' package has been updated from version 5.0.2 to 5.2.3. Ensure that this
    update doesn't introduce any breaking changes in your TypeScript definitions. Review any
    code that uses types from this package and update if necessary.

    storybook/package-lock.json [78]

    +"@types/diff": "^5.2.3",
     
    -
    Suggestion importance[1-10]: 7

    Why: Updating type definitions can potentially cause TypeScript compilation errors, but it's less critical than runtime issues.

    7
    Best practice
    Conduct thorough testing after dependency changes to ensure system stability

    After making these changes to dependencies, it's crucial to run a full test suite to
    ensure that all functionality still works as expected. Pay special attention to features
    that might have been affected by the removed or updated packages.

    storybook/package-lock.json [47-48]

    +"@SolaceDev/maas-react-components",
    +"version": "15.9.0",
     
    -
    Suggestion importance[1-10]: 9

    Why: Comprehensive testing after dependency changes is crucial to catch any potential issues introduced by the updates and ensure system stability.

    9
    Ensure package.json is updated to match package-lock.json changes

    Consider updating the dependencies in the package.json file as well to ensure consistency
    between package.json and package-lock.json. This will help prevent potential conflicts and
    ensure that all developers are using the same versions of dependencies.

    storybook/package-lock.json [48]

    +"version": "15.9.0",
     
    -
    Suggestion importance[1-10]: 8

    Why: Keeping package.json and package-lock.json in sync is crucial for consistent dependency management across different environments and developers.

    8
    Possible bug
    Add a type guard to ensure the property is an object before recursive processing

    Consider adding a type guard to check if properties[property] is an object before
    recursively calling hideProperties. This will prevent potential runtime errors if the
    property is unexpectedly not an object.

    src/components/jsonschemaform/SolaceJsonSchemaForm.tsx [139-144]

    -if (properties[property] && typeof properties[property] === "object") {
    +if (properties[property] && typeof properties[property] === "object" && !Array.isArray(properties[property])) {
       if (!uiSchema[property]) {
         uiSchema[property] = {};
       }
    -  hideProperties(isHidden, uiSchema[property], properties[property]);
    +  hideProperties(isHidden, uiSchema[property], properties[property] as Record<string, unknown>);
     }
     
    Suggestion importance[1-10]: 8

    Why: This suggestion improves type safety and prevents potential runtime errors by ensuring the property is a non-array object before recursive processing.

    8
    Add null checks before assigning properties to uiSchema[property]

    Consider adding a null check for uiSchema[property] before assigning properties to it.
    This will prevent potential runtime errors if uiSchema[property] is unexpectedly null or
    undefined.

    src/components/jsonschemaform/SolaceJsonSchemaForm.tsx [130-137]

     uiSchema[property] = { "ui:hideError": true };
     if (isHidden(FormFieldType.property, property, properties[property])) {
       // hide rjsf property widget
    -  uiSchema[property]["ui:widget"] = "hidden";
    +  if (uiSchema[property]) {
    +    uiSchema[property]["ui:widget"] = "hidden";
    +  }
     } else if (isHidden(FormFieldType.description, property, properties[property])) {
       // hide rjsf property description via custom handling - descriptions are displayed with custom widgets
    -  uiSchema[property]["ui:description"] = CustomProperty.hidden;
    +  if (uiSchema[property]) {
    +    uiSchema[property]["ui:description"] = CustomProperty.hidden;
    +  }
     }
     
    Suggestion importance[1-10]: 7

    Why: This suggestion adds a layer of safety by checking for null/undefined values, preventing potential runtime errors, but it's less critical as uiSchema[property] is initialized earlier.

    7
    Improve the robustness of the data.const check to prevent potential runtime errors

    In the isHidden function, the check for data?.const !== undefined might not be sufficient.
    Consider adding a more robust check to ensure that data is not null or undefined before
    accessing its properties. This can prevent potential runtime errors.

    storybook/src/stories/layout/SolaceJsonSchemaForm.stories.tsx [467-473]

     return (
    -	data?.const !== undefined ||
    +	(data && 'const' in data) ||
     	propertyName === "longStringProp" ||
     	propertyName === "hiddenNested" ||
     	propertyName === "hiddenDeep" ||
     	propertyName === "hiddenInA" ||
     	propertyName === "hiddenInB"
     );
     
    Suggestion importance[1-10]: 7

    Why: This suggestion addresses a potential bug that could lead to runtime errors, which is important for code reliability.

    7
    Enhancement
    Ensure proper integration and usage of the newly added 'diff' package

    A new dependency 'diff' has been added with version 8.0.2. Make sure to update your import
    statements and usage of this package throughout your codebase. Also, consider if this
    addition is necessary and if it replaces any existing functionality.

    storybook/package-lock.json [91]

    +"diff": "^8.0.2",
     
    -
    Suggestion importance[1-10]: 8

    Why: Adding a new package requires careful integration and potentially refactoring existing code, making this an important suggestion.

    8
    Maintainability
    Break down the large schema into smaller, more manageable parts to improve code organization and readability

    The comprehensiveSchema is quite large and complex. Consider breaking it down into
    smaller, more manageable parts using separate constants for each section (e.g., basic
    fields, nested objects, conditional sections). This will improve readability and
    maintainability of the code.

    storybook/src/stories/layout/SolaceJsonSchemaForm.stories.tsx [116-230]

    +const basicFields = {
    +	enumProp: {
    +		default: "exclusive",
    +		enum: ["exclusive", "non-exclusive"],
    +		description: "enumProp description",
    +		type: "string"
    +	},
    +	// ... (other basic fields)
    +};
    +
    +const nestedObjects = {
    +	// ... (nested object properties)
    +};
    +
    +const conditionalSection = {
    +	// ... (conditional section properties)
    +};
    +
     const comprehensiveSchema = {
     	type: "object",
     	title: "Comprehensive Form Test",
     	description: "Tests all hiding scenarios in one schema",
     	properties: {
    -		// Basic fields from demoSchema
    -		enumProp: {
    -			default: "exclusive",
    -			enum: ["exclusive", "non-exclusive"],
    -			description: "enumProp description",
    -			type: "string"
    -		},
    -		// ... (other properties)
    +		...basicFields,
    +		...nestedObjects,
    +		...conditionalSection,
     	},
     	required: ["enumProp", "stringProp"]
     };
     
    Suggestion importance[1-10]: 8

    Why: This suggestion significantly improves code organization and maintainability, which is crucial for complex schemas.

    8

    @sonarqube-solacecloud
    Copy link

    Quality Gate failed Quality Gate failed

    Failed conditions
    1 New issue

    See analysis details on SonarQube

    Catch issues before they fail your Quality Gate with our IDE extension SonarQube IDE SonarQube IDE

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    Bug fix enhancement New feature or request

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants