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

[suggestion] Alternate storages (indexeddb/websql) for big assets #89

Open
puzrin opened this issue Nov 11, 2013 · 20 comments
Open

[suggestion] Alternate storages (indexeddb/websql) for big assets #89

puzrin opened this issue Nov 11, 2013 · 20 comments

Comments

@puzrin
Copy link

puzrin commented Nov 11, 2013

After integrating to my projects, i've found, that localStorage can be not enougth for big assets. We can rely only on 2.5mb size (because of unicode). That's not much, if you wish to store wysiwig editor like tinymce and so on. Also, current FF is a bit mad about quotas in mixed (http/https) enviroments.

I suggest to consider adding support for indexeddb/websql. That should be enougth for all modern browsers (who cares about browser-cache fallback in old ones?). SInce basket requests are primitive, it seems that alternate store adapters should not take much of code).

Though, that can make impossible sync basket.get(). But i'm not sure, that it's critical.

I like basket.js idea and could try writing alternate storages support. But wish to know first, if that's ok for this project and will be accepted into mainstream.

@addyosmani
Copy link
Owner

Thanks for the suggestion. We've considered alternative storage methods a few times before but I believe the consensus was that this would be outside the scope of this project. Instead, we generally delegate to either using Lawnchair or a solution like https://github.com/agektmr/PortableCache.js, which does solve the same tiered storage question you present above. Could you try it out and let us know if it solves your use case?

@puzrin
Copy link
Author

puzrin commented Nov 23, 2013

Thanks for the links. Those projects are not useful directly, but contains valuable info for devaloppers.

I did https://github.com/nodeca/bag.js that contains all features i talked about + simple KV storage. But if you ever decide to reconsider basket API, it's never late to merge features back to your mainstream. I'm not gready about glory and don't insist on keepeng bag.js separate :) . Just need bigger storages + kv for Nodeca & Fontello projects.

PS. Not sure, if i should to place your copyrights/info in bag.js header, since almost everything was rewritten. But if you know what to add - let me know. Now info is placed in readme.

@addyosmani
Copy link
Owner

Thanks for sharing the info about bag.js, Vitaly! I would be really interested to hear what @wibblymat and @sindresorhus think of the changes in your project as we were just talking about this the other day. I personally wouldn't mind us exploring other storage formats if we can land them in a way that is modular or can be added in an optional way that wouldn't heavily increase the basic script size for all users.

Could you talk us through how you've approached this? Does your core include larger storage support directly? Love to hear more!

@puzrin
Copy link
Author

puzrin commented Nov 26, 2013

I didn't modularized bag.js, because had no such goal - imho size is ok. Everything is in 1 file and does not require RSVP. https://github.com/nodeca/bag.js/blob/master/bag.js . Splitting to modules is not a big problem, if it's critical. Also, code can be polished to take less size.

More serious problem is, that basket api should be reconsidered (async), to support indexeddb/websql. All key differences & api are described in readme https://github.com/nodeca/bag.js#readme . Don't know, if such radical change is acceptable for your project. It's just my personal vision.

@addyosmani
Copy link
Owner

re-pinging @wibblymat and @sindresorhus for their opinions here.

@addyosmani
Copy link
Owner

and re-pinging again..

@puzrin
Copy link
Author

puzrin commented Dec 27, 2013

Hm... actually, that means taking bag.js sources and overwriting your ones.

  1. That will break API
  2. That will increase size to 3.2kb (gipped)
  3. That will change tests (i've rewritten those to mocha, splitted to parts and removed some)
  4. Version bump

Are you sure???

Of cause, i will be glad to move bag.js back your repo. But please, confirm that you are ready to accept such changes. You can review rurrent code/tests here https://github.com/nodeca/bag.js . If it's ok for you, i will make one PR with code/tests and one PR with docs update (in your style, without my copyrights)

@addyosmani
Copy link
Owner

Feasibility review

  • Basket.js is currently 4.4KB and 1.1KB minified and gzipped. Switching to bag.js takes us up to 3.2KB gzipped + minified. Quite a large change. I would expect us to try offering a build of bag/basket something smaller than 3.2KB which just localStorage support for legacy users and offer up the complete build for developers interested in the whole thing. That said, 3.2KB is still relatively small considering the additions we get. How modular do you think we could get @puzrin? We could always land the changes then work towards modularity if we decide to move forward with the changes.
  • Changing over means that we would not be 100% backwards compatible. I would be interesting in nailing down exactly what breaking changes this brings (what in the bag.js API doesn't cover what is in basket?) over so that at least, we can document them if we go ahead. At present, basket.js does one thing and does it well. Some might argue that what basket.js offers at present is enough and that the newer additions may not be used as often. I'd love to hear more from users here.
  • We recently landed a bunch of changes around different types of resource handling. Are you using the same codebase for that, does bag.js achieve the same thing or similar in terms of what is possible?
  • The changes to tests I'm interested in, but they won't be affecting a decision on my part. Your test coverage includes tests for most main use-cases so we should be solid on that front. Version bump is also okay :)

One idea is to label the current version of basket.js a final 1.0 and make the bag.js version our 2.0, which could indeed come with breaking changes.

@addyosmani
Copy link
Owner

also cc @andrewwakeling for thoughts

@puzrin
Copy link
Author

puzrin commented Jan 4, 2014

  • https://github.com/nodeca/bag.js/blob/master/bag.js#L403 technically it should not be difficult to make adapters pluggable. I had no reasons to do that for my projects.
  • there is one main reason for incompatibility. basket uses sync api for localstorage access (basket.get()). indexeddb & websql are async. You can do nothing with that without api change. Second reason is using node.js-like callbacks instead of promisses.
  • logic is very similar. i've tryed to implement all features of basket from master, because don't like to reinvent the wheel. Main differences are:
    • get method is now for kv access (in pair with set). In basket it looks like dirty hack (can't imagine that someone need it in real life except testing).
    • bag uses arrays of params for parallel loading instead of chaining. And execute scripts when then all loaded. Executing can be improved, but i had no practical reasons to spend time for that.
    • expiration logic for localstore is simplified: when no space - everything is cleared. That can be improved, but in general, if you have big data - just use indexeddb/websql.
    • coding style is from my progects, instead of your one.

IMHO, someone should compare APIs & internals to understand all details. If you skip parts of indexeddb/websql implementations, the rest of code is quite small.

@addyosmani
Copy link
Owner

Thanks for getting back to the questions so quickly :)

  • I'll read up on the adapters, but anything that gives us pluggable options would be amazing. That would definitely make this work landable.
  • Re: incompatibility. We had once discussed using https://github.com/slightlyoff/async-local-storage to offer an async API that felt like localStorage on top of IndexedDB. If we're basically getting a basket-like API on top of the other storage forms, I would be okay with the break in API.
  • Re: callbacks vs promises. Is there a reason bag.js prefers callbacks to promises? With ES6 promises landing in browsers and a shim being available on top of RSVP, it seems like a step backwards to not support them. Is this something we might be able to change? I'm just trying to understand the value of the callback approach.
  • Thanks for your explanation of the implementation and differences. I agree that we can improve the expiration logic to be a little closer to what the current basket offers. I also think that we could help adapt the coding style to adhere to what we've been using so far.

I'm going to spend time reviewing the rest of the bag.js implementation today. Once again, I REALLY appreciate the effort you're taking to explain it to us and work to maybe contribute it back to the project. I'd love to see that happen.

@puzrin
Copy link
Author

puzrin commented Jan 4, 2014

Using callbacks instead of promisses is my personal choice. ES6 is very cool, but we live in real world :) . Using callbacks simplified build system and shortened development time. Note that i'm server-side developper first. And just try to write in similar way on the client.

Personally, i don't like using promisses for simple things :) . I had no chance to land my requests in basket fast, and had to rewrite almost everything because of archtecture/api change. So, i just did all as was convenient for me.

@addyosmani
Copy link
Owner

That's completely understandable. We all have our personal choices :) In my opinion, we could probably switch bag.js to use a similar RSVP based promises solution but I'll review how much work would be involved and how much I could help with it.

Btw, this evening I pushed a new pre-release for basket.js with some of the recent changes we've been working on. I wouldn't mind if 0.3.x was the last version to support our current API. I'll do some research on just how many users are currently relying on 0.3.

@puzrin
Copy link
Author

puzrin commented Jan 5, 2014

By the way, after removing indexeddb/websql adapters, bag.js size is 2.3K gzipped. Not sure, that 0.8K size benefit is big enougth to care about modularity.

Also, when compared sizes, you missed RSVP dependency. Full basket version takes 4.9K gzipped. That's bigger than bag.js with all storages included :)

@andrewwakeling
Copy link

bag.js looks great!

Regarding the CSS feature in bag.js, I would consider appending the style element before setting the CSS. There's some known issues with orphaned style elements in IE.
The CSS feature in bag.js (unlike the JS solution) has some caveats and in some cases, people may need to modify their CSS to get it working. I have discussed these issues in #60. Documenting those caveats would be preferable as reliably solving those issues would be difficult.

I have limited experience with Promises, but I usually find that callbacks are sufficient for my needs. I am not a huge fan of shims as they tend to bloat libraries. Even though they are "optional", I find myself always loading them because I need to support older browsers. Having said that, I do not feel strongly about the RSVP dependency being in basket.js. The difference in size is still fairly negligible.

Can we support callbacks if the RSVP dependency is absent?

I am a big fan of modularity but I lack experience over both projects to say that it's needed right now.

Also, I really like the new pre-release changes to basket.js. Thanks!

@puzrin
Copy link
Author

puzrin commented Jan 5, 2014

Thanks for notes about CSS. I'm not expert about browser issues. Prefer waiting final solution in basket, to do the same in bag.

Supporting both callbacks & promisses will be overengeneering.

@addyosmani consider upgrade to RSVP v2+. It's more compact.

@mitermayer
Copy link

would be keen to know whats the current state of this :) loved both projects

@addyosmani
Copy link
Owner

I actually haven't had a chance to compare final binary sizes here at all. What are folks thoughts on us switching over to using the global Cache API that used to just be part of Service Worker but is now part of the global window and in Chrome (soon coming to Firefox)? We mentioned it in #131. If it's less desirable than us looking into other storage mechanisms we can think about that. Ultimately, I think continuing to strive for a lean, but modern base here would be a good target.

@puzrin
Copy link
Author

puzrin commented Jun 21, 2015

What are folks thoughts on us switching over to using the global Cache API that used to just be part of Service Worker but is now part of the global window and in Chrome (soon coming to Firefox)?

As user i need script that "just works" and don't care much about internals.

@puzrin
Copy link
Author

puzrin commented Feb 16, 2016

May be close? Seems nothing changed to many years. I'm surfing https://github.com/issues and try to short list of issues opened by me.

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

4 participants