Let's talk about dependencies...

[mojodna]

peerDependencies don't make sense because npm 3 doesn't install them, introducing an extra step before being able to work on this module. Conceptually, they're right for the non-bundled, webpack-ed build.

dependencies seems like the most appropriate way of defining them (they install when developing on this and are included for npm-style resolution), but it adds to the footprint when only the bundled assets are used.

devDependencies would install necessary runtime libraries and facilitate bundling (and keep the footprint small), but it would preclude npm-style module resolution (because nested modules would be unavailable).
#10's approach (facilitating npm-style resolution, which we're not currently doing) seems like the most appropriate mechanism for our use-cases, but we're also trying to hit these:

1. bundled assets, suitable for inclusion via <script>, with external (to avoid potential duplication) references to compatible dependencies (e.g. Leaflet, d3, ...)
2. bundled kitchen sink, suitable for inclusion via a single <script> tag (including all dependencies).

We're currently using (1), but incorporating it into a separate build system and potentially introducing confusion around how to incorporate upstream dependencies (e.g. react-leaflet).

Thoughts?

[ericsoco]

I'm not sure that supporting inclusion via <script> is something we really want to aim for. We're targeting an intermediate+ JS developer audience, one who should be ok running an npm install. If we make that explicit in our docs (I think we already have, in @panorama/toolkit and in panorama-template, I think our bases are covered.

That means that #10 is all we really need to aim for.

Going the npm route means also that we don't have to worry about the upstream deps confusion you mention -- npm will handle this if we get our build right.

[ericsoco]

Re: peerDependencies, yeah, it's annoying to have to run npm install twice, but peerDependencies theoretically allows us to avoid duplicate package versions in a @panorama/toolkit consumer. I'm still leaning toward peerDependencies because
a) I don't like the extra footprint of dependencies being installed in projects that don't use all of them ( / all of the components), and
b) because devDependencies will leave out required libs in an npm install scenario, as you describe above.

[ericsoco]

@mojodna have you seen @sconnelley's work here? I think he's moving toward what I describe above / what you are talking about in #10. @sconnelley correct me if I'm wrong / missed anything.

[mojodna]

I'm not sure that supporting inclusion via <script> is something we really want to aim for. We're targeting an intermediate+ JS developer audience, one who should be ok running an npm install. If we make that explicit in our docs (I think we already have, in @panorama/toolkit and in panorama-template, I think our bases are covered.

That means that #10 is all we really need to aim for.

Going the npm route means also that we don't have to worry about the upstream deps confusion you mention -- npm will handle this if we get our build right.

If this is the case, we only need a babel prepublish step (to make things ES5-compatible), webpack is unnecessary and confusing. #10 is webpack-based, as I understand it.

Re: peerDependencies, yeah, it's annoying to have to run npm install twice, but peerDependencies theoretically allows us to avoid duplicate package versions in a @panorama/toolkit consumer. I'm still leaning toward peerDependencies because
a) I don't like the extra footprint of dependencies being installed in projects that don't use all of them ( / all of the components), and

If we're going pure npm-facilitated require (followed by a webpack build of the eventual artifact), the extra footprint is only on the local filesystem. If the versions match, they'll use the same instance, if not, minor additional footprint (though without problematic version interference).

Does npm dedupe as a post-install step help?

I'm against using peerDependencys here because we're declaring direct dependencies, not stating assumptions about dependencies that the parent project may include.

b) because devDependencies will leave out required libs in an npm install scenario, as you describe above.

Agreed.

[ericsoco]

Coming back into this after a hiatus.

IIRC, we use peerDependencies because a project using @panorama/toolkit is not necessarily going to be using the modules that require each of those peerDependencies. So yes, these are direct dependencies, to be used at runtime, but only if the modules that require them are used in the downstream project.

Would there be any problems with a postinstall step that manually npm installs each of the peerDependencies? Other than maintenance of that list in package.json when new peerDependencies are added.

AFAICT, the goal is to have all the peerDependencies available locally in case they're needed, but to only package and deploy the ones that are actually used.

[ericsoco]

Here's one reason peerDependencies is weird: npm throws EPEERINVALID warnings for uninstalled peer deps from a downstream project (using @panorama/toolkit). Doesn't have anything to do with whether or not any modules that use those peer deps are in use in the downstream project, it happens on npm install.

I'm beginning to wonder if we want a solution like what lodash does, with all modules contained within a single repo but built out into a series of separate, discrete npm packages.

I don't know how to do that tho, especially with our need for per-module stylesheets.

[ericsoco]

And in fact...if the downstream project doesn't manually install those peer deps, it won't run, because @panorama/toolkit currently builds a hard requirement for those peer deps into dist/components.js (which is what is specified in package.json as the main for the package).

This really makes me think we need to figure out how to have separate builds per module.

[mojodna]

Agreed on package per module.
On Wed, Mar 23, 2016 at 6:56 PM Eric Socolofsky notifications@github.com
wrote:

And in fact...if the downstream project doesn't manually install those
peer deps, it won't run, because @panorama/toolkit currently builds a
hard requirement for those peer deps into dist/components.js (which is
what is specified in package.json as the main for the package).

This really makes me think we need to figure out how to have separate
builds per module.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#12 (comment)

[mojodna]

If it doesn't run without them, aren't they true dependencies? ;-)
On Wed, Mar 23, 2016 at 8:14 PM Seth Fitzsimmons seth@mojodna.net wrote:

Agreed on package per module.
On Wed, Mar 23, 2016 at 6:56 PM Eric Socolofsky notifications@github.com
wrote:

And in fact...if the downstream project doesn't manually install those
peer deps, it won't run, because @panorama/toolkit currently builds a
hard requirement for those peer deps into dist/components.js (which is
what is specified in package.json as the main for the package).

This really makes me think we need to figure out how to have separate
builds per module.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#12 (comment)

[ericsoco]

I misspoke -- it won't build without them.

[ericsoco]

Here's another style: https://github.com/mattdesl/eases

I'm not sure yet if this would work for us, needs some thought. But the idea is that the library is not compressed or transpiled (the latter, we can discuss), but is brought in as raw source. And each component has a single entry point, a js/x file, and you just import that file (and it imports whatever other local files are necessary).

Still don't see how this will help with dependencies at the npm level (would still have to specify everything used by each component in dependencies).

Another, somewhat-related thought: maybe we need to offload some of the complexity to the downstream project, in terms of determining which dependencies from the toolkit to package into its build? A bit less portable but we can shepherd the process, esp. on internal Richmond/Stamen projects. Might be simpler.

[ericsoco]

Oh yeah and that ^^ is somewhat similar to react-leaflet, maybe something in that structure we can use...

[ericsoco]

Looks like this is all worked out now in our fork. Closing.