formidable-react-component-boilerplate icon indicating copy to clipboard operation
formidable-react-component-boilerplate copied to clipboard

Move build deps to devDependencies

Open exogen opened this issue 8 years ago • 14 comments

Some discussion has happened around this already, but the gist is that we have builder and builder-react-component in dependencies. And most of our current repos, which are pre-Builder, have babel and webpack in their production dependencies. This not only makes installing our stuff extra slow, but I've found that we're more likely to experience version conflicts or the need to completely wipe node_modules just to upgrade or install a new dependency (some tools in the Karma and webpack ecosystem declare peerDependencies that end up getting mad that all our components have webpack as a production dep).

When transpiling with Babel, nearly all the projects I've surveyed do their build in version or prepublish. But that means they don't support git install URLs, because the build artifacts are only present in the NPM package. To support git URLs, we landed on a solution of conditionally building in postinstall. The problem is that even though the build is conditional, the build dependencies like Babel and Webpack are not: everyone installing through NPM has to bear the cost of installing those, too, no matter what. Not to mention that on principle, they just aren't true deps: they aren't require'd anywhere in the package. They're build dependencies!

I'd prefer to align ourselves with what everyone else is doing: building in version or prepublish. Here are a couple options:

  1. Drop our postinstall method and just don't support git URLs. @coopy says they're an antipattern and that the NPM folks recommend publishing under a scoped package name if you need to depend on a fork. Another option would be to check lib into the repo in a branch of your fork, just for the purpose of being git installable.
  2. Use a smarter postinstall that not only builds conditionally, but installs dev dependencies temporarily only when a build is needed. That way the common case of installing via NPM, which already has the build artifacts, never even pulls down the build deps. To test how this would work, I made a helper for exactly this purpose: postinstall-build

As for Bower, I don't know how it works or why our current method is required to support it, so I'll let someone else explain. But I also don't give a hoot about Bower.

exogen avatar Nov 16 '15 07:11 exogen

@exogen -- Thanks for capturing a potential rearchitecture. I'd like to focus us first by hashing out our project-wide motivations for what we should support as a team and community member, and then after that we can decide on the propers means to get there!

@alexlande @kenwheeler @chaseadamsio @exogen -- I'd love your commentary here about our values and goals for public JS frontend projects (where what can go in git repo source includes src (raw source), lib (es5 transpiled individual files for node / individual requires) and dist (bundle, minified for script tag inclusion).

Some things I would posit that are important to support from a project installation perspective are:

  • [ ] Script Link via URL Reference: Many folks use dist/bundle.min.js URLs from easily identifiable online resources.
  • [ ] npm dependency: I think it's fair to say "installation from npm is a given" ;)
  • [ ] Bower dependency: As much as bower's sustainability is being questioned in the twittersphere and HN-ish venues, there is still a wide install and use base. We should get some real hard stats before dropping this support. Bower has it's own stats page that looks like use is still growing (although likely npm's rate is much faster).
  • [ ] npm dependency via Git fork installations: Patches / forks happen and upstream repos don't often move fast enough to merge and release changes, so teams need an outlet to integrate specific changes earlier. Also, this is a great way to test new packages widely via a branch reference before they're ready for publishing (instead of forcing local download + build + npm link). And, although npm for teams may make publishing a forked project easier, my sense is not publishing a fork and just installing off a git URL is a desirable thing widely in the community.
  • [ ] npm dependency via Private Git installations: Many smaller teams (startups etc) don't currently have the capacity to get private npm running (and/or private npm is too much of a hassle to maintain). I would say it's not super-critical that we support the ability of teams to privately fork projects, but it is a "nice to have".

Thoughts, comments?

ryan-roemer avatar Nov 16 '15 07:11 ryan-roemer

Re: Bower stats, wow I'm wondering if their "Top packages over past 7 days" is correct, because it looks bleak. Number of jquery installs in the past week: apparently 147 (on npm, 178,590). moment installs in the past week: 68 (on npm, 611,658). Is the Bower number missing a few zeros or is that real?

Anyway, one thing I forgot to capture re: Bower support: @alexlande pointed out this redux issue, in which someone points out that you can still install via Bower using the npmcdn.com URL to your dist file. We should consider whether that's "good enough" for us too, or whether we want first-class Bower support.

exogen avatar Nov 16 '15 08:11 exogen

@exogen -- Yeah, the bower numbers for the individual numbers are almost certainly wrong. A certain client project we have easily eclipses that regularly ;)

Using an overly-broad heuristic of searches of GitHub public projects:

ryan-roemer avatar Nov 16 '15 08:11 ryan-roemer

Let's say we wanted complete first-class support for Bower: I'm still unclear how moving those dependencies is related. Looking at Radium's bower.json, it points to dist, ignores both lib and src, and has no build command. And dist is checked into version control. So I'm not seeing how this particular issue would affect Bower – someone enlighten me!

exogen avatar Nov 16 '15 08:11 exogen

If we switched to a prepublish strategy, we could npm publish lib and dist and simply document that bower installation takes place by running bower install http://npmcdn.com/ourlib/dist/index.js.

This provides access for Bower users and those who want a UMD build for things like CodePen. While Bower is still relevant in certain ecosystems, I see very little use in the React ecosystem. Bower use would be an edge case at best, and I feel like providing a UMD build via npmcdn would be accommodating enough.

kenwheeler avatar Nov 16 '15 14:11 kenwheeler

Depending on the complexity of the library at issue, "bower support" can include any of the following:

  • A UMD build (aka dist/)
  • An ES5-ish build (aka lib/)

... and bower unfortunately, cannot run build steps. But, strictly speaking, only the UMD build is a requirement for us, so npmcdn technically support that.

Other side not, IIRC, going off of a URL for bower (like would be the case for npmcdn) gives up ability to manage versions / list outdated packages / etc. Not a deal-breaker, but worth noting (as our bower support is most likely to land in "something's that's better than nothing" once we're done with this discussion).

Let's still chat about desired support scenarios though -- git dependencies (something that I've seen regularly in current and past projects) is something I'd like to have a full discussion on ;)

ryan-roemer avatar Nov 16 '15 14:11 ryan-roemer

Re: git dependencies-- given that this use case is mainly meant to support forks, people in that situation could just remove lib or dist or whatever from their fork's .gitignore, right? That's easier than publishing their own version (although they could in a scoped namespace or similar if they wanted to), and that way we don't have to support the additional complexity of something like a postinstall step on our end for that edge case.

alexlande avatar Nov 16 '15 17:11 alexlande

On a separate note, if bower will work with npmcdn I think that's great.

That said, I think that the recent memification of "bower is dead" is overblown. Bower is still huge. They've only got one maintainer now and not a ton of work is happening, but they've been in that situation for ~2 years at this point. The internet has just jumped on a recent discussion of that fact (plus the npm3 release) to pronounce that Bower is dead/terrible/bad for the JavaScript community/whatever.

I can't say with certainty whether npm has eclipsed bower for front-end usage (I think it's heading in that direction, at least), but it's not something that should be written off.

alexlande avatar Nov 16 '15 17:11 alexlande

I echo Ken's comment.

To wrap @ryan-roemer's list:

  • :white_check_mark: Script Link via URL Reference: npmcdn would solve this
  • :white_check_mark: npm dependency: npm already solves this
  • Bower dependency: I believe npmcdn would solve this and be easy to vet with little work and no impact to projects today (test npmcdn on any projects that reference git@github*/dist/)
  • npm dependency via Git fork installations: Is it possible that people who are using this could just use release clients with npm?
  • npm dependency via Private Git installations: I have no answer for this, but if it's "small", my immediate thought is to not worry about this group until someone raises a concern and has legitimate reasons for it.

curiouslychase avatar Nov 16 '15 18:11 curiouslychase

npm dependency via Git fork installations: Is it possible that people who are using this could just use release clients with npm?

@chaseadamsio I don't follow what this means, could you elaborate?

exogen avatar Nov 16 '15 18:11 exogen

@exogen for sure!

These are just my (not so well-formulated) thoughts on it:

Patches / forks happen and upstream repos don't often move fast enough to merge and release changes, so teams need an outlet to integrate specific changes earlier.

I'm not sure the answer is a good one for the above scenario: If they're using the npmcdn url, this scenario won't work without they have their own package. I think this is the one scenario that exists that I don't have a really good answer for.

Also, this is a great way to test new packages widely via a branch reference before they're ready for publishing (instead of forcing local download + build + npm link). And, although npm for teams may make publishing a forked project easier, my sense is not publishing a fork and just installing off a git URL is a desirable thing widely in the community.

This is where the release client with npm (npmcdn) felt like it could easily apply: If the team has ownership of this package, they can publish a -rc, in which case the version would only be available if -rc was at the end of the dependency in the package.json. This allows you to bump the version anyway you're planning on doing it without actually releasing the version itself.

curiouslychase avatar Nov 16 '15 19:11 curiouslychase

@chaseadamsio Oh! I wasn't following release clients but now I see – release candidates as in -rc builds?

If they're using the npmcdn url, this scenario won't work...

I don't think we're proposing you need to be able to install via arbitrary git URLs using Bower, only that you should be able to use a git URL dependency in npm. The npmcdn thing is only something we're proposing for Bower support, so it's unrelated (AFAIK anyway).

As for publishing an RC build to preview changes, it's an option but I don't think is in the spirit of what @ryan-roemer wants (and doesn't cover forks with a different owner, as you mention – for example, we have a long-running dependency on a require-handlebars-plugin patch that still hasn't been merged, so we depend on the git URL to our fork and can't release an -rc build of someone else's package).

But publishing a @scoped package is a very similar solution, and I think we should investigate just how much friction is involved there – even though it's probably more than using git, it's still probably not much. And you get much saner versioning out of it (...as opposed to no versioning). Let's say your fork was long-running, and you had to keep it in sync with the origin to get other features – you'd have to either keep generating new branches with your patch applied, or risk breaking stuff by reusing the same branch.

exogen avatar Nov 16 '15 19:11 exogen

@exogen sorry for the confusion on using client instead of correctly saying candidate...I couldn't think of the word "hypnotize" earlier either...it's been a weird Monday.

curiouslychase avatar Nov 16 '15 22:11 curiouslychase

This may be a naive suggestion... but for small libraries I've worked on, I tend to just have babel as a devDependency, and I put lib and dist in my repo (ie. don't gitignore them), and I just tell contributors that they have to run the build step before checking in any meaningful changes. Maybe this isn't ideal, and it certainly leaves a bit of room for human error (if the contributor forgets to run build). However, it very simply sidesteps the questions above (I think?), and it makes it the responsibility of the contributor to do the build correctly, rather than hoping the user's setup can build. Everyone always ends up with the same built code this way.

Just a thought. :) In any case, +1 for npm git repo dependencies being an important use case to support - I use them quite often.

dandelany avatar Dec 08 '15 20:12 dandelany