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

Evaluate what would be involved in updating to the current version of d3 (v7) #15

Open
alykat opened this issue Apr 15, 2021 · 5 comments

Comments

@alykat
Copy link
Member

alykat commented Apr 15, 2021

No description provided.

@thomaswilburn thomaswilburn changed the title Evaluate what would be involved in updating to the current version of d3 (v6) Evaluate what would be involved in updating to the current version of d3 (v7) Jul 2, 2021
@thomaswilburn
Copy link
Contributor

This will depend on upgrading the rig to handle ES modules, but probably won't necessitate too much more on top of that.

@thomaswilburn
Copy link
Contributor

thomaswilburn commented Jul 2, 2021

The rig has been (theoretically) updated to handle ES modules.

Migration guides from D3:
V5-V6: https://observablehq.com/@d3/d3v6-migration-guide
V6-V7 (release notes): https://github.com/d3/d3/releases/tag/v7.0.0

It doesn't look to me like there are real syntax/API problems for our graphics. I think we could proceed as follows:

  1. Pick some standout graphics, preferably one for each of the major template types, and use those for testing.
  2. Update our individual modules used in those graphics (e.g., moving from d3-array@2 to d3-array@3).
  3. If individual modules work, upgrade the overall D3 library, which we don't really use anywhere but just in case.

@thomaswilburn
Copy link
Contributor

thomaswilburn commented Aug 25, 2021

Unexpected snag! D3 exports itself as ES modules, but still maintains a UMD build in the /dist folder for bundlers. However, while Browserify happily ignores the package.json and loads this file, Node itself does not: the "exports" key in the package.json doesn't mark it as public in a way that Node will follow. You can import in Node, but you can't require.

This is a problem because in our HTML, we sometimes use D3 that's ported from the client-side code to do templating. We need to require() it, because we want modules to resolve from the graphics-js folder, not from the rig location, which is what import does (and this doesn't appear to be monkey-patchable).

We're going to file a bug with D3 and hold off on upgrading for now, but our eventual options look like this:

  • If D3 fixes the bug - we're all good, we can require these modules to our hearts' content.

OR if D3 declines to fix the bug:

  • we move ahead with the upgrade, but specifically warn against using require() for modules in EJS templates. You can still do basic Node processing for data in a template, but nothing requiring a library. This sucks but is sustainable long-term
  • we don't upgrade until D3 changes syntax or introduces significant new features, which they did not really do in the 5->7 transition. This creates a potential stumbling block that I don't know how to circumvent
  • we create an additional hook for requiring modules that's added to the localRequire in processHTML.js, piping the file through Babel before passing it to Node This isn't terrible, but adds complexity
  • we create an alternate load mechanism that's specifically used for EJS templates, similar to but not the same as require(), that can handle both ES (via await import()) and CJS modules, making sure that the pathing is correct for the graphics folder and not the host app. This is elaborate and probably fragile.

@thomaswilburn
Copy link
Contributor

Perhaps unsurprisingly, the D3 project is not at all interested in fixing this bug, so we'll have to figure out other options. d3/d3#3532

@wemod123
Copy link

wemod123 commented Sep 2, 2021

Totally not understand why d3 package only support import but not require,

@thomaswilburn, thank you for the detail advice, we are creating a separate loading mechanism only to handle d3 package, and making it as kind of function-api, really uncomfortable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants