Frontend refactoring - Githubissues

Hey there!

I'm going to declare that I don't have enough time for Remark now, and it's pretty sad, because frontend part should be refactored (or maybe rewritten). I'm sure that I can't do it by myself without any help (believe me, I've tried). So I hope that there are some guys which want to be a part of this project and make all things better.

Right now frontend part of the project has some mistakes that should be fixed.

It works inside an iframe

Yeah, I thought it was a good choice, because when we started to create Remark we believed (at least I did) that it would be Disqus-like system with all related things. After some months of development it became clear that ideas moved to the unexpected way, and today frontend part of Remark is more like yet another one frontend component, such as any npm library, rather than iframe-widget, such as Disqus. And it's fine for users to control everything around Remark manually. They want to change styles, and it's OK for them to have some sort of side effects.

So, it's clear that the future of Remark is iframe-less component, such as any other framework-based component from npm (for instance, like many others React components).

If we remove iframe from the basement of the lib, we will achieve the next goals:

We will reduce lags and freezes that are caused by message-passing between parent and child windows.
We will cut off some useless code, that was duplicated because we have some “external” parts of the main widget, such as user-info widget.
We will automatically implement styles customization (#5), because it will be possible just to override styles on the top level. And they will be able to inject Remark in more gentle way (#184, #216).
We will make possible user extensions. For example, it will be easy to pass some function-like parameters to change a renderer, which will help us with things like #31 or #32. Or to localize it by passing object with strings or connecting sort of middleware (#10).
We will fix major part of problems with auto-scrolling, anchoring and things like #82, because widget will have access to the main window object and things like listening location changes, etc.
We will make it work with things like Grammarly (#138).
We will fix major part of the problems that caused by CORS and things like these (#218), because it will become a “native” part of the website, not “injected from some sources”.
We will make it easier to implement things like DOM watching, because we will use Preact / React and all related things in the context of the main window, not though an iframe.
We will be able to implement things like pushes (using Push API) or other similar notifications (using Web Notification API). It looks like it's an another one interesting approach to implement notifications (#265). I'm not sure that it's possible to do now, because I think that we must have direct access to the main window to implement things like these (but I didn't check actually).
And, of course, we will publish an npm library to make it easier to use Remark in SPA-like projects (#240).

It uses a custom store

...and some pieces of Redux.

I see now that @Guria was right about Redux-way here. When we started to develop Remark, I chose the way of implementing my own bicycle with squared wheels. And it works, it has easy to understand API, but it's now a usual frontend-2019-way to implement stores in web applications. I think that it's followed by some other problems:

It's harder to find people who want to support Remark development. Of course it's not the root of the evil here, but I'm sure if we had regular store, it would be easier for other developers to help us.
It doesn't covered by tests.
We actually not sure how it works. I mean, it's clear how store works, but it's not so clear how it interacts with Preact. When does it trigger render? Is it sync or async? We don't have proper docs for our store, and it's bad.

Actually, everything here is connected to #95 and was discussed there.

It has a bad structure

We don't have proper tests (#93). Yeah, it's not too easy to run them on each build, but it would be nice to have them and to run before every release. It also can make it easier to understand how things work.
We don't have mocks (#86). Yes, we have a dev env for Docker, but, c'mon, I really doubt that every frontend developer understands how it works, how to run it and how to build their part using it. It's better option just to have mocks on frontend side, which is way more better and easier than try to understand some “backend stuff”.
We don't have understandable component system (#96). Yeah, we have some, but most of them are hundreds-lines monsters and it's impossible to understand how they work.
We have several widgets in one repo and it's not clear how they are separated between each other (are they actually separated?; #146).

Yep, it was a huge list of mistakes and missed opportunities, but I'm sure it's possible to fix everything. We have to ways:

Refactor it step by step, discussing every next movement.
Write it from scratch, controlling every next step.

Usually I prefer second option, but when I started I lost my passion and couldn't find time to complete work. It's hard to build things using only two hands and one head ¯_(ツ)_/¯. So I think that we should choose the right way using discussion. And the main problem right now is to find people who want to build frontend part of Remark.

I'm calling for @Guria, @Reeywhaar, @alexelev, @DmitryTsepelev, @lexich, @jackburg, @thepocp, @Mavrin, @Clearic, @life777, @Arelav, @PavelLoparev, @ur300, @steppefox, @biggieman and @slawiko. Is there a chance that anyone of you wants to help us with rewriting and refactoring? I see that each of you fixed frontend bugs or implemented some new features. And I think that it would be possible to find a task for any level of developers.

Now I have skeleton of new frontend part (as I said, I prefer option 2—rewrite anything; but you should vote for the first one, it's OK): https://github.com/umputun/remark/tree/improvement/frontend-refactoring/frontend

What I have there:

Some sort of docs that I wrote during “developing” (https://github.com/umputun/remark/tree/improvement/frontend-refactoring/frontend/docs). It would be nice to document new features and building things to make it possible to understand them tomorrow.
Linters, pre-commit hooks (https://github.com/umputun/remark/blob/improvement/frontend-refactoring/frontend/package.json). I don't like Prettier, so it isn't there, but we can discuss it.
SCSS (https://github.com/umputun/remark/blob/improvement/frontend-refactoring/frontend/config/webpack.config.js#L76-L91). Now it's possible to use dart-sass (https://github.com/sass/dart-sass) implementation, which builds faster on CI, so I don't see any reason not to use it. I think that current PostCSS configuration isn't enough clear for new contributers, but plain SCSS is.
Mocks (https://github.com/umputun/remark/tree/improvement/frontend-refactoring/frontend/src/mocks). There just several files, but it works and it's easy to extend them by creating new ones.
Redux (https://github.com/umputun/remark/blob/improvement/frontend-refactoring/frontend/src/app/store/actions.js). Actually, it is redux-zero (https://github.com/redux-zero/redux-zero/), but I'm not sure that it's a big difference between original redux and zero-version.
Separated demo page, separated widgets, separated app. Because it's better to have everything separated than to have mess with files :-)

But I don't have anything else, because I don't have enough time. I think, that I can create a roadmap, but I need someone to help me with implementing things. I think, that I can create small tasks and review them, but I don't have enough time to complete them.

So. Is it possible that any of you, guys, can help me with it? And what do you think about everything described above?

Heya!

While I'm agree with you, I think that refactoring can be made more easily.

remove custom store, that's for sure
a bit of JSDoc
guideline. Someone likes to make this.somemethod = this.somethod.bind(this), someone just uses onclick={e => this.somemethod(e)}. This rules should be documented.
components decomposition (comment.jsx needs it badly)
typescript? ( ͡° ͜ʖ ͡°) It really helps to keep consistency. the messy part is to make scaffolding with preact, jest, typescript and webpack. You can check out https://github.com/radio-t/rtnews-ui. There is a problem with preact and jest, they don't work as good as with react though. Adding ts increases the problem. Tooling lacks a bit.
what's wrong with prettier? I'm just used to it (just like to gofmt, rustfmt), and the formatting problem gone, at least it not StandardJS (・`ω´・), it has as little settings as possible (so there is nothing to discuss), it's output always idempotent (looking at jslint), it has good IDE support and has big number of supporters, and it's actually a small package.
as for docker my point is totally opposite. I'd rather put webpack-dev-server straight into docker, so only thing you will need to start developing is to type docker-compose -f frontend.yml up. Right now I'm constantly struggling that after pull I have to rebuild docker image, and also reinstall npm deps just to be sure. Notice that npm deps downloaded twice this way. I don't like idea of somewhat proxying, it's very confusing for a novice. And still, developer required to have docker to build backend.
iframe-less

What options we are actually have? I can't think of any. In terms of ui isolation there are webcomponents, but there are lot of old devices that don't support it. In terms of keeping privacy there is nothing. Iframe is a good way to keep users private data from been intercepted. Main difference between react components that you can find on npm, is that remark is embedded solution which manages user authentication and things on its own. Iframe-less solution will either: (a) will require user to log in on each website that have same remark api host. (b) remark host will have to have setting that tells that it allows embedding on site-a, site-b, site-c and still I'm as user will be concerned as now any tracker installed either on purpose or accidentally will have access to remark's DOM and can read if I'm logged in, what I have typed in input box etc. Iframe is black box for it.
as for npm package i'm highly doubt it's worth it, because remarks ui version tightly coupled to backend's version, and I don't think you want problems like "npm's version 1.22.133 doesn't work with commit #ab787ba". Better to keep calling it from host, I think.

Regarding refactoring process, I think it will be good if you just make scaffolding like:

root todo.txt which have list of things to be done, and basic strategy from your point of view, like create /src/store/store.js with get(value: string) -> string function or something.
if there must be file somewhere, then you put src/webcomponent/comment.jsx and put // TODO:. Or even make basic implementation of class which has every function that is just throw new Error("unimplemented");

Right now it's hard to see big picture of what you have in mind, just upon looking on code.

So, from my perspective it's not a shift of paradigm, just needs a little combing and unifying.

a bit of JSDoc

It's a strange thing, but OK.

guideline. Someone likes to make this.somemethod = this.somethod.bind(this), someone just uses onclick={e => this.somemethod(e)}. This rules should be documented.

I can't imagine it, but let's try.

typescript? ( ͡° ͜ʖ ͡°) It really helps to keep consistency. the messy part is to make scaffolding with preact, jest, typescript and webpack. You can check out https://github.com/radio-t/rtnews-ui.

Can you give me an example why TS is better than JS? I don't understand this 'consistency' point. What's the problem with JS's 'consistency'?

what's wrong with prettier? I'm just used to it (just like to gofmt, rustfmt), and the formatting problem gone

What is the formatting problem? Usually I don't have any problems with formatting. I set up ESLint precommit with fixing errors and it works fine.

Prettier changes code in unexpected way. What if I don't want to split some lines?

as for docker my point is totally opposite. I'd rather put webpack-dev-server straight into docker, so only thing you will need to start developing is to type docker-compose -f frontend.yml up. Right now I'm constantly struggling that after pull I have to rebuild docker image, and also reinstall npm deps just to be sure. Notice that npm deps downloaded twice this way. I don't like idea of somewhat proxying, it's very confusing for a novice. And still, developer required to have docker to build backend.

I'm pretty sure that there was at least one developer who changed both parts: frontend and backend. So usually frontend-guys don't need to build backend and install Docker at all. That's the reason of using mocks. As a frontend developer I don't want to set up these DevOps tools and try to understand how they work together.

Also, how do you change responses from server now? What if you need to check UI reaction on some error response got from backend? Do you need to go inside the backend container and do some things with DB? In case of using mocks you just need to edit one JS file. I.e. when I want to change some config keys I edit https://github.com/umputun/remark/blob/improvement/frontend-refactoring/frontend/src/mocks/entities/config.js, when I want to change userinfo I edit https://github.com/umputun/remark/blob/improvement/frontend-refactoring/frontend/src/mocks/entities/user.js, etc.

How does it work with Docker?

iframe-less Main difference between react components that you can find on npm, is that remark is embedded solution which manages user authentication and things on its own.

No. That was the reason why I chose the wrong way on the first stages of development.

Remark today is not a embedded solution. You can't inject it like Disqus, because you need to have your own backend and to understand how these tools work. It's impossible to use it without this knowledge.

Remark today is a product that you need to host and build by yourself, that consists of two parts: backend with Go and frontend with JS. And when you need to inject Remark's “widget” you actually need to (and want to) use it as a regular npm component for React. But we have this iframe-wrapper and it just makes everything messy.

remark host will have to have setting that tells that it allows embedding on site-a, site-b, site-c

And it's fine. I don't remember properly, but we had some issues where users weren't against changing their CORS policies because it makes everything easier and that's how web works today.

I'm as user will be concerned as now any tracker installed either on purpose or accidentally will have access to remark's DOM and can read if I'm logged in, what I have typed in input box etc. Iframe is black box for it.

Oh, c'mon. These trackers are everywhere today and solve this problem in that way is pretty strange. It's like deciding not to go outside because there is a possibility that you got a brick on your head.

Also, I'm not sure that this is Remark's problem. I don't see Remark like “injectable external thing”, but like “another one component that some guys made”. In this case developers must operate with Remark's widget just like with any other React component.

And also today we have lots of problem because of iframe basement. From duplicated code to lack of growth possibility.

as for npm package i'm highly doubt it's worth it, because remarks ui version tightly coupled to backend's version, and I don't think you want problems like "npm's version 1.22.133 doesn't work with commit #ab787ba". Better to keep calling it from host, I think.

But why? We have releases now. And @umputun tries to ship them properly. So I think it's easy to version npm package in the same way as backend.

I think that rewriting isn’t right way. Maybe better to apply improvements step by step according you plan. It’s easy to coordinating and remark hasn’t so big untouchable frontend codebase.

We need only approving rfc.

guideline. Someone likes to make this.somemethod = this.somethod.bind(this), someone just uses onclick={e => this.somemethod(e)}. This rules should be documented.

I can't imagine it, but let's try.

Haven't understood

Can you give me an example why TS is better than JS? I don't understand this 'consistency' point. What's the problem with JS's 'consistency'?

TS helps a lot in environments with many collaborators. It allows to define interfaces like what server response is in actual code that can be analyzed, what props/state does component have, so there is less fear to remove some variable you think is unused but may be used somewhere in remote component, Also it reduces time for some new collaborator to rediscover each component's structure, with js I constantly must to find some variables example usage to understand what it is actually used for. So many time lost by people trying to understand "is this this.props.notification is string, object or what?". It also helps to define some interface and let other collaborator to make it's implementation. Again, unlike Rust, TS type system can be anytime weakened to any type for fast prototyping, and then hardened upon finishing. It works great, I mean great, in VSCode. Not to discuss pros of static analysis here further.

What is the formatting problem? Usually I don't have any problems with formatting. I set up ESLint precommit with fixing errors and it works fine.

If you have set up eslint with idempotent automated formatting there is no problem.

Prettier changes code in unexpected way. What if I don't want to split some lines?

you either just bear with it, because it could be messy but consistent (and so other collaborator will write same way), and by time you just get used to it, or you may apply // prettier-ignore which can be useful if you work with some-matrix like array for example. Again, code-style is flame like topic, not to discuss here.

Also, how do you change responses from server now? ... Do you need to go inside the backend container and do some things with DB? In case of using mocks you just need to edit one JS file. ... How does it work with Docker?

Same goes with docker. You just map /web/src directory into docker's container and change same files you described, nothing changes here in terms of mocking. Docker just taking care of building and ensures that everyone have one node/npm version, and that built version will be same across everyone. Also it's more secure, as I am personally don't like that any package in endless node_modules can make arbitrary code execution while installing.

about iframe

I get your point and just can't agree. It changes remark behavior so much it can be just forked under different name, or line that Remark is Privacy-focused must be removed, because it just passes the liability of storing user's private data to the ones who will be installing it. It is also completely different from GDPR point of view.

Also I haven't tested, I'm not sure how it will comply with anti-csrf, because parent window will have access to user's controls and will be able to forge request from the name of user. This imply that backend will have to have advanced settings for cors headers.

Oh, c'mon. These trackers are everywhere

And it's a tragedy actually! I think that if something can be done to not just pass user's data to these spies should be done. Remark is isle of sanity across this wild web.

And also, I can't see what restrictions does iframe apply:

We will reduce lags and freezes that are caused by message-passing between parent and child windows.

Haven't noticed any

We will cut off some useless code, that was duplicated because we have some “external” parts of the main widget, such as user-info widget.

user-info uses same comment.jsx component, what duplication here?

We will automatically implement styles customization (#5), because it will be possible just to override styles on the top level. And they will be able to inject Remark in more gentle way (#184, #216).

style customization can be made by introducing style: string | URL property into remark's initialization, like:

  var remark_config = {
    site_id: 'YOUR_SITE_ID',
    ...
    style: "https://somehost.com/remark.css"
  };

We will make possible user extensions. For example, it will be easy to pass some function-like parameters to change a renderer, which will help us with things like #31 or #32. Or to localize it by passing object with strings or connecting sort of middleware (#10).

I doubt anyone will actually will use it. Like what extensions? Extensions system is always hard, and after it's done there's no difference is it iframe-based or not. Again it can be made by passing script: string | URL into remark's config. Or more than that it can be injected at build stage, if remark shipped via docker

We will fix major part of problems with auto-scrolling, anchoring and things like #82, because widget will have access to the main window object and things like listening location changes, etc.

Remark handles it pretty well right now as for me, anchoring works, and if someone told me that thing like this is possible before, I would be rather skeptical. Btw: I see that #82 is resolved by @jackburg ?

We will make it work with things like Grammarly (#138).

It doesn't works with grammarly because of (a) browser extension's system is itself rather strict in terms of privacy (b) Remark cares about privacy too (c) grammarly gathers data you have typed and use it for it's own purposes. I think it's problem of Grammarly that it doesn't work in iframe.

We will fix major part of the problems that caused by CORS and things like these (#218), because it will become a “native” part of the website, not “injected from some sources”.

and introduce a ton of new. Actually one way to solve CORS issues is via server proxying, so from remarks point of view you it will be talking to same host, but cookies still be missing.

We will make it easier to implement things like DOM watching, because we will use Preact / React and all related things in the context of the main window, not though an iframe.

In the #221 I already showed that it can be made right now. The question is api: whether to watch automatically via MutationObserver (which can be costly), or pass init/deinit into hands of the parent window owner.

We will be able to implement things like pushes (using Push API) or other similar notifications (using Web Notification API). It looks like it's an another one interesting approach to implement notifications (#265). I'm not sure that it's possible to do now, because I think that we must have direct access to the main window to implement things like these (but I didn't check actually).

But we have access to parent window! It just parent doesn't have access to iframe's content. All that can be done by message passing. WebNotifications even doesn't have to require access to parent window.

Whew. Lot of talking here.

@igoradamenko & @Reeywhaar - I think the conceptual difference in iframe vs no-iframe approach is the most important thing here. To be honest I was the one asked the question "why the hell we even bother with iframes" in multiple conversations with @igoradamenko. However, reading the proposal and discussion I think - maybe it was not such a good idea. Currently, remark in an iframe is easy to add to most of the sites and works ok and reasonably safe to use. It doesn't require much trust from the site "embedding" our widgets, which is also a good thing.

We have customization/localization/styling issues, but hopefully, we could resolve it somehow even in iframe. For example, we can make backend to apply some templating with the user-defined configuration on top of generic css and move all this customization away from UI. Maybe we can found other ways, not sure. But this problem seems to be resolvable.

We may have some other issues with widgets not working well with some sites, and I have no clue if this issue even resolvable. I guess if it is not - so be it. The same for Grammarly.

Generally, I have a feeling - restrictions enforced by iframe can be a pain, but embedding remark directly feels like a limited solution completely killing the potential use of externally hosted, untrusted remark server(s) in SAAS/cloud model. It even questionable in case of the shared private remark backend serving multiple related sites.

Maybe there is a middle-ground of some kind? Like making all UI components (or whatever it called) directly embeddable if one need/want such a deep integration, but keeping them wrapped in widget-level (iframe) integration as the primary option.

UPD: pls note - I have very little understanding about UI side and may propose complete BS :)

@Reeywhaar,

guideline. Someone likes to make this.somemethod = this.somethod.bind(this), someone just uses onclick={e => this.somemethod(e)}. This rules should be documented.

I can't imagine it, but let's try.

Haven't understood

Nevermind. Let's try.

TS helps a lot

For me it looks like it helps people who don't use things like React's propTypes, defaultProps; and who don't use any methodologies like BEM. And who just want to have a hummer for any kind of nails.

But if you think it's good to use it, OK.

If you have set up eslint with idempotent automated formatting

OK, let's leave prettier and set it up for JS/TS files too.

Docker just taking care of building and ensures that everyone have one node/npm version, and that built version will be same across everyone. Also it's more secure, as I am personally don't like that any package in endless node_modules can make arbitrary code execution while installing.

I think it's a paranoidal thing, and it's a excess barrier for anyone who wants to contribute something and for some reason don't have Docker (because usually nobody except backenders don't have Docker installed).

But OK, let's move everything into Docker.

It changes remark behavior so much it can be just forked under different name

Everything can be forked under different name.

Remark is Privacy-focused must be removed, because it just passes the liability of storing user's private data to the ones who will be installing it. It is also completely different from GDPR point of view.

As a developer to make everything work you need to setup server for comments, and then setup frontend for this server. What's the problem with privacy and GDPR and anything else? Remark is not Disqus or any else service that stores your website's data somewhere. As a developer you store your website's data by your own, so isn't it your problem if you have some lack of security or other things?

As I see Remark right now, it's just a “part of codebase”. It's like you have a IDK, let's say news website. And you have comments there. Your own code that runs everything, stores everything, etc. Then you decide to split things and you move backend of your comments to a docker container, and separate comments to a big frontend component. And, voila, you have your own Remark-like project—backend in Docker + frontend as a big component. Why for God's sake will you move it to iframe if you can control everything around your own code?

And it's a tragedy actually! I think that if something can be done to not just pass user's data to these spies should be done. Remark is isle of sanity across this wild web.

If user doesn't want to let data flows away, maybe he just must no to install trackers? Why should it be Remark's problem, not user's?

We will reduce lags and freezes that are caused by message-passing between parent and child windows.

Haven't noticed any

They are everywhere. If you open, let's say, https://radio-t.com/p/2019/01/26/podcast-634/ in Chrome mobile simulator and turn on slowing down CPU for 6x times, you will see height of iframe changing. And there are only 30 comments. I see lags and freezes on my MBP'15 on https://remark42.com/demo/ without any simulation and slowing down. When I open this page on my mobile and click on “Show more comments” button I see these lags too.

style customization can be made by introducing style: string | URL property into remark's initialization, like

Add external files instead of building everything into one bundle?

@umputun,

Maybe there is a middle-ground of some kind? Like making all UI components (or whatever it called) directly embeddable if one need/want such a deep integration, but keeping them wrapped in widget-level (iframe) integration as the primary option.

I think that's similar to thing that we have now.

restrictions enforced by iframe can be a pain, but embedding remark directly feels like a limited solution completely killing the potential use of externally hosted, untrusted remark server(s) in SAAS/cloud model

Okay, it looks like a major point here.

Let's sum things up.

~1. Keep the iframe-based structure.~

Configure Prettier to use it for JS files.
Rewrite to TS.
Remove the custom store.
Add some JSDoc.
Add docs with sort of description of best practices.
Move frontend env into Docker.

...and find someone who wants to do anything from the list above.

Hi! I would like to participate the frontend part development, but I'm little bit confused if it is a good time to fix other issues cause everything is going to be refactored (or even re-written) soon. Have you considered JS Flow instead of the TS if we will not re-write from scratch (and if we will)? It can be easily applied to the existing code file-by-file.

@AnyRoad, one thing that I can say now is that we're not going to rewrite anything.

I'm not sure that I can answer about Flow vs TS, but I think @Reeywhaar can.

Sorry guys, absolutely no time for thinking about it right now. Hope, eventually I'll get some. I think it'll be great to have a spare week to work at night. As for flow vs ts, my preference to ts, it has a lot better vscode support and better documentation and compilation time, but it's just my point of view maybe. And as for applying, ts can be to applied on top of existing codebase loosely. Instead of making ts project, codebase can be even built via webpack, where we have different loaders for js, and ts.

Just in case: I'm working on refactored version. Ported to typescript, and removed store for now. There is a bit of work required (port tests), so not pushing yet.

umputun / remark42

Frontend refactoring #267

It works inside an iframe

It uses a custom store

It has a bad structure

iframe-less

about iframe