refactor: replace framer-motion with motion/react and update imports #709
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @anyulled, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request focuses on refactoring the codebase by replacing the framer-motion library with @motion/react. This involves updating imports across multiple components and style files to use the new library. Additionally, the pull request updates the package.json and package-lock.json files to reflect the change in dependencies. The useFetchSpeakers.ts file was also updated to include error handling with Sentry and to add the Accept header to the axios get requests.
Highlights
- Library Replacement: Replaces
framer-motionwith@motion/reactfor animation purposes. - Import Updates: Updates imports in multiple components and style files to reflect the new library.
- Dependency Management: Updates
package.jsonandpackage-lock.jsonto include@motion/reactand removeframer-motion. - Error Handling: Implements error handling in
useFetchSpeakers.tsusing Sentry for reporting exceptions. - Axios Configuration: Adds the
Acceptheader to the axios get requests inuseFetchSpeakers.ts.
Changelog
Click here to see the changelog
- package-lock.json
- Adds
motionas a dependency. - Updates the resolved URL and integrity hash for
motion.
- Adds
- package.json
- Adds
motionas a dependency.
- Adds
- src/2023/Home/components/Faqs/Faqs.style.ts
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
ColorandBIG_BREAKPOINT.
- Replaces import from
- src/2023/Home/components/Home/Style.Home.tsx
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
ColorandBIG_BREAKPOINT.
- Replaces import from
- src/2023/Home/components/SpeakersCarousel/SpeakerSwiper.tsx
- Updates import paths for
Color,ROUTE_SPEAKER_DETAIL,useFetchSpeakers, anduseSentryErrorReport.
- Updates import paths for
- src/2023/SpeakerDetail/Speaker.style.ts
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
Colorand breakpoint constants.
- Replaces import from
- src/2023/SpeakerDetail/SpeakerDetail.style.ts
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
BIG_BREAKPOINTandColor.
- Replaces import from
- src/2023/Speakers/Speakers.style.ts
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
BIG_BREAKPOINT,TABLET_BREAKPOINT, andColor.
- Replaces import from
- src/2023/Sponsors/Sponsors.style.ts
- Replaces import from
framer-motiontomotion/react.
- Replaces import from
- src/2023/TalkDetail/Style.MeetingDetail.tsx
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
BIG_BREAKPOINT,LARGE_BREAKPOINT, andColor.
- Replaces import from
- src/2024/Home/Style.Home.tsx
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
Colorand breakpoint constants.
- Replaces import from
- src/2024/Speakers/Speakers.style.ts
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
BIG_BREAKPOINT,TABLET_BREAKPOINT, andColor.
- Replaces import from
- src/2024/Sponsors/Sponsors.style.ts
- Replaces import from
framer-motiontomotion/react.
- Replaces import from
- src/components/JobOffers/JobsCard.tsx
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
Color,StyledFaqCard,StyledFaqText,StyledFaqTitle, andOffer.
- Replaces import from
- src/components/LeftHashWithText/LeftHashWithText.tsx
- Replaces import from
framer-motiontomotion/react. - Updates import path for
MOBILE_BREAKPOINT.
- Replaces import from
- src/components/Navigation/Style.Navigation.tsx
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
Color,MAX_WIDTH, andMOBILE_BREAKPOINT.
- Replaces import from
- src/components/RightHashWithText/RightHashWithText.tsx
- Replaces import from
framer-motiontomotion/react. - Updates import path for
MOBILE_BREAKPOINT.
- Replaces import from
- src/components/Swiper/SpeakersCarousel.tsx
- Replaces import from
framer-motiontomotion/react. - Updates import path for
LessThanBlueWhiteIcon.
- Replaces import from
- src/components/YearSpecific/Speakers/Speakers.style.ts
- Replaces import from
framer-motiontomotion/react. - Updates import paths for breakpoint constants and
Color.
- Replaces import from
- src/hooks/useFetchSpeakers.test.tsx
- Adds import for Sentry.
- Mocks Sentry.
- Adds tests for handling network errors, CORS errors, and empty responses.
- Adds
Acceptheader to the axios get requests.
- src/hooks/useFetchSpeakers.ts
- Adds import for Sentry.
- Adds try-catch block to handle errors and report to Sentry.
- Adds the
Acceptheader to the axios get requests.
- src/hooks/useFetchTalks.test.tsx
- Updates the type of mockedAxios to MockedFunction.
- src/views/Home/HomeWrapper.tsx
- Sets the sessionizeUrl to 'default'.
- src/views/Home/components/Faqs/Faqs.style.ts
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
ColorandBIG_BREAKPOINT.
- Replaces import from
- src/views/Home/components/Home/Style.Home.tsx
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
Colorand breakpoint constants.
- Replaces import from
- src/views/Home/components/Sponsors/Sponsors.style.ts
- Replaces import from
framer-motiontomotion/react.
- Replaces import from
- src/views/MeetingDetail/Style.MeetingDetail.tsx
- Replaces import from
framer-motiontomotion/react. - Updates import paths for breakpoint constants and
Color. - Fixes style formatting issues.
- Replaces import from
- src/views/SpeakerDetail/Speaker.style.ts
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
Colorand breakpoint constants.
- Replaces import from
- src/views/SpeakerDetail/SpeakerDetail.style.ts
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
BIG_BREAKPOINTandColor.
- Replaces import from
- src/views/Speakers/Speakers.style.ts
- Replaces import from
framer-motiontomotion/react. - Updates import paths for
BIG_BREAKPOINT,TABLET_BREAKPOINT, andColor.
- Replaces import from
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
In realms of code, where motions flow,
A shift occurs, a vibrant show.
Framer's embrace, now set aside,
Motion/React, a graceful stride.
Animations bloom, a lively glow.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request replaces framer-motion with @motion/react and updates imports accordingly. The changes seem straightforward and address the intended refactoring. However, there are a few areas where improvements can be made, particularly in the useFetchSpeakers.ts file regarding error handling and consistency in test assertions.
Summary of Findings
- Error Handling in
useFetchSpeakers: TheuseFetchSpeakershook returns an empty object{}upon error, which can lead to unexpected behavior. Returning an empty array[]is recommended for type consistency. - Inconsistent Import Order: The order of imports
styledandkeyframesfromstyled-componentsis not consistent across files. While functionally correct, maintaining a consistent order improves readability. - Redundant Curly Braces in CSS: The curly braces around CSS properties in
StyledExtraInfoandStyledNameContainerinsrc/views/MeetingDetail/Style.MeetingDetail.tsxare redundant and can be removed for cleaner code.
Merge Readiness
The pull request is almost ready for merging. Addressing the error handling in useFetchSpeakers and ensuring consistent return types is crucial. The other comments are minor and can be addressed at the author's discretion. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging.
| import { Color } from "@styles/colors"; | ||
| import { motion } from "motion/react"; |
There was a problem hiding this comment.
Consider using a direct import from the motion package instead of motion/react if the component is intended to be used outside of React components. This might provide more flexibility and avoid unnecessary dependencies if the core animation logic is needed elsewhere.
| import { Color } from "@styles/colors"; | |
| import { motion } from "motion/react"; | |
| import { Color } from "@styles/colors"; | |
| import { motion } from "motion"; |
| import { motion } from "framer-motion"; | ||
| import { Color } from "../../styles/colors"; | ||
| import { BIG_BREAKPOINT, BIGGER_BREAKPOINT } from "../../constants/BreakPoints"; | ||
| import { motion } from "motion/react"; |
There was a problem hiding this comment.
Consider using a direct import from the motion package instead of motion/react if the component is intended to be used outside of React components. This might provide more flexibility and avoid unnecessary dependencies if the core animation logic is needed elsewhere.
| import { motion } from "motion/react"; | |
| import { motion } from "motion"; |
| import LessThanBlueWhiteIcon from "../../assets/images/LessThanBlueIcon.svg"; | ||
| import { motion } from "framer-motion"; | ||
| import LessThanBlueWhiteIcon from "@assets/images/LessThanBlueIcon.svg"; | ||
| import { motion } from "motion/react"; |
There was a problem hiding this comment.
Consider using a direct import from the motion package instead of motion/react if the component is intended to be used outside of React components. This might provide more flexibility and avoid unnecessary dependencies if the core animation logic is needed elsewhere.
import { motion } from "motion";
src/hooks/useFetchSpeakers.ts
Outdated
| return speakerAdapter(returnData); | ||
| } catch (e) { | ||
| Sentry.captureException(e); | ||
| return {}; |
There was a problem hiding this comment.
Returning an empty object {} might not be the best approach for handling errors. It could lead to unexpected behavior in components that consume this data. Consider returning an empty array [] to maintain type consistency and avoid potential issues when mapping over the data or accessing its properties.
Sentry.captureException(e);
return [];| const mockedAxios = axios as jest.Mocked<typeof axios>; | ||
| const mockedSentry = vi.mocked(Sentry); |
There was a problem hiding this comment.
It's good to mock external dependencies like axios and Sentry. However, ensure that the mocked Sentry instance is used in the tests to verify that errors are indeed captured when expected. Consider adding assertions to check if mockedSentry.captureException is called with the correct error in the error handling tests.
| const mockedAxios = axios as jest.Mocked<typeof axios>; | |
| const mockedSentry = vi.mocked(Sentry); | |
| const mockedAxios = axios as jest.Mocked<typeof axios>; | |
| const mockedSentry = { captureException: vi.fn() }; | |
| vi.mock('@sentry/react', () => ({ captureException: mockedSentry.captureException })); |
src/hooks/useFetchSpeakers.test.tsx
Outdated
| expect(mockedSentry.captureException).toHaveBeenCalledWith(networkError); | ||
| expect(result.current.data).toEqual({}); |
There was a problem hiding this comment.
Returning an empty object {} from the hook when an error occurs can lead to issues in components consuming the data. Consider returning an empty array [] instead to maintain type consistency. Update the assertion to reflect this change.
| expect(mockedSentry.captureException).toHaveBeenCalledWith(networkError); | |
| expect(result.current.data).toEqual({}); | |
| expect(mockedSentry.captureException).toHaveBeenCalledWith(networkError); | |
| expect(result.current.data).toEqual([]); |
src/hooks/useFetchSpeakers.test.tsx
Outdated
| expect(mockedSentry.captureException).toHaveBeenCalledWith(corsError); | ||
| expect(result.current.data).toEqual({}); |
There was a problem hiding this comment.
Returning an empty object {} from the hook when an error occurs can lead to issues in components consuming the data. Consider returning an empty array [] instead to maintain type consistency. Update the assertion to reflect this change.
| expect(mockedSentry.captureException).toHaveBeenCalledWith(corsError); | |
| expect(result.current.data).toEqual({}); | |
| expect(mockedSentry.captureException).toHaveBeenCalledWith(corsError); | |
| expect(result.current.data).toEqual([]); |
✅ Deploy Preview for dev-bcn ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…d remove unused styles
|
No description provided.