Skip to content
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

Bump d3 from 6.7.0 to 7.5.0 #3361

Closed
wants to merge 1 commit into from

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Jun 29, 2022

Bumps d3 from 6.7.0 to 7.5.0.

Release notes

Sourced from d3's releases.

v7.5.0

v7.4.5

v7.4.4

  • Fix incorrect behavior of d3.bisector when given an asymmetric comparator.

v7.4.3

v7.4.2

  • Fix off-by-one bin assignment due to rounding error in d3.bin.

v7.4.1

  • Significantly improve the performance of d3.bin.
  • Fix the implementation of d3.thresholdScott.
  • d3.pack and d3.packEnclose are now fully deterministic.
  • d3.pack and d3.packEnclose now handle certain floating point errors better.

v7.4.0

v7.3.0

v7.2.1

  • Fix stratify.path when the top-level directory is only a single character.
  • Fix stratify.path when paths contain multiple trailing slashes.

v7.2.0

v7.1.1

  • d3.rank can now take a comparator in addition to an accessor.
  • d3.rank, d3.sort, d3.bisector, and d3.groupSort now require comparators to have exactly two arguments.

v7.1.0

... (truncated)

Commits

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot dependabot bot added the dependencies Pull requests that update a dependency file label Jun 29, 2022
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/d3-7.5.0 branch 3 times, most recently from ef3bcc7 to 3a77ea9 Compare June 29, 2022 22:36
@shaunanoordin
Copy link
Member

OK, this is a major bump. Let's see what details I can dig up before I begin testing...

On FEM, any D3.js changes should only mostly affect:

  • lib-classifier's LightCurveViewer (subject viewer used on TESS)
  • lib-classifier's BarChartViewer (subject viewer... but not sure which project uses it)
  • lib-classifier's ScatterPlotViewer (same as BarChartViewer)
  • app-project's Completion Bar (part of Project Statistics)

The major changes from D3 v6 to v7 are...

  • D3 now ships as pure ES modules and requires Node.js 12 or higher. - shouldn't be a problem for us
  • d3.bin now ignores nulls. d3.ascending and d3.descending no longer consider null comparable. - we don't use d3.bin, as far as I can tell
  • Ordinal scales now use InternMap for domains; domain values are now uniqued by coercing to a primitive value via object.valueOf instead of coercing to a string via object.toString. - I don't think we use this? We use d3.scale() (for continuous-values) on our charts (e.g. LCV), but none of our code has d3.scaleOrdinal() (for discrete values)
  • Array-likes (e.g., a live NodeList such as element.childNodes) are converted to arrays in d3.selectAll selection.selectAll - Hmmm... selectAll is a pretty standard op, so this needs testing.

Status

Next actions:

  • boot up this PR
  • yarn bootstrap
  • try out the TESS project (LightCurveViewer)
    • I should be able to zoom in/out on the Subject Viewer
    • I should be able to add, edit, and delete annotations
    • I should be able to submit Classifications with the correct annotations

If TESS works, the whole D3 upgrade should work, since the LCV uses most of the D3 functions we need to be concerned with. If anybody has any idea where/how I can also test the BarChartViewer and/or ScatterPlotViewer, please let me know.

@shaunanoordin
Copy link
Member

@dependabot rebase

Rebase, I command you! But will additional text in a comment screw up the command? I shall find out.

