-
Notifications
You must be signed in to change notification settings - Fork 115
Update d3 to v7 from v5 and update related dependencies and devDependencies. #5835
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: master
Are you sure you want to change the base?
Conversation
…encies. This change modifies the package.json and package-lock.json files to reflect the updated versions. This change also modifies javascript code to be compatible with d3 v7. Main d3 changes from v5 to v7 include: Removal of d3.event (https://observablehq.com/@d3/d3v6-migration-guide) Removal of d3.mouse and use d3.pointer instead
Summary of ChangesHello @shixiao-coder, 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 focuses on a critical technical upgrade, migrating the D3.js charting library from version 5 to version 7. This update necessitates broad changes across the codebase to adapt to D3's revised event handling mechanisms and API. Concurrently, the TypeScript environment has been modernized with an upgrade to the latest compiler version and updated D3 type definitions, ensuring continued type safety and compatibility with the new library version. The overall impact is a more current and maintainable charting infrastructure. Highlights
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 successfully updates d3 from v5 to v7, addressing breaking changes like the removal of d3.event and d3.mouse. The changes are generally correct and follow the migration guide. My review focuses on improving type safety by replacing the widespread use of any for event handlers with more specific types like MouseEvent, PointerEvent, or d3.D3ZoomEvent. This will enhance code maintainability and catch potential bugs. I've also identified a potential issue with handling an undefined dataset value that could lead to runtime errors.
As part of #5835 , the target in tsconfig.json needs to be upgraded from es6 to es2021. This PR isolates the es2021 change. The main difference appears to be how dynamic imports are treated. This PR: * Updates the module in tsconfig.json to "esnext" instead of "commonJS". This allows webpack to correctly use `import()` as we have across the codebase * Updates the handling of dynamic imports (primarily in `loadLocaleData()`) to be es2021 compatible * Includes some trailing lint fixes to get pre-merge tests to pass --------- Co-authored-by: Xiao Shi <shixiao@google.com>
…encies. This change modifies the package.json and package-lock.json files to reflect the updated versions. This change also modifies javascript code to be compatible with d3 v7. Main d3 changes from v5 to v7 include: Removal of d3.event (https://observablehq.com/@d3/d3v6-migration-guide) Removal of d3.mouse and use d3.pointer instead
dwnoble
left a comment
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.
Thanks @shixiao-coder ! i tested the various web components, the /explore search results, and the /disasters pages locally and everything worked for me
|
thanks xiao! would be worth running this through the screenshot suite -- @gmechali can help point you to instructions on how to do that for a local change. separately, would be great to hold off on merging this until after the later part of this week |
This change modifies the package.json and package-lock.json files to reflect the updated versions.
This change also modifies javascript code to be compatible with d3 v7. Main d3 changes from v5 to v7 include:
Removal of d3.event (https://observablehq.com/@d3/d3v6-migration-guide) Removal of d3.mouse and use d3.pointer instead