Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Drop chokidar #364

Closed
Rich-Harris opened this issue Aug 19, 2018 · 8 comments
Closed

Drop chokidar #364

Rich-Harris opened this issue Aug 19, 2018 · 8 comments

Comments

@Rich-Harris
Copy link
Member

See #363.

I had resigned myself to keeping chokidar, even though it accounts for most of Sapper's size, on the basis that since webpack also uses it, any user of Sapper is going to have to install it anyway.

But that's not entirely accurate. webpack is a dev dependency, whereas Sapper is a main dependency. That means that if Sapper dropped chokidar, a production Sapper app wouldn't depend on it. That would be great in e.g. serverless environments.

So maybe we should try cheap-watch instead after all? All we're doing is seeing if files get added/removed in routes in dev mode.

@Conduitry
Copy link
Member

If there's anything you run into while trying to use cheap-watch, let me know! Probably the main weirdness with it is that you need to specify the files to watch via a function that inputs a path and outputs a boolean, but that's probably not a huge setback with Sapper's usage of file system watchers.

@Conduitry
Copy link
Member

Another thing I guess is that cheap-watch async/await and so needs Node 8. I don't know what the desired target for Sapper is.

@Rich-Harris
Copy link
Member Author

The filter function is great, makes it straightforward to ignore 'private' directories e.g. _components, wherever they are in the tree. Not sure how you'd go about that with globbing.

I think the Node 8 thing is probably ok, since it's only used in dev mode. If people are still developing in Node 6 (as opposed to deploying to Lambda or whatever) then it's self-inflicted.

@Rich-Harris
Copy link
Member Author

Rich-Harris commented Aug 20, 2018

Released 0.17. Think it's time for a quick retrospective — these are the sizes of node_modules if you install nothing but Sapper to a fresh project:

0.15 — 22.2Mb

sapper-dependencies

0.16 — 15.1Mb

sapper-0 16-dependencies

0.17 — 4.3Mb

sapper-0 17-dependencies


(html-minifier isn't Rollup-friendly, it turns out. Oh well. Also, I've been thinking that source-map-support probably shouldn't be installed by default; will open a separate issue for that. tslib needs to be installed as a dependency rather than a dev dependency otherwise you get red squigglies; if we ditch Node 6 support, we can change the compilation target and that will no longer be the case.)

Now we just need to get the rest of the JS ecosystem on board.

@lukeed
Copy link
Member

lukeed commented Aug 20, 2018

Super cool!

I only have one hesitation. In the case of Sapper it doesn't matter much, due to the application context/targeting, but not sure how I feel about getting rest of the ecosystem on board. All these deps are still included in Sapper, but they're now inlined (which is amazing for boot time & true code usage) but now any other peer dependencies in a project that may be using some of the same libs will result in duplicate code .. and potentially a lot of it. Even if other library authors got on board and inlined, you'd still wind up with duplicate (partial) dependencies which are no longer identifiable & so cannot be tree shaken further.

I think the short term fix, on an ecosystem level, is to choose packages with shallow trees behind them.

In the long term, we need a better install tool and registry to cut down on our network costs per project. Then, we need a single, dependency-wide Rollup/bundler that builds out the project with treeshaking & co.

Libraries ship with dependencies attached, for identity (and possible, automatic inlining at application level) whereas applications build for production by default as they're not to be further consumed.

Basically, I've just described Rust's Cargo and I think it's totally feasible.

@Rich-Harris
Copy link
Member Author

By 'getting everyone on board' I was more thinking about persuading developers to reject silly dependencies, which would take a combination of a) more in-your-face tooling, b) education and c) providing alternatives (which no-one is doing more of than you!). But the question of when to bundle and when not to is interesting, so allow me to ramble for a bit:

In theory I agree. In practice, I very often find that the potential for deduplication is highly overstated — partly because different packages only occasionally have overlapping functionality within the same app, partly because different authors favour different dependencies, and partly because the JS community is somewhat undisciplined about keeping versions up to date.

I'll pick on micromatch one more time because it's a good example. There are 83 nodes in its dependency graph, 4 of which are different versions of kind-of — even though kind-of and many of the packages that directly depend on it (some of which are also duplicated within that graph) are written by the author of micromatch! When you're talking about first-order dependencies by different authors, it generally gets worse.

I don't think there's a hard and fast rule about when it makes sense to bundle and when it doesn't, but the heuristic I go by is something like this:

  • is this dependency small enough that it doesn't really matter?
  • is this dependency unlikely to be depended on by other packages that are used in the same project as this one?
  • if either is true, then bundle, otherwise don't

It's also a question of what you're optimising for. In Sapper's case I'm less concerned about avoiding overlapping dependencies with (say) webpack than I am with bringing startup time to the absolute minimum, since a production Sapper app doesn't depend on webpack. I don't think we need worry about dev time duplication.

So, yeah. If the ecosystem wasn't such a mess (i.e. packages with ridiculously deep dependency trees weren't more or less unavoidable) I would be entirely on board with that plan. Until then I think judicious bundling is a sensible workaround. As a final exhibit, let me just leave this here...

npm install sapper@0.15  9.86s user 3.98s system 111% cpu 12.383 total
npm install sapper@0.17  3.90s user 0.96s system 112% cpu 4.322 total
yarn add sapper@0.15  3.44s user 3.49s system 117% cpu 5.883 total
yarn add sapper@0.17  0.73s user 0.36s system 82% cpu 1.324 total

@leeoniya
Copy link

leeoniya commented Aug 20, 2018

would be neat to crawl npm, grab packages with most dependants/dependencies, generate the graphviz trees and auto-send an email to the authors with the graphic and describing the problem. or auto-open github issues. it just might work.

@lukeed
Copy link
Member

lukeed commented Aug 20, 2018

@Rich-Harris Completely agree, 100% 👍 Again, wasn't picking on Sapper at all as I think the choice here makes perfect sense.

One day, I'd like to see (or make) that Cargo for JS. I do think it can solve these problems for us, even the version mismatching n-levels deep.

I also like @leeoniya's idea (👋) about automatic PRs for outdated dependencies that wouldn't naturally resolve to the latest version of XYZ deps, either by pinned versions or wrong majors.

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

No branches or pull requests

4 participants