@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/d3-7.5.0 branch from 3a77ea9 to 891b568 Compare June 30, 2022 12:17
@shaunanoordin shaunanoordin self-requested a review June 30, 2022 12:47
Bumps [d3](https://github.com/d3/d3) from 6.7.0 to 7.5.0.
- [Release notes](https://github.com/d3/d3/releases)
- [Changelog](https://github.com/d3/d3/blob/main/CHANGES.md)
- [Commits](d3/d3@v6.7.0...v7.5.0)

---
updated-dependencies:
- dependency-name: d3
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/d3-7.5.0 branch from 891b568 to c8e70f0 Compare June 30, 2022 12:51
@shaunanoordin
Copy link
Member

Testing update, with macOS + Chrome 103:

  • Planet Hunters TESS/LightCurveViewer works fine on lib-classifier
  • LightCurveViewer renders fine on lib-classifier's Storybook
  • BarChartViewer renders fine on lib-classifier's Storybook
  • ❗ ScatterPlotViewer crashes on lib-classifier's Storybook

Ehhh? Let me check if ScatterPlotViewer is working on master.

@shaunanoordin
Copy link
Member

On master 2fbfa3e, testing in lib-classifier's Storybook, we see that the ScatterPlotViewer...

  • 🟢 renders for Light Theme, Dark Theme, Narrow View, Data with Error Bars, and Kepler.
  • ❌ crashes for "Pan and Zoom" with error: TypeError: Cannot read properties of undefined (reading 'active')
  • ❌ crashes for "Multiple Series Data" with same error as above

On dependabot/npm_and_yarn/d3-7.5.0, the results are the same

OK then, no changes. The Pan & Zoom and Multiple Series Data scenarios need to be fixed, but at least they're not made worse by this PR.

Copy link
Member

@shaunanoordin shaunanoordin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

LGTM 👍 Major bump to D3.js doesn't affect any of our Subject Viewers that use D3. Progress Bar looks fine when testing on some random projects [1] [2] on app-project? Please see full notes above for investigation.

@github-actions github-actions bot added the approved This PR is approved for merging label Jun 30, 2022
@shaunanoordin
Copy link
Member

shaunanoordin commented Jun 30, 2022

WHAT?? The tests for app-project and lib-classifier are failing on:

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/shaun/projects/front-end-monorepo/node_modules/d3/src/index.js from /Users/shaun/projects/front-end-monorepo/packages/app-project/src/shared/components/ProjectStatistics/components/CompletionBar/CompletionBar.js not supported.
Instead change the require of index.js in /Users/shaun/projects/front-end-monorepo/packages/app-project/src/shared/components/ProjectStatistics/components/CompletionBar/CompletionBar.js to a dynamic import() which is available in all CommonJS modules.

and

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/shaun/projects/front-end-monorepo/node_modules/d3/src/index.js from /Users/shaun/projects/front-end-monorepo/packages/lib-classifier/src/components/Classifier/components/Feedback/components/Graph2dRangeFeedback.js not supported.
Instead change the require of index.js in /Users/shaun/projects/front-end-monorepo/packages/lib-classifier/src/components/Classifier/components/Feedback/components/Graph2dRangeFeedback.js to a dynamic import() which is available in all CommonJS modules.

and etc

OK, so upon review:

  • D3 now ships as pure ES modules and requires Node.js 12 or higher. - this IS a problem.

Additional reading:

Status

🤷 throw_hands_up_emoji

Good gravy, I need to check with the other devs to see if anybody else has encountered the ESM import issues. I might have missed something someone on the team is already aware of, since this doesn't seem to be a niche problem.

@eatyourgreens
Copy link
Contributor

@shaunanoordin I've posted about this problem on Slack a few times, and also on the other PRs that it broke. Unfortunately, my comments on the old d3 PR weren't carried across to this new PR.
#2665 (comment)

@shaunanoordin
Copy link
Member

Oh god damn it, Dependabot! This same bump was shut down before, and now you're starting it up again. Blargh.

Thanks Jim! 👍 I'll close this PR.

@dependabot @github
Copy link
Contributor Author

dependabot bot commented on behalf of github Jun 30, 2022

OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting @dependabot ignore this major version or @dependabot ignore this minor version. You can also ignore all major, minor, or patch releases for a dependency by adding an ignore condition with the desired update_types to your config file.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

@dependabot dependabot bot deleted the dependabot/npm_and_yarn/d3-7.5.0 branch June 30, 2022 14:37
@shaunanoordin
Copy link
Member

@dependabot ignore this major version

@dependabot @github
Copy link
Contributor Author

dependabot bot commented on behalf of github Jun 30, 2022

OK, I won't notify you about version 7.x.x again, unless you re-open this PR or update to a 7.x.x release yourself.

@goplayoutside3
Copy link
Contributor

@dependabot unignore d3

Copy link
Contributor Author

dependabot bot commented on behalf of github Dec 10, 2024

OK, I will stop ignoring the d3 dependency.

Copy link
Contributor Author

dependabot bot commented on behalf of github Dec 10, 2024

Looks like this PR is closed. If you re-open it I'll rebase it as long as no-one else has edited it (you can use @dependabot reopen if the branch has been deleted).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved This PR is approved for merging dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants