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

Some devDependencies should be dependencies? #3

Closed
curran opened this issue Jun 25, 2015 · 12 comments
Closed

Some devDependencies should be dependencies? #3

curran opened this issue Jun 25, 2015 · 12 comments

Comments

@curran
Copy link
Contributor

curran commented Jun 25, 2015

I noticed that d3-interpolate and d3-format are listed under devDependencies rather than dependencies in package.json, however they are hard dependencies (as in, the Rollup build breaks if they are not installed). Perhaps these should be promoted to dependencies?

Here are some notes from my experience trying to use only the ticks functionality of d3-scale:

When importing linear from d3-scale, I get the following error when running Rollup:

Could not find package d3-interpolate (required by /Users/curran/repos/data-reduction/node_modules/d3-scale/src/linear.js)

Also, after I npm install d3-interpolate, I get another error:

Could not find package d3-color (required by /Users/curran/repos/data-reduction/node_modules/d3-interpolate/src/interpolators.js)

Now I need to install d3-color to use d3-scale, however I just want to use the ticks functionality from d3-scale. This feels like it should not be necessary.

After installing d3-color, I get another error:

Could not find package d3-format (required by /Users/curran/repos/data-reduction/node_modules/d3-scale/src/tickFormat.js)

After installing d3-format, the build works. In the end, it is necessary to install d3-interpolate, d3-color and d3-format in order to use the tick generation functionality from d3-scale.

@mbostock
Copy link
Member

The distinction between devDepedencies and dependencies is an NPM concept, not an ES6 concept. They are listed as devDepedencies because you can npm install d3-scale and then require("d3-scale") without needing to also install the devDependencies. The CommonJS main defined in the package.json points to the built file (build/scale.js).

If you’re running d3-bundler (the thin wrapper on Rollup) inside the d3-scale repo, then yes, you’ll need to install the devDependencies. That’s standard practice when you are doing development inside a repository (as opposed to just using require to load it).

If you’re running d3-bundler outside the d3-scale repo (as demonstrated by the examples in the d3-bundler repo), then you’ll need to install the d3-scale dependencies explicitly at the top level. That’s because d3-bundler requires all the modules to be at the top-level, so that there’s no duplication if multiple modules share the same dependencies. For example, both d3-scale and d3-interpolate depend on d3-color, and you don’t want to rollup two copies of that code.

So, if you want to just import linear from the d3-scale repo, you need to first install d3-scale and all of its dependencies:

npm install d3-scale d3-arrays d3-color d3-format d3-interpolate

You also need to install d3-bundler, but you can do that globally if you prefer:

npm install -g d3-bundler

Then you need to define an index module that imports and re-exports whatever code you want to use. Here’s a simple example index.js that creates a d3-like object with only d3.scale.linear:

import {linear} from "d3-scale";

export default {
  scale: {
    linear: linear
  }
};

Lastly, build the bundle:

d3-bundler --format=umd --name=d3 -- index.js > d3.js

In d3/d3-bundler#1, I mention having a d3-bundler --install command (or similar) that would somehow compute the transitive closure of dependencies and npm install them. I’d like to do that, but I also want to build a web interface for point-and-click custom builds. If you want to help with either of those things, that would be great. 😁

@curran
Copy link
Contributor Author

curran commented Jun 25, 2015

Oh yes I see what you mean - thank you for the reminder that devDepedencies and dependencies relate only to the CommonJS build. That makes total sense the way you explain it, closing the issue. Thanks also for the info on how to create a custom D3 bundle.

