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

Thoughts on a Browser version? #142

Closed
abritinthebay opened this issue May 13, 2015 · 14 comments
Closed

Thoughts on a Browser version? #142

abritinthebay opened this issue May 13, 2015 · 14 comments
Assignees
Labels
feature New functionality or improvement

Comments

@abritinthebay
Copy link

I'm using Call to create an isomorphic Router for code that will run on Hapi - Hoek pulls in several things (and has several features) that aren't really required for most Browser usage.

Any objection to me submitting a PR for adding a browser build (including in the package.json) for things like Browserify that minimizes the code for a front-end build?

@nlf
Copy link
Member

nlf commented May 22, 2015

So long as it's easy for me to maintain for future releases, I'm open to a PR adding a browser build

@abritinthebay
Copy link
Author

It would basically involve making the core code a little more modular and then just have simple browser shims for things that weren't required (possibly in separate files)

A simple example would be - add a browser directive in the package.json that tells browserify not to pull in the Crypto lib (as none of that should be happening in the browser because.. oh god that's a bad idea).

But it may be possible to have a few other wins (like smaller browser only shims for some functionality in path or util) and the maintenance should be minimal - worst case scenario would be that browser functionality would lag behind server side, which isn't too bad.

I'd have a vested interest in keeping it up to date too ;)

@jeffbski
Copy link

jeffbski commented Aug 6, 2015

Yes, I was thinking the same thing. I know the Crypto lib is huge in the browser, so that would be nice to eliminate that. Joi uses Hoek, so any wins we have in Hoek will also benefit Joi.

@abritinthebay
Copy link
Author

Once my work project that spawned all this is over (next week or two) I'll be able to open a PR on this.

@jeffbski
Copy link

jeffbski commented Aug 6, 2015

In my experiment, simply stubbing out crypto in package.json shaved off 82KB gzipped from original size.

"browser": {
    "crypto": false
  }

I was building Joi which included Hoek. I documented in this comment hapijs/joi#528 (comment)

@jeffbski
Copy link

jeffbski commented Aug 6, 2015

I think crypto was only used in one place to create a random file name which probably doesn't even apply for browsers. This might be all that we need to do. Simply add the exclusion in package.json.

@abritinthebay
Copy link
Author

Yeah, that's the major case. It's the single biggest difference at least.

@arb
Copy link
Contributor

arb commented Aug 7, 2015

It might be worth shimming in Node util as well. Especially since all that Hoek uses is inherits and format.

@jeffbski
Copy link

jeffbski commented Aug 7, 2015

I pushed a branch on my fork with this change for anyone that wants to try it out. Original is 96KB (gzipped), so it cuts out 82KB (gzipped) resulting in a browser size of 14KB (gzipped).

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

You can easily test this from a project using

browserify -r hoek -s Hoek -i | tee hoek.js | uglifyjs - -c warnings=false -m | tee hoek.min.js | gzip > hoek.min.js.gz

Try with normal hoek, then try npm i jeffbski/hoek#browser to see the reduced version.

@johnbrett
Copy link

@abritinthebay did you ever make progress on the isomorphic Router? Would like to see the code if there is any :)

@abritinthebay
Copy link
Author

I did! The first version is live on BleacherReport.com atm - thought it's mostly in a "beta" form atm. Not published it as OSS yet tho....

@johnbrett
Copy link

Cool! Let me know if you ever do publish it! :)

@nlf nlf self-assigned this Mar 31, 2016
@nlf
Copy link
Member

nlf commented Mar 31, 2016

per outmoded/hapi-contrib#67 (comment) closing this

@nlf nlf closed this as completed Mar 31, 2016
@Marsup Marsup added feature New functionality or improvement and removed request labels Sep 21, 2019
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

7 participants