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

Refactor data store on frontend #95

Closed
igoradamenko opened this issue Jun 23, 2018 · 10 comments
Closed

Refactor data store on frontend #95

igoradamenko opened this issue Jun 23, 2018 · 10 comments

Comments

@igoradamenko
Copy link
Collaborator

On frontend we use Store class which is defined here:

https://github.com/umputun/remark/blob/master/web/app/common/store.js

It's a regular singleton, which implements some logic for getting / setting data and adds availability to listen changes. I didn't want to use something like Redux, because we used Preact and it would be strange to bloat our frontend with a huge library because of a simple store abstraction.

But I'm pretty sure that it's possible to rewrite this implementation to make it clearer. I think we should try to do it.

@Guria
Copy link
Collaborator

Guria commented Jun 23, 2018

Redux is not so bloated in fact. But it isn't limited as just state store. It could bring much more. Devtools support will help better create mental image of the app. It opens a lot of render optimizations for free. It also helps to better decouple components #96 .
It will also make contributions for newcomers easier.

@igoradamenko
Copy link
Collaborator Author

Redux is not so bloated in fact.

It's true for SPA, but we create a widget and we need to try keep them as small as possible.

I understand cons and pros of Redux, but IDK why we need it here. Do you have any ideas? Why we need redux-like store?

@igoradamenko
Copy link
Collaborator Author

Also don't forget about bindings for redux — preact-redux:

image

So, these two have size which equals to whole remark42 package (except polyfills: #102).

@Guria
Copy link
Collaborator

Guria commented Jun 24, 2018

It still just 1/4 of babel-polyfill. :) I will return to this issue right after #102

@Guria
Copy link
Collaborator

Guria commented Jun 25, 2018

Why we need redux-like store?

It just too hard to pass-through state managed in few root components to multiple nested childs by props. It's hard to manage and hard to build a mental image of the app in the head.

Redux enables us the way to pass data directly to components and track what is happening in the app.
And I am sure it is crucial for 2 important things:

What about bundle sizes. #102 already shrinks a lot of space in main remark.js bundle. We can go further and split the vendor bundle to contain preact, axios, redux and preact-redux. It will then remove duplication in existing bundles. So in sum we will be still in the win.

@Guria
Copy link
Collaborator

Guria commented Jun 25, 2018

A bit of stats with minified uncompressed sizes:

current:

Length Name
20310 counter.js
2673 embed.js
57366 last-comments.js
144121 remark.js
224470 Total

with #102

Length Name
20298 counter.js
2673 embed.js
57272 last-comments.js
70086 polyfills.js
91998 remark.js
242327 Total (old browsers)
172241 Total except polyfills

with #102 and redux + preact-redux

Length Name
20298 counter.js
2673 embed.js
64400 last-comments.js
70086 polyfills.js
113157 remark.js
270614 Total (old browsers)
172241 Total except polyfills

with #102 and redux + preact-redux and vendor bundle

Length Name
5701 counter.js
2234 embed.js
33970 last-comments.js
70056 polyfills.js
50425 remark.js
63086 vendor.js
225472 Total (old browsers)
155386 Total except polyfills
  • we will have to load vendor bundle with counter and last-comments entry points even though they don't need preact and other stuff. But it allows dedupe axios and other possible shared deps. Anyway vendor bundle will be downloaded only once and reused from cache for every entry.

@Guria
Copy link
Collaborator

Guria commented Jun 29, 2018

@igoradamenko what about a little PoC with redux before making final decision?

@igoradamenko
Copy link
Collaborator Author

@Guria, let's do it.

I am definitely not against Redux at all. And I like your point about getting more contributors and clearer components structure.

I don't like Redux everywhere in my projects, but there I can control everything. Remark42 is an OSS, so we really need to think about contributors and easy way for them here.

Let's try to use Redux, if you want to.

@Guria
Copy link
Collaborator

Guria commented Jun 29, 2018

Ok, I'll take this task then

@Reeywhaar
Copy link
Collaborator

Everything is moved to redux, consider it fixed.

@umputun umputun added this to the v1.3 milestone Apr 15, 2019
@umputun umputun closed this as completed Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants