caolan / forms

An easy way to create, parse and validate forms in node.js
MIT License
1.01k stars 168 forks source link

Browser bundle is very large #192

Open Pinpickle opened 8 years ago

Pinpickle commented 8 years ago

I'm currently trying to use this library for isomporphic form validation/rendering. However, this lifted my (uncompressed) JS size from ~700kb to 3.1mb.

After removing formidable, async and qs as dependencies (formidable has the largest contribution), and leaving the server implementation to parse form data, my bundle size is sitting at around 900kb. You can see the fork here

You could also remove the dependencies on shims (object-keys, is, object.assign, string.prototype.trim), if you just specified that people who want support for older browsers/runtimes need to include polyfills themselves.

That will result in zero dependencies with almost no loss in functionality. The changes being:

I know legacy support is an important feature for this library, as is ease of "plug and play" use, so I wanted to see if there was any interest in reducing dependencies.

ljharb commented 8 years ago

Everybody should be supporting older browsers/runtimes, so I won't be removing any of the shims - also, "zero dependencies" is not a good metric to shoot for. More deps are better.

I'd be interested in refactoring to avoid uses of async, but forms is primarily meant for node, not the browser - fwiw, there is https://www.npmjs.com/package/browser-forms (I'm not sure of its status).

Separately, if there's a way forms could be architected so that it can be used without formidable, and so that those who want the default behavior could somehow inject it, I'd be OK with that too.

Pinpickle commented 8 years ago

Good to know what would line up with your opinions if I were to open a PR. Removing dependencies on formidable, qs and async is very straight-forward. If you change it so handle only accepts an ordinary JS object, and write two util functions to replace async.forEach and async.forEachSeries, you're done. The former only requires users to pass req.body, for example, instead of req. Naturally, this is a breaking change.

I disagree with your point that we should aim to support all older runtimes (I believe this is currently as far reaching as IE8/Node 0.4?). Or that more/less dependencies is strictly better than the other. Nonetheless, there is less to be gained with removing shims so I'll end the discussion there.

browser-forms is quite out of date (last published 4 years ago), so I'll opt for maintaining a fork at this stage.

Pinpickle commented 8 years ago

One more point, the readme has this line:

  • Exploring write-once validation that works on the client and the server. There are some unique advantages to using the same language at both ends, let's try and make the most of it!

If forms is not meant for client-side usage, this line is pretty misleading

ljharb commented 8 years ago

My hope would be to avoid a breaking change for as much of it as possible, and to make the breaking change for users who want the formidable behavior to be as minimal as possible.

forms is indeed meant for clientside usage but that hasn't been its primary use case, is all.

ljharb commented 4 years ago

I'm still happy to accept a PR that removes the async dependency.

voxpelli commented 3 years ago

async gets removed as part of #218

ljharb commented 3 years ago

2f4ba3a4471a0a64c719d0bbcaa6e9d4b1e9ad8a has dropped the bundle size by 12% without any changes, so that's a start. #218 seems to drop bundle sizes an additional 4% (from 1.09MB to 1.05MB). Thus, I suspect async is not the biggest contributor.