hapijs / joi

The most powerful data validation library for JS
Other
20.93k stars 1.51k forks source link

Slim down dependencies for browser support #528

Closed geek closed 6 years ago

geek commented 9 years ago

Related to this issue: https://github.com/AmpersandJS/ampersand-state/issues/72

hueniverse commented 9 years ago

Doesn't seem worth the effort. Also moving forward, this will be a constant battle to keep the footprint small. People are using joi today with client side code.

Marsup commented 9 years ago

If you see simple things that would help browserify in its task I'll take it, but it's not worth crippling joi for that. Maybe the solution is to work on a better browserify bundle like the one started in fhemberger/joi-browserify.

arb commented 9 years ago

We could also try using the browser option in package.json. Maybe stub out isemail as well and just say it's not supported in the browser?

geek commented 9 years ago

Can we remove moment, I'm not seeing that used anywhere.

Marsup commented 9 years ago

It is used, bad grep skills? :-)

arb commented 9 years ago

I suggest using the browser option and stubbing out the external dependencies (moment, isemail, etc.) with shims and the understanding that they won't work in the browser.

geek commented 9 years ago

Close, I hadn't pulled the latest :/

Marsup commented 9 years ago

Moment already exists for the browser, don't know for isemail. I don't know browserify enough to do that work, what impact would it have?

arb commented 9 years ago

I'm pretty sure it would have 0 impact in node land. The main impact would all be client side: there would be certain tests that would no longer function: probably most of the date ones and the email check. We would use the browser option in package.json to tell browserify to skip "moment" and "isemail" and just provide shims for the client. They wouldn't function, but calling them would also not cause an error to be thrown.

Marsup commented 9 years ago

Why would the date stop working if we have moment on the client side as well ? It seems the shim for isemail is already done in fhemberger/joi-browserify, it seems to be a copy of the interesting parts of isemail. Can't we hint browserify to drop dead code so it can work without shimming ?

arb commented 9 years ago

One of the things mentioned in the related ampersand issue is the moment dependency is why I keep mentioning that and suggest removing or shimming it. I don't think browserify can drop dead code automatically.

It does support transforms though that we could maybe do something with. It also supports swapping modules in and out. https://github.com/substack/browserify-handbook#packagejson.

I feel like we should find out if they REALLY want to use Joi specifically or just use something like Joi.

Coobaha commented 9 years ago

@arb some of dead code can be dropped with uglify when the condition is always false (react example).

Was testing bundling with webpack. (net and dns modules was mocked to empty modules), first minified bundle was 86kb gziped. Removing all locales from momentjs (it was importing all locales files when imported) reduced bundle size to 55kb. Dropping momentjs and isemail reduced bundle size to 40kb.

Maybe dependencies can be swaped to lightweight alrtenatives of isemail/momentjs if some env variable is set and document this? They are used only once.

I really like Joi and it is so cool to share validation schemas with client side, hope it will be more friendly for bundling to the browser in the future :)

steffenmllr commented 9 years ago

@Coobaha Could you please share your webpack config? Tried to slim it down with browserify but wasn't that successful...

Coobaha commented 9 years ago

@steffenmllr to drop all custom locales from moment: new webpack.IgnorePlugin(/^\.\/locale$/, /moment$/) minimize plugins:

   new webpack.optimize.UglifyJsPlugin(),
      new webpack.optimize.DedupePlugin(),
      new webpack.DefinePlugin({
        'process.env' : {
          NODE_ENV : JSON.stringify('production')
        }
      })

and to mock node modules:

  node          : {
      net : 'empty',
      tls : 'empty',
      dns : 'empty'
    }
dvp0 commented 9 years ago

It will be really really awesome to use joi in browser seamlessly (including email validations) so people like me can use the same validations in the REST api + the front/middleware <- This is awesome

tsunammis commented 9 years ago

May be we can have distinct behavior between the server and client side, no ?

Marsup commented 9 years ago

I don't want to have to maintain every tiny documentation difference between both, I'd rather have it all or not.

Marsup commented 9 years ago

As mentioned in another issue, I'm looking for ways to test it without changing the whole test infrastructure, if you have ideas...

chrisabrams commented 9 years ago

Does anyone have an example of how to get Joi to work with Browserify? I get one thing after another...

Buffer is undefined, etc.

jeffbski commented 9 years ago

