bluewave-labs / bluewave-onboarding

https://bluewavelabs.ca
GNU Affero General Public License v3.0
30 stars 24 forks source link

Toast.id type warning #336

Closed akshikrm closed 2 weeks ago

akshikrm commented 4 weeks ago

Fixed the invalid prop issue. Issue was because of in ToastItem.PropTypes id was of the type number but the generated toast.id is a string.

closes #315

coderabbitai[bot] commented 4 weeks ago

Walkthrough

The changes in the ToastItem component involve an update to the PropTypes declaration, specifically altering the id property within the toast object from a required number to a required string. This adjustment indicates that the id is now expected to be a string type. The component's overall functionality, including state management and rendering, remains intact, continuing to manage visibility and handle toast removal.

Changes

File Change Summary
frontend/src/components/Toast/ToastItem.jsx Updated id PropType from number.isRequired to string.isRequired
frontend/src/tests/components/Toast/ToastItem.test.jsx Modified mockToast to use dynamic id generation and updated properties for testing

Assessment against linked issues

Objective Addressed Explanation
Change toast.id PropType to avoid warnings (#315)

📜 Recent review details **Configuration used: .coderabbit.yaml** **Review profile: CHILL**
📥 Commits Reviewing files that changed from the base of the PR and between ff6032a715a5aa1f7b4f1fc6a84c4a7d458165fb and 93dce8aaed22d5ede62d308df856d46f2df36a6d.
📒 Files selected for processing (1) * `frontend/src/tests/components/Toast/ToastItem.test.jsx` (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1) * frontend/src/tests/components/Toast/ToastItem.test.jsx

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?

❤️ Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
🪧 Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): > ‼️ **IMPORTANT** > Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged. - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` 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) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.
erenfn commented 3 weeks ago

This change will get rid of the warning but not fix the issue. We need to find out where this originates. We use the same lines of code to generate a toast notification, so that behavior is unexpected

akshikrm commented 3 weeks ago

I found out that in the Toast Component line number:13, the id is generated as a string and not a number as mentioned in the prop-types, this is where the issue is coming from.


useEffect(() => {
....

>         id: `${Date.now()}-${Math.random()}`,

....
  }, []);

and if you take a look at ToastItem.test.jsx line:7 the id is a number, hence the test is passing without throwing any error.


describe('ToastItem', () => {
....

>   const mockToast = { id: 1, message: 'Test Toast', duration: 1000 };

...

With the above findings the generated in the Toast component can be converted into a number type and it should fix the source of the issue.

akshikrm commented 3 weeks ago

I have made the changes changes, could you take a look at them

erenfn commented 3 weeks ago

Now I get this in tests:


stderr | src/tests/components/Toast/ToastItem.test.jsx > ToastItem > renders toast message correctly
Warning: Failed prop type: Invalid prop `toast.id` of type `number` supplied to `ToastItem`, expected `string`.
    at ToastItem (C:\Users\Eren\Documents\bluewave-onboarding\frontend\src\components\Toast\ToastItem.jsx:13:22)

Where do we supply type number?

erenfn commented 2 weeks ago

Any updates?