forms icon indicating copy to clipboard operation
forms copied to clipboard

Browser bundle is very large

Open Pinpickle opened this issue 9 years ago • 7 comments
trafficstars

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:

  • You have to parse the form data before handing it over
  • You have to polyfill JS features that aren't available

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.

Pinpickle avatar Oct 29 '16 17:10 Pinpickle

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.

ljharb avatar Oct 29 '16 17:10 ljharb

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 avatar Oct 29 '16 18:10 Pinpickle

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

Pinpickle avatar Oct 29 '16 18:10 Pinpickle

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 avatar Oct 29 '16 20:10 ljharb

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

ljharb avatar Mar 10 '20 21:03 ljharb

async gets removed as part of #218

voxpelli avatar Jan 19 '21 23:01 voxpelli

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.

ljharb avatar Jan 20 '21 00:01 ljharb