@chrisabrams I didn't have to do anything special to use Joi with Browserify. I just require joi and it works.

jeffbski commented 9 years ago

I found that since Joi uses Hoek, Hoek includes a browserify version of Node.js crypto which is huge, so making the browser version of Hoek not use that or making Joi not use Hoek would eliminate crypto. By stubbing out crypto from Hoek and moment, it comes down to 30KB gzipped, and can go smaller if eliminating buffer and isemail.

Here is a issue discussing working on a browser version of hoek to reduce size https://github.com/hapijs/hoek/issues/142

chrisabrams commented 9 years ago

@jeffbski could you provide an example of how you stubbed?

jeffbski commented 9 years ago

First I went to a project that was using joi and added an npm run script that runs the following so I could quickly see what size gzipped, minified, browserified joi is.

browserify -r joi -s Joi | tee joi.js | uglifyjs - -c warnings=false -m | tee joi.min.js | gzip > joi.min.js.gz

Default size without optimizations is 126KB gzipped

Then I created my own local copy of Hoek and changed its package.json adding

  "browser": {
    "crypto": false
  }

This stubs out crypto from Hoek and that alone dropped size of joi (and dependencies) to 44KB gzipped.

Then changing my copy of joi's package.json I added the following:

  "browser": {
    "moment": false
  }

Without crypto (in Hoek) and moment (in joi) we get: 31KB gzipped.

Modifying joi's package.json to stub isemail like

  "browser": {
    "moment": false,
    "isemail": false
  }

No crypto, moment, isemail joi size drops to 29KB gzipped.

Next I wanted to see what happens if I eliminate Buffer. I didn't know how to do that with package.json browser field, so I modified the original browserify command adding in the -i buffer

browserify -r joi -s Joi -i buffer | tee joi.js | uglifyjs - -c warnings=false -m | tee joi.min.js | gzip > joi.min.js.gz

and now with no crypto, moment, isemail, and no buffer drops to joi to 23KB gzipped.

Obviously you can't use any joi or hoek functions that rely on those packages, but we can at least start to figure out how we can slim this down or provide lighter implementations for some things.

Summary

Config Joi and dependencies gzipped
Full Joi 126KB
w/o crypto (in Hoek) 44KB
w/o crypto (in Hoek), moment 31KB
w/o crypto (in Hoek), moment, isemail 29KB
w/o crypto (in Hoek), moment, isemail, buffer 23KB
jeffbski commented 9 years ago

So getting rid of Crypto in Hoek (or not using Hoek) is the biggest win losing 82KB off the original size.

jeffbski commented 9 years ago

I think crypto is only used in one place in Hoek in uniqueFilename to generate a random file name path. There are lots of other ways to do this. Or maybe this function isn't even needed in browsers, in which case stubbing out crypto would be fine.

jeffbski commented 9 years ago

I made the changes to package.json browser changes I mentioned above to hoek and joi and pushed them out as branches on my forked repos. It excludes crypto, moment, and isemail but not buffer so joi and dependencies weighs in at about 29KB gzipped.

You can npm install jeffbski/joi#browser to try it out.

https://github.com/jeffbski/joi/tree/browser https://github.com/jeffbski/hoek/tree/browser

jondlm commented 9 years ago

@jeffbski thank you so much for figuring that stuff out! I was going down a much darker path trying to rip stuff out in my fork. @jeffbski's changes mostly consist of adding a few flags to browserify through properties in the package.json. Is there any reason those couldn't land? My team really wants to use Joi in the browser as well.

jeffbski commented 9 years ago

The safe thing to do would be to start with getting the browser hoek w/o crypto and then using that in Joi. That is the biggest win and doesn't eliminate any relevant functionality for the browser (generate uniqueFilename wouldn't work but that should be fine).

Then the other changes do eliminate some functionality around dates, isemail, and buffer so those changes can be discussed. I'm not sure the best way forward for all those since some people might want to use some of them.

Marsup commented 9 years ago

As long as it's non-intrusive in the node world, I'll be glad to review any proposal (to the best of my knowledge in that area).

LKay commented 8 years ago

Any update on this? I'm trying to build browser version of the Joi but it constantly fails on minifying step. I'm using uglifyjs 2.6.1 and browserify 12.0.1. Getting the error:

events.js:146
      throw err;
      ^

