-
Notifications
You must be signed in to change notification settings - Fork 19.9k
refactor: replace react-papaparse with native papaparse #31565
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
base: main
Are you sure you want to change the base?
refactor: replace react-papaparse with native papaparse #31565
Conversation
Summary of ChangesHello @majiayu000, 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! This pull request refactors the application's CSV handling by migrating from the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request refactors CSV handling across the application by removing the react-papaparse library. It introduces a new web/utils/csv.ts utility file that encapsulates downloadCSV and parseCSV functionalities using the underlying papaparse library. All components previously using react-papaparse hooks for downloading CSV templates, exporting data, and reading uploaded CSVs have been updated to utilize these new utility functions. Corresponding test files have been adjusted to mock the new downloadCSV and parseCSV utilities. Review comments suggest improving a test mock for parseCSV to be more dynamic, removing redundant variables in test mocks, and eliminating an unnecessary type cast in the new parseCSV utility function.
web/app/components/share/text-generation/run-batch/csv-reader/index.spec.tsx
Outdated
Show resolved
Hide resolved
| {children} | ||
| </div> | ||
| ) | ||
| let _lastCSVDownloaderProps: Record<string, unknown> | undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| mockCSVDownloader.mockClear() | ||
| lastCSVDownloaderProps = undefined | ||
| mockDownloadCSV.mockClear() | ||
| _lastCSVDownloaderProps = undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
web/utils/csv.ts
Outdated
| ): void { | ||
| const { header = false, skipEmptyLines = true, complete } = options || {} | ||
|
|
||
| Papa.parse(file as unknown as string, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file parameter is typed as File, but it's cast to unknown as string before being passed to Papa.parse. Papa.parse can directly accept a File object. The cast to string is unnecessary and could be misleading. It's better to pass the File object directly.
| Papa.parse(file as unknown as string, { | |
| Papa.parse(file, { |
|
Can you help resolve the conflict? |
Remove the outdated react-papaparse package and replace with native papaparse library. This provides better React 19 compatibility and reduces bundle size. Changes: - Add utils/csv.ts with downloadCSV and parseCSV utilities - Update all CSV download/reader components to use new utilities - Update tests to mock the new utilities - Remove react-papaparse dependency, add papaparse Fixes langgenius#31522 Signed-off-by: majiayu000 <[email protected]>
…index.spec.tsx Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Remove redundant _lastCSVDownloaderProps variable in test - Use cleaner type assertion (as any) for Papa.parse - Simplify mock function implementation - Add eslint-disable for necessary type cast Signed-off-by: majiayu000 <[email protected]>
117d956 to
882b89f
Compare
Signed-off-by: majiayu000 <[email protected]>
Signed-off-by: majiayu000 <[email protected]>
|
这ci真难解决卧槽。。。 |
|
It's okay to skip the test for now. It should be fixed in #31729. I will look at it after then. |
The Promise.resolve().then() approach caused timing issues with React Testing Library's waitFor, as microtasks execute at different points in the event loop compared to macrotasks (setTimeout). Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary
Fixes #31522
Remove the outdated
react-papaparsepackage and replace with nativepapaparselibrary. This provides better React 19 compatibility and reduces bundle size.Changes
utils/csv.tswithdownloadCSVandparseCSVutilitiesreact-papaparsedependency, addpapaparseand@types/papaparseFiles Changed
web/utils/csv.ts- New CSV utility functionsweb/app/components/share/text-generation/run-batch/csv-download/index.tsxweb/app/components/share/text-generation/run-batch/csv-reader/index.tsxweb/app/components/share/text-generation/run-batch/res-download/index.tsxweb/app/components/app/annotation/header-opts/index.tsxweb/app/components/app/annotation/batch-add-annotation-modal/csv-downloader.tsxweb/app/components/datasets/documents/detail/batch-modal/csv-downloader.tsxScreenshots
Checklist
make lintandmake type-check(backend) andcd web && npx lint-staged(frontend) to appease the lint gods