Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browser bundle is very large #192

Open
Pinpickle opened this issue Oct 29, 2016 · 7 comments
Open

Browser bundle is very large #192

Pinpickle opened this issue Oct 29, 2016 · 7 comments

Comments

@Pinpickle
Copy link

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.

@ljharb
Copy link
Collaborator

ljharb commented Oct 29, 2016

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
Copy link
Author

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
Copy link
Author

Pinpickle commented Oct 29, 2016

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
Copy link
Collaborator

ljharb commented Oct 29, 2016

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
Copy link
Collaborator

ljharb commented Mar 10, 2020

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

@voxpelli
Copy link
Contributor

async gets removed as part of #218

@ljharb
Copy link
Collaborator

ljharb commented Jan 20, 2021

2f4ba3a 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.

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

No branches or pull requests

3 participants