Error: Uncaught, unspecified "error" event. (Unexpected token: name (newObj) while parsing file: /Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/joi/node_modules/hoek/lib/index.js (line: 33, col: 8, pos: 524)

Error
    at new JS_Parse_Error (eval at <anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/uglifyify/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:1526:18)
    at js_error (eval at <anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/uglifyify/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:1534:11)
    at croak (eval at <anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/uglifyify/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2025:9)
    at token_error (eval at <anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/uglifyify/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2033:9)
    at unexpected (eval at <anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/uglifyify/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2039:9)
    at semicolon (eval at <anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/uglifyify/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2059:43)
    at simple_statement (eval at <anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/uglifyify/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2239:73)
    at eval (eval at <anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/uglifyify/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2112:19)
    at eval (eval at <anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/uglifyify/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2072:24)
    at block_ (eval at <anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/uglifyify/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2352:20))
    at Readable.emit (events.js:144:17)
    at Labeled.<anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/browserify/node_modules/read-only-stream/index.js:28:44)
    at emitOne (events.js:77:13)
    at Labeled.emit (events.js:169:7)
    at Labeled.<anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/browserify/node_modules/labeled-stream-splicer/node_modules/stream-splicer/index.js:130:18)
    at emitOne (events.js:82:20)
    at Labeled.emit (events.js:169:7)
    at Deps.<anonymous> (/Users/lapkom/Documents/Projects/My-App-ui/v2.1/node_modules/browserify/node_modules/labeled-stream-splicer/node_modules/stream-splicer/index.js:130:18)
    at emitOne (events.js:77:13)
    at Deps.emit (events.js:169:7)

The only thing on that line is:

    let newObj;

so I see nothing wrong.

My browserify gulp task looks like follow:

gulp.task("compileJoi", () => {
    return browserify({
        entries       : ["./" + paths.src.joi],
        standalone    : "Joi",
        debug         : false
    })
        .ignore("buffer")
        .ignore("crypto")
        .ignore("moment")
        .transform("babelify")
        .transform({ global : true }, "uglifyify")
        .bundle()
        .pipe(source("joi.min.js"))
        .pipe(buffer())
        .pipe(gulp.dest(paths.dist.joi))
});
arb commented 8 years ago

With this setup does babelify run on node_modules? I don't use browserify so I'm not sure. I had this problem as well until i added the path the joi to my webpack babel configuration.

LKay commented 8 years ago

When I disable joi then all other libraries included in node_modules compile just fine. The problem is with joi 7.0.0, it seems as if I downgrade to 6.x.x everything works. I guess it's because porting everything including dependencies to ES6 and not providing ES5 compiled dist file. The joi itself seems browserified but dependencies, mainly hoak and topo are not being transpiled as imported code still contains ES6 code such as let const etc. Using node 5.1.0 btw, tried with 4.x and result is the same.

jeffbski commented 8 years ago

Yes, you are right, joi 7.0.0 started using es6 and also many of its dependencies did too and they are not including the

  "browserify": {
    "transform": [
      "babelify"
    ]
  },

in the package.json, so things are not ES5 compatible any longer.

I'd suggest using 6.x.x until we get this sorted out.

bisubus commented 8 years ago

I've forked the repo to bisubus/joi-browser and bundled the library to dist with Webpack. Everything looks great so far.