The d3-bundle work is very cool, but currently I am using your ES6 modules directly via Rollup in my own ES6 modules, not using d3-bundler at all (here's one example). I suppose in this case, I should include the ES6-land dependencies as devDependencies.

I'm not sure if I am taking the right approach by using your ES6 modules directly via Rollup, perhaps I should start experimenting with using d3-bundler. It seems though that if my project is using ES6 and Rollup, it makes sense not to touch d3-bundler. I'm not sure..

@curran curran closed this as completed Jun 25, 2015
@mbostock
Copy link
Member

Yeah. The only thing that’s different between d3-bundler and Rollup is that d3-bundler forces all external imports (e.g., import {linear} from "d3-scale") to be resolved at the top level node_modules. It would certainly be nice to have a more established practices for releasing libraries as ES6 modules (with dependencies on other external ES6 modules), but I think we’re kind of figuring things out as we go here.

@curran
Copy link
Contributor Author

curran commented Jun 26, 2015

I was under the impression that Rollup does exactly that, by looking at jsnext:main (in defaultExternalResolver). So import {linear} from "d3-scale" will be resolved at the top level node_modules (d3-scale/index.js) via the jsnext:main declaration in d3-scale/package.json. It sounds like you are saying d3-bundler forces only this behavior, and does not allow any other kinds of external resolving to happen.

In an ideal world, it would be cool to have something like jsnext:dependencies as a special property of package.json for ES6 packages to declare their dependencies on other ES6 packages that live in NPM. Then there could be some tool that wraps npm install and traverses the jsnext:dependencies dependency graph. Or, maybe such an installer tool could just assume that all external imports correspond to NPM modules. I could imagine a command rollup install that does this, because Rollup already can parse the ES6 dependency graph structure from source files. I wonder if this is something @Rich-Harris has thought about...

Indeed we are in the wild west of ES6, but the way you have set things up is the cleanest and simplest I have seen so far, and I'm thrilled that I can actually write code that depends on the new d3 modules via Rollup and NPM and actually works.

@mbostock
Copy link
Member

The issue with Rollup as it currently stands is that it allows external imports to be resolved locally (like Node does), which can lead to duplicate code. I might be okay with duplicate code if the versions were different, but even with identical versions the external imports result in duplicate code.

For example, let’s imagine that d3-scale depended on d3-color directly (it doesn’t, but imagine…), so its dependencies are: d3-arrays, d3-color, d3-format, and d3-interpolate. d3-interpolate also depends on d3-color. And again for the sake of argument lets say that these dependencies are listed in the package.json dependencies, not devDependencies, so the resulting tree structure looks like this:

d3-scale/
  node_modules/
    d3-arrays/
    d3-color/
    d3-format/
    d3-interpolate/
      node_modules/
        d3-color/

If there’s code in d3-scale that says import {rgb} from "d3-color"; and code in d3-interpolate that says the same import {rgb} from "d3-color";, they will resolve to different symbols (and thus duplicate code) because d3-interpolate has a copy of d3-color available in its own node_modules, rather than using the top-level one in d3-scale.

If you remove the d3-color from d3-interpolate, you no longer get duplicate code, but it seemed error-prone to allow external imports to be resolved this way. If people actually relied on this behavior to install nested dependencies, you can see how it would quickly duplicate code. Like imagine if d3-format also depended on d3-color: now you have three copies of it.

So d3-bundler forces you to install it at the top-level. It sucks that this requires you knowing the transitive closure of all dependencies, but I think we can fix that by having d3-bundler compute the transitive closure of dependencies based on some initial set of direct dependencies… perhaps as jsnext:dependencies… and then install everything to node_modules.

The other thing that d3-bundler fixes is rollup/rollup#24 but that’s trivial. 😁

@Rich-Harris
Copy link

Oops, forgot rollup/rollup#24 was still open... will get to work on fixing that.

Aye, it's vexed. Rollup's default resolver just mimics the node resolver, the only change being that it looks for jsnext:main instead of main. I think the package manager (npm) is probably better equipped than the bundler (Rollup) to handle deduping (especially when you have thorny situations like foo@1.0.0, foo@1.1.0 and foo@2.0.0 as nested dependencies, and you want to be maximally efficient without violating semver constraints), though I'm not sure if npm dedupe handles devDependencies. (It looks like npm 3 will default to flatter installs, incidentally.)

Pragmatically, I think d3-bundler supplying a custom resolver is the best solution, at least for the moment. Ultimately we need a package manager and a bundler that work together to solve these problems, and I think JSPM will be it (some progress towards integrating Rollup into JSPM here).

That doesn't solve @curran's problem though. I wonder if the solution for people who want to use D3's ES6 modules without using d3-bundler is for there to be a master d3 package that simply re-exports bindings from individual packages? Then rather than import {rgb} from 'd3-color', it'd be import {rgb} from 'd3'.

@curran
Copy link
Contributor Author

curran commented Jun 27, 2015

Or, if the package.json of each D3 package declares JSPM dependencies, then JSPM can be used as a package manager for installing, and Rollup can be used for bundling.

JSPM dependencies can be declared like this:

{
  "name": "d3-scale",
  ...
  "main": "build/scale",
  "jsnext:main": "index",
  ...
  "devDependencies": {

    "faucet": "0.0",
    "tape": "4",
    "uglifyjs": "2"
  },
  "jspm": {
    "dependencies": {
      "d3-arrays": "npm:d3-arrays@^0.0.4",
      "d3-color": "npm:d3-color@^0.2.1",
      "d3-format": "npm:d3-format@^0.2.3",
      "d3-interpolate": "npm:d3-interpolate@^0.1.1"
    }
  }
}

In this way, JSPM could be used solely as a package manager, and Rollup could be used solely as a bundler. The two tools don't really need to know about each other to be used together in a development workflow. Just an idea..

I did some experiments:

If I run:

jspm install npm:d3-arrays npm:d3-color npm:d3-format npm:d3-interpolate

The following package.json is generated:

{
  "jspm": {
    "dependencies": {
      "d3-arrays": "npm:d3-arrays@^0.0.4",
      "d3-color": "npm:d3-color@^0.2.1",
      "d3-format": "npm:d3-format@^0.2.3",
      "d3-interpolate": "npm:d3-interpolate@^0.1.1"
    }
  }
}

At this point if I delete jspm_packages, then run

jspm install

Then JSPM generates the following directory structure:

.
├── config.js
├── jspm_packages
│   ├── npm
│   │   ├── d3-arrays@0.0.4
│   │   │   ├── LICENSE
│   │   │   ├── README.md
│   │   │   ├── build
│   │   │   │   ├── arrays.js
│   │   │   │   └── arrays.min.js
│   │   │   ├── index.js
│   │   │   ├── package.json
│   │   │   └── src
│   │   │       ├── ascending.js
│   │   │       ├── bisect.js
│   │   │       ├── bisector.js
... source files
│   │   ├── d3-arrays@0.0.4.js
│   │   ├── d3-color@0.2.1
... source files
│   │   ├── d3-color@0.2.1.js
│   │   ├── d3-format@0.2.3
... source files
│   │   ├── d3-format@0.2.3.js
│   │   ├── d3-interpolate@0.1.1
... source files
│   │   └── d3-interpolate@0.1.1.js
│   ├── system-csp-production.js
│   ├── system-csp-production.js.map
│   ├── system-csp-production.src.js
... source files
└── package.json

15 directories, 117 files

So JSPM basically does the installing that we need (and it does install transitive dependencies). However it seems like it also does some things that we don't need if we are using it just to install packages, like the intermediate files e.g. d3-arrays@0.0.4.js whose contents look like this:

module.exports = require("npm:d3-arrays@0.0.4/build/arrays");

It also installs System.js (and I see no way to turn this off), which we also don't need if we are using Rollup for bundling.

Lastly it generates a config.js, which maps names like "d3-arrays" to the directories they live in, like "npm:d3-arrays@0.0.4". This seems unnecessary, because the mapping is already defined in package.json JSPM dependencies.

In order to make this work, Rollup needs to know how to resolve external imports that live in the JSPM directory structure. This could be implemented by a simple resolver that looks into package.json jspm.dependencies, then resolves into the JSPM directory structure. For transitive dependencies not listed in package.json, the resolver could search the jspm_packages directory tree for packages with matching names, taking the most recent version in the case of multiple versions. I realize this may violate semver, but I'd be happy with this behavior.

It would be ideal if JSPM and Rollup could be used together, and the only thing that would need to change is package.json (specifically, just adding jspm.dependencies). I wish there were a "package manager only" mode of JSPM that only did the bare minimum required for package management, leaving out the following generated files:

  • config.js
  • system-csp-production.js, system-csp-production.js.map, system-csp-production.src.js
  • intermediate source files like d3-arrays@0.0.4.js

I wonder if @guybedford has thought about this case - where one wants to use JSPM as a package manager only, and not as a bundler, and without SystemJS.

@billyjanitsch
Copy link
Contributor

@mbostock npm@3 has now been out and is in fairly widespread use. It flattens dependencies by default, so the problem you mention:

d3-scale/
  node_modules/
    d3-arrays/
    d3-color/
    d3-format/
    d3-interpolate/
      node_modules/
        d3-color/

If there’s code in d3-scale that says import {rgb} from "d3-color"; and code in d3-interpolate that says the same import {rgb} from "d3-color";, they will resolve to different symbols (and thus duplicate code)...

is no longer an issue. (d3-color required by d3-interpolate now resolves to the same d3-color as required by d3-scale, as long as they are the same version.) Further, even for people still using npm@2, most modern bundlers (e.g. webpack) run module de-duplication anyway to prevent such code duplication.

Given this, would you reconsider moving hard devDependencies to dependencies, and compiling the (at least non-minified) builds such that they use require calls to access their dependencies, rather than folding dependencies into the compiled build?

This is what extremely popular libraries like lodash (see the completely modularized npm build) and React (npm build not posted on Github, but see npm i react && cd node_modules/react) are doing these days. (lodash is actually very similar to d3 in terms of its size and (complicated) modularity.)

The current method makes it much more difficult for bundlers to de-duplicate code, and essentially forces the use of d3-bundler, which, while a nice tool on its own, is a pain to integrate into larger build pipelines. (It wouldn't be that bad if it only had to be done once, but it has to be redone every time new parts of d3 are used.) I don't see why this should be necessary when vanilla npm now already solves this problem.

I realize this is just a stepping stone on the road to ES6 modules anyway, but it would be a really nice improvement in the meantime. (And just to be clear, you would still use ES6 modules in the source, but have Rollup compile to CommonJS imports for the npm build.)

TL;DR: switching to dependencies and CommonJS imports for the npm build is standard practice, wouldn't interfere with d3-bundler, Rollup, or ES6 modules, and would alleviate a huge pain point when using vanilla npm and/or other bundlers.

@mbostock
Copy link
Member

mbostock commented Nov 4, 2015

Thanks for the suggestion, @billyjanitsch. I’ve switched the devDependencies on other D3 modules to be dependencies (across all D3 modules), and upgraded to the latest version of Rollup. The main entry point is now a CommonJS build with external dependencies, though I’m also providing pre-built UMD bundles in the NPM repository and as a standalone zip file for convenience.

@billyjanitsch
Copy link
Contributor

@mbostock Thanks! Quick note -- it looks like in some of your commits across the modules, you've changed the build script and updated the dependencies but haven't actually moved them from devDependencies to dependencies (example).

@mbostock
Copy link
Member

mbostock commented Nov 4, 2015

The one you linked is intentional: d3-arrays is only used by d3-random for testing. If you find another one that’s incorrect, please let me know.

@billyjanitsch
Copy link
Contributor

Ah, my mistake, sorry.

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

No branches or pull requests

4 participants