-
-
Notifications
You must be signed in to change notification settings - Fork 387
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
Frontend refactoring #267
Comments
Heya! While I'm agree with you, I think that refactoring can be made more easily.
Regarding refactoring process, I think it will be good if you just make scaffolding like:
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. |
It's a strange thing, but OK.
I can't imagine it, but let's try.
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 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?
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?
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.
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.
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.
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. |
Haven't understood
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
If you have set up eslint with idempotent automated formatting there is no problem.
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
Same goes with docker. You just map about iframeI 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 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.
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:
Haven't noticed any
user-info uses same comment.jsx component, what duplication here?
style customization can be made by introducing
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
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 ?
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.
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.
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.
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 :) |
Nevermind. Let's try.
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.
OK, let's leave prettier and set it up for JS/TS files too.
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.
Everything can be forked under different name.
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?
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?
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.
Add external files instead of building everything into one bundle?
I think that's similar to thing that we have now.
Okay, it looks like a major point here. Let's sum things up.
...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. |
@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. |
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 (Implement and document 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 (User comment history sidebar doesn't work with fixed header websites #184, Add z-order to remark overlay #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 Allow LaTeX style formulae #31 or Add github style syntax highlighting. #32. Or to localize it by passing object with strings or connecting sort of middleware (UI localization support #10).
We will fix major part of problems with auto-scrolling, anchoring and things like Mobile navigation to comment doesn't work for hidden comments #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 (Comment area and Grammarly #138).
We will fix major part of the problems that caused by CORS and things like these (The operation is insecure #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 (email 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 (Publish client library to npm #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 (Add tests for frontend #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 (Add mocks to frontend #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 (Improve frontend components structure #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?; Split vendor dependencies into separate bundle #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?
The text was updated successfully, but these errors were encountered: