Skip to content
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

Fix the dependabot related issue. #1094

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Tatsinnit
Copy link
Member

@Tatsinnit Tatsinnit commented Nov 26, 2024

Hiya, This PR fixes #1088

Sample VISX:

vscode-aks-tools-1.5.2-dependabot-fix.vsix.zip

Details

I think the error TS2322: Type '{ children: unknown[]; }' is not assignable to type '{ children?: ReactNode; }' occurs because the children prop in JSX expects a type assignable to ReactNode, and in the code, the children prop contains unknown[], which is incompatible.

This issue typically arises from returning <>{value}</> where value has the type unknown. To resolve the error, you need to ensure that the value is safely cast or transformed into a type compatible with ReactNode before being rendered.

Additional Details:

  • In cases where value is unknown, explicitly cast or transform it into a string using String(value). This makes the type compatible with ReactNode.

  • For ValueType.Bytes and the default case, value is wrapped in String(value) to ensure it's rendered as a string.

  • For ValueType.AddressArray, ensure that the value is correctly parsed into a string array and joined with a separator. Ensure JSON.parse operates on valid JSON to avoid runtime errors.

This approach ensures type safety and fixes the TS2322 error while maintaining the intended functionality.

fyi: please: ❤️ @hsubramanianaks , @ReinierCC , @tejhan , @qpetraroia

@Tatsinnit Tatsinnit added the dependencies Pull requests that update a dependency file label Nov 26, 2024
@Tatsinnit Tatsinnit self-assigned this Nov 26, 2024
Copy link
Member

@bfoley13 bfoley13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Copy link
Collaborator

@hsubramanianaks hsubramanianaks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, Hope testing worked for you. Thank you @Tatsinnit

@Tatsinnit
Copy link
Member Author

Looks good to me, Hope testing worked for you. Thank you @Tatsinnit

⛩️ Indeed, I was wondering if anyone of you can double test to make sure, it is not the works in Mac case :) but I think it should be safe, will ask around and get folks to do a quick shake. cc: @ReinierCC + @tejhan ❤️ if one of you have small window of time to test this please. Thank you so much in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants