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

Ideas about reducing library size? #41

Closed
mejackreed opened this issue Dec 21, 2018 · 15 comments
Closed

Ideas about reducing library size? #41

mejackreed opened this issue Dec 21, 2018 · 15 comments

Comments

@mejackreed
Copy link
Contributor

We chatted briefly about this in Edinburgh, but I wanted to create an issue to explore possible ways to reduce the bundle size (188kB). Have you explored any avenues of doing this?

I've thought about a few options including:

  • Modifying build process to provide a slimmed down version
  • Changing from commonjs modules and browserify

Any other things worth trying here?

@edsilv
Copy link
Member

edsilv commented Dec 22, 2018

The first ~700 lines are enums using StringValue. Now that TypeScript string enums are available I've moved all of them to here:

https://github.com/IIIF-Commons/vocabulary/blob/master/src/index.ts

This should slim things down a little bit. I need to update the references in manifesto (and UV + components...) This is probably a day or two's Open Collective work.

I'm minifying manifesto along with other dependencies in the UV build process using uglify. We need to provide a pre-minified version distributed in the npm package.

I want to remove exjs at some point, but it's only ~20k.

I would also like to remove all of the interfaces - they're unnecessary, but don't impact the built file size.

Regarding commonjs and browserify, I've been looking for an alternative to this, but haven't been able to identify another way to build into a single global object from multiple source files without using AMD or System.js.

In other smaller projects (like vocabulary) I've simply put everything in one .ts file and compiled to es6.

An approach like this could work: microsoft/TypeScript#10178 (comment)

@mejackreed
Copy link
Contributor Author

Thanks for all of the helpful context and suggestions here. I may take a stab at one or two of these and submit a PR.

@edsilv
Copy link
Member

edsilv commented Jan 6, 2019

http, https, and url are included here: https://github.com/IIIF-Commons/manifesto/blob/master/src/Utils.ts#L1

Just looking at the outputted UMD module and https://github.com/feross/buffer seems to constitute a significant portion of it. Assuming that's being included as part of http.

I'm wondering if we can just use plain XMLHttpRequest instead or http.request, as per the Dat code here: https://github.com/IIIF-Commons/manifesto/blob/master/src/Utils.ts#L251

However, I'm pretty sure this will break the commonjs-based tests and mean manifesto is no longer isomorphic (at least for http and https). Currently there's no reason why you couldn't use it as a server-side utility too.

@stephenwf
Copy link
Member

Githubs fetch polyfill is quite small: https://github.com/github/fetch and could eventually be removed in the future once IE11 and Safari 10 finally disappear! As another option, theres also the isomorphic fetch which is built on top of that for running on node

@stephenwf
Copy link
Member

Also maybe related, removed jQuery from Manifold using a quick shim around fetch: https://github.com/digirati-co-uk/canvas-panel/blob/master/packages/canvas-panel-core/src/utility/Manifold.js

To reduce bundle size there

@edsilv
Copy link
Member

edsilv commented Jan 6, 2019

@stephenwf I've been working on porting manifold's $.ajax calls to XMLHttpRequest: https://github.com/IIIF-Commons/manifold/blob/webpack/src/ExternalResource.ts#L236
Haven't tested yet. It's a little bit scary porting this stuff as I've encountered all kinds of weird cross browser behaviour in the various auth scenarios. I'm fairly certain that I've covered everything that $.ajax was doing, however $.ajax could have been doing unknown things internally.

I'm wondering if a lot of what's in manifold could move to manifesto. The implementation of IExternalResource for example could be made isomorphic using isomorphic-fetch. The stuff around sortable trees could move I think. It's really the "metadata for all the things" method and getting previous and next ranges that make it valuable, i.e. utilities that must have a notion of your current "position" in the manifest.

I put this together a while ago: http://edsilv.github.io/redux-experiments/www/

Was thinking to add this to manifold to manage state. I maintain that having a generic IIIF redux store implementation on IIIF Commons would be a desirable thing for everyone. Would be keen to get your thoughts.

@mejackreed
Copy link
Contributor Author

I'd be more inclined to looking at fetch option as that feels like a more future proof solution (to replace http, jQuery) in all the things.

@edsilv
Copy link
Member

edsilv commented Jan 6, 2019

@mejackreed isomorphic-fetch, or just plain fetch? I read this the other night that kind of put me off fetch a bit: https://medium.com/@shahata/why-i-wont-be-using-fetch-api-in-my-apps-6900e6c6fe78
What do you think about the state management stuff? Is it worth me pursuing redux as per that example above? TypeScript has been an absolute lifesaver for me over the years for the various refactors I've implemented - hard to shake that habit now... But I don't see the point of me making a separate implementation of something that is 100% generic, just because it's in TypeScript. I can always write a definition file if necessary. I worked on a specialised IIIF annotation project recently that could have benefited from redux. I guess this could be seen as going off topic, but I'm trying to decide what to do with manifold.

@stephenwf
Copy link
Member

isomorphic fetch is the same api, it adds a global fetch function in both node and web. Fetch's promise API is verbose, made to be abstracted, so I don't think theres any harm in either writing your own TS abstraction, or grabbing another abstraction like axios.

+1 for Typescript + Redux implementation for Manifold. I think it would make it clearer what should be in manifold and what should be in manifesto if you move to Redux.

@edsilv
Copy link
Member

edsilv commented Jan 7, 2019

@stephenwf Cool. Excited to move into redux finally. Fetch looks ok - we're really not doing anything too complex other than sending request headers. Keen to use asyc/await syntax with it.

So I'm thinking to:

  • Move ExternalResource from manifold to manifesto and reimplement all ajax calls with isomorphic-fetch (I wonder if it's possible to write tests for the different auth scenarios using the httpserver + mocha setup?)
  • Move my simple TS redux demo into manifold and use that as a store (will extend this to use ids as well as indices eventually)
  • Move sortable trees to manifesto (I think... need to experiment)

@mejackreed
Copy link
Contributor Author

👍 to the fetch conversation and how that is moving forward. isomorphic-fetch or node-fetch seems fine to me.

I haven't really dived too deep into what manifold is doing yet and the relationship to manifesto (I know the theoretical basics) so I don't have strong opinions on that really. But I like the idea of having the ExternalResource in a manifesto. This seems doable without additional overhead.

Any split off items that you want help on? I know a lot of things are moving around, but let me know if there are ways we can work together to move a few of these things forward together.

@edsilv
Copy link
Member

edsilv commented Jan 8, 2019

@mejackreed I'm full-time on a project at the moment, can only look at this on weekends really. I think the main thing now is getting ExternalResource out of manifold and into manifesto, then switching it to use isomorphic-fetch. The webpack branch has the current xhr ported code: https://github.com/IIIF-Commons/manifold/blob/webpack/src/ExternalResource.ts
Not sure what it will take to port that to isomorphic-fetch... Maybe it's not that big of a deal.

@edsilv
Copy link
Member

edsilv commented Jan 8, 2019

We could potentially write some auth tests once in manifesto. Not sure how that would work yet... We'd have to spoof an auth server's responses.

@edsilv
Copy link
Member

edsilv commented Jan 8, 2019

I'd use PR #46 for new work. It's quite far ahead of master and totally refactored.

@stephenwf
Copy link
Member

Library size is now ~60kb will close for now.

https://unpkg.com/browse/manifesto.js@4.2.0/dist-umd/

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