crypto (used by hoek, irrelevant) and dns (used by isemail with checkDNS option, which isn't supported by the package) were stubbed.

util was mocked with inherits for util.inherits method.

net was mocked with is-ipv6-node for net.isIPv6.

moment was mocked with dummy moment.isValid to fail silently.

Buffer is being replaced with browser-friendly buffer package by Webpack in automagical manner.

105Kb minified, 30Kb gzipped. Not exactly suitable for every application, but looks like a good start. And a huge advantage for isomorphic workflow.

@Marsup It shouldn't hurt Node, I presume. With the ability for Moment to be provided as external dependency in validation options (or dumping it and sticking to ISO date, I seriously don't see how input conditioning can be validator's job) everything else is straightforward. Buffer and everything else can be abstracted. And it would be better to have hoek modularized, like lodash-node (it may save a few Kbs, and crypto is still the problem).

I've made a branch for browser tests (the specs are hard-coded to use Lab, my guess is that they could be refactored to be plugged with Mocha and PhantomJS instead). Moment-related specs are skipped, and Node Buffer was replaced with buffer. It is green for now.

jeffbski commented 8 years ago

I also just got through experimenting with using webpack to create a browser version of joi.

I was able to get it down to 45KB gzipped while keeping moment intact. So the only things stubbed out are crypto, net, dns, and the locales for moment (english is already included as the default).

Thus with this version everything that should work in the browser still works in the browser. We can get it even smaller by excluding things, but that is up to the use case.

For my version, I didn't fork joi but simply included it as an npm dependency it so it would be easy to maintain.

https://github.com/jeffbski/joi-browser

bisubus commented 8 years ago

@jeffbski Yes, wrapping it with another repo is a reasonable move, I don't intend to keep the fork up to date, bundle testing is the only reason there.

Feature-filling util may save some minified kbs, and stubbing net will screw up string.hostname validator.

Marsup commented 8 years ago

The way I see it, bundling is a complete mess currently, and it's mostly worthless to do it on the library's side.

jeffbski commented 8 years ago

@Marsup browserify makes it pretty painless if joi, hoek, topo, and isemail all had the proper settings in package.json.

joi needs the browserify transform set to babelify, same with hoek, topo, and isemail. babelify settings can be specified in package.json or .babelrc.

"browserify": {
  "transform": ["babelify"] 
},
"babel": {
    "plugins": [
      "transform-es2015-modules-commonjs"
    ],
    "presets": [
      "es2015"
    ]
  }

Then hoek's package.json needs browser: { crypto: false } as well to stub out that.

"browser": {
  "crypto": false
}

With those changes, browserify can build right out of what npm installs and we would have the 45KB gzipped version. I tested this just recently.

So if we can just get those properties set in the package.json's for those packages (joi, hoek, isemail, topo) we can get a reasonable browser build.

If people want to squeeze things down further then, they would need to do additional stuff in on their end, but this would be a full working browser build.

Marsup commented 8 years ago

That's the thing, you already made a choice on the bundler, and it will be inefficient on eg. webpack. Sorry but the ecosystem sucks currently, I don't plan on supporting everything that's out there.

AdriVanHoudt commented 8 years ago

can't you just easily make a fork that includes these changes in the package.json? I mean keeping that up to date will be no work since merging the package.json is pretty painless imo

jeffbski commented 8 years ago

@AdriVanHoudt yes, but if wanting to enable browserify builds it is a fork of joi, hoek, isemail, and topo.

So from that standpoint it's easier to just have do a joi-browser repo which uses webpack and just depends on joi. (which is my https://github.com/jeffbski/joi-browser). So no fork just a dependency. It will simply need to be updated when joi updates.

It's main drawback (compared to browserify version) is that since everything is bundled, you can have duplication if you use moment, or hoek in your project in addition to joi. So in that case, if you are using webpack then you would want to merge in the necessary webpack settings and build your own version to avoid the duplication.

The browserify way keeps the settings in each repo and you can still combine as needed, but will require maintaining those four forks.

bisubus commented 8 years ago

@jeffbski I currently made Moment a conditional stub which relies on moment global, I guess it is the most flexible solution, unless the team has plans to dump Moment or make it optional. Relying on global variable is acceptable for browser context and still much better than bundling an instance of either the whole Moment hog or stripped-down piggy (i18n is there for some reason, right?).

jeffbski commented 8 years ago

@bisubus that's a great idea. I have been trying to figure out an elegant way to handle that.

ndabAP commented 6 years ago

Is this something that will be addressed any time soon?

Marsup commented 6 years ago

Is this something that still needs to be according to you ?

ndabAP commented 6 years ago

Yes, I'd like to use Joi for real time frontend validation and later at my backend again to prevent a cluttered database. Therefore, I need the same code twice.

I could use joi-browser but an original vendor support would be much better.

cjnqt commented 6 years ago

Same here - we are currently maintaining a big set of validation rules in 2 different libraries (one for the browser, and Joi for backend). Would love to be able to just Joi

LKay commented 6 years ago

@cjnqt If you're using bundler for browser app such as webpack or rollup then you can easily stub or mock native node libraries or dependencies which you will not be using in your ruleset (net package for ip address validation for example). That's what I'm doing and I'm quite happy with.

ErickWendel commented 6 years ago

This issue can be closed ?

tony-kerz commented 6 years ago

@LKay can you share some examples of the mock/stub technique you are happy with?