Create a home page#4
Conversation
…m the spotify api
…ich is consumed in the CardSong component to perform the search endpoint of the spotify api
…tionality code was also separated from the cardSong views
… states both in the painting of the song lists and in the search engine
|
This pull request was cloned from Note: the URL is not a link to avoid triggering a notification on the original pull request. |
brendanator
left a comment
brendanator
left a comment
There was a problem hiding this comment.
Hey @brendanator - I've reviewed your changes and they look great!
General suggestions:
- Consider renaming the file with a typo in its name for clarity and consistency.
- Review the handling of errors and undefined returns in asynchronous functions to ensure robust error handling.
- Evaluate the use of inline styles for scalability and maintainability in larger applications.
- Move static style objects outside of components to improve performance and avoid unnecessary re-renders.
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| const urlencoded = new URLSearchParams(); | ||
| urlencoded.append("grant_type", "client_credentials"); | ||
| urlencoded.append("client_id", process.env.REACT_APP_SPOTIFY_CLIENT_ID || '' ); | ||
| urlencoded.append("client_secret", process.env.REACT_APPP_SPOTIFY_CLIENT_SECRET || ''); |
There was a problem hiding this comment.
REACT_APPP_SPOTIFY_CLIENT_SECRET. It likely should be REACT_APP_SPOTIFY_CLIENT_SECRET (with two 'P's instead of three). This typo could prevent the application from correctly accessing the client secret from the environment variables.
| const responseData = await data.json(); | ||
| return responseData; | ||
| } catch (error) { | ||
| return; |
There was a problem hiding this comment.
undefined in case of an error during token generation might lead to unhandled exceptions later in the code where the token is used. It could be beneficial to throw a specific error or return a more descriptive error object to facilitate error handling and debugging.
| @@ -0,0 +1,38 @@ | |||
| import { getToken } from "./generateToken"; | |||
There was a problem hiding this comment.
requetsToEndpoint.ts seems to be a typo. Consider renaming it to requestsToEndpoint.ts for clarity and consistency.
| @@ -0,0 +1,56 @@ | |||
| import React, { useState, useEffect } from "react"; | |||
There was a problem hiding this comment.
style={{display:'inline-block', marginTop: '20px'}}) is acceptable for small projects or prototypes, for larger applications or components that might be reused, consider using a CSS module or styled-components for better maintainability and scalability.
| @@ -0,0 +1,42 @@ | |||
| import React, { useState } from 'react'; | |||
There was a problem hiding this comment.
style object defined here for InputBase could be moved outside of the component or into a separate CSS module for better separation of concerns and to avoid the style object being recreated on each render.
1 participant
No description provided.