go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
45.26k stars 5.51k forks source link

Frontend discussion: jQuery & Fomantic-UI #18302

Closed wxiaoguang closed 2 years ago

wxiaoguang commented 2 years ago

There are many discussions about the frontend refactorings, this issue is to record the details, and avoid bikeshedding in future.

Background

Gitea is using DOM Native, jQuery and Vue2 together, the UI part is heavily using Fomantic-UI.

Discussion Topics

Should Gitea use a new UI framework?

Some maintainers mentioned Tailwind CSS before.

Gitea frontend now uses Fomantic-UI components heavily, ex: Dropdown, Modal, Checkbox, etc.

Currently there is no conclusion or detailed plan about how to migrate the UI to a new framework.

Should Gitea drop jQuery?

The mentioned Cons of jQuery are:

jQuery Performance

Question: jQuery was proven to be much slower than native (sometimes as much as 10x), just look at their code which is full of hacks to support weird edge cases and legacy browsers. While the performance difference will probably not matter in most cases, I still think it's unnecessary baggage we can avoid by using the underlying APIs directly.

Discussion: In most time it is not that slow and it doesn't affect user experience. JS is slower than C but it doesn't mean that we should write everything in C.

jQuery Popularity

Question: Modern web developers don't even learn this API any more (SPA frameworks strongly discourage jQuery), so the contributor base that knows jQuery will shrink over time.

Discussion: The current situation is: Gitea has a lot of legacy MVC code and uses many jQuery features: event system, ajax, filter and chained-call $().parent().foo().bar(). If we drop jQuery, we would rewrite them all and introduce a house-made jQuery-like framework. if we replace jQuery with a home-made framework, it would be much less popular than jQuery.

Potential Problems for Refacotring

New PRs using jQuery during migration

Question: If we start the migration, a new PR uses jQuery, what should we do? Force the contributor to rewrite, or we rewrite for them, or just accept it?

Discussion: It's not friendly to force contributors to rewrite without jQuery. If we accept the new PRs with jQuery then new code comes and the percentage of jQuery increases. No conclusion yet.

Continuity

If we decide to start the refactoring, we should guarantee it can succeed. If we stop half-way, then more and more frameworks would be mixed together, it would be a big disaster. That's why we should think carefully and have a clear plan before we continue.

End-to-end tests

by singuliere: The primary problem with refactoring the UI is the lack of end to end testing (e.g. running a browser and have a test click on the web page, using a selenium driver). Not that it will make it impossible. Just significantly more difficult and prone to regressions. It would make sense to start with adding a reasonable amount of end to end testing so that the result of a human interaction with the UI can conveniently be verified to not change after the framework is replaced.

by wxiaoguang: If we do framework level migration and refactoring, then the pages should be rewritten, then all old tests become invalid and should be rewritten too. So the tests had better to be written in a long-term & stable framework.

Next Step

If we choose to drop jQuery, before we make a final decision, we should document all detailed steps about:

  1. How to deal with Fomantic-UI and its components? If we still use Fomantic-UI heavily, jQuery is a must.
  2. How will the home-made library for jQuery replacement look like?
  3. How can we guarantee the refactoring succeed? We can not stop half-way, otherwise there would be a big disaster.

If we choose to continue using jQuery and Fomantic-UI, we can make some new guidelines.

singuliere commented 2 years ago

The primary problem with refactoring the UI is the lack of end to end testing (e.g. running a browser and have a test click on the web page, using a selenium driver). Not that it will make it impossible. Just significantly more difficult and prone to regressions. It would make sense to start with adding a reasonable amount of end to end testing so that the result of a human interaction with the UI can conveniently be verified to not change after the framework is replaced.

wxiaoguang commented 2 years ago

The primary problem with refactoring the UI is the lack of end to end testing (e.g. running a browser and have a test click on the web page, using a selenium driver). Not that it will make it impossible. Just significantly more difficult and prone to regressions. It would make sense to start with adding a reasonable amount of end to end testing so that the result of a human interaction with the UI can conveniently be verified to not change after the framework is replaced.

If we do framework level migration and refactoring, then the pages should be rewritten, then all old tests become invalid and should be rewritten too. So the tests had better to be written in a long-term & stable framework.

kdumontnu commented 2 years ago

The primary problem with refactoring the UI is the lack of end to end testing

I completely agree. I don’t have a strong opinion for what front-end test tool to use, but I checked out some options last year. BackstopJS and Cypress seemed like the two best contenders. Backstop is cool, but the problem is where to save the screenshot data without blowing up our repo size (or adding LFS dependency).

I think Cypress is the best contender. They have a free open source offering, you can test across different browsers, and there is a ton of support and integrations. The downside is that its a bespoke testbed language.

I don’t have a ton of time to develop this near term, but I would be happy to help get things set up and help sponsor development.

techknowlogick commented 2 years ago

While I am still reading through the above, I'd also like to explicitly state accessibility as part of the acceptance criteria.

silverwind commented 2 years ago

Regarding jQuery, we can use eslint-plugin-jquery to gradually migrate off of it. Each time a new feature is refactored, a lint rule can be enabled to forbid it. For example, refactor .ajax to fetch and then enable jquery/no-ajax.

kolaente commented 2 years ago

My two cents:

tboerger commented 2 years ago

My two cents:

silverwind commented 2 years ago

If we drop jQuery, we would rewrite them all and introduce a house-made jQuery-like framework. if we replace jQuery with a home-made framework, it would be much less popular than jQuery.

No library/frameworks/wrappers of such kinds please. Modern DOM APIs are potent enough to be used without wrappers. Cases like fetch could be wrapped for the purpose of authentication/CSRF token, but generally we should just use them as they are.

wxiaoguang commented 2 years ago

If we drop jQuery, we would rewrite them all and introduce a house-made jQuery-like framework. if we replace jQuery with a home-made framework, it would be much less popular than jQuery.

No library/frameworks/wrappers of such kinds please. Modern DOM APIs are potent enough to be used without wrappers. Cases like fetch could be wrapped for the purpose of authentication/CSRF token, but generally we should just use them as they are.

I was not talking about ajax. I mean, what about these chained-calls $().parent().foo().bar()? What's the best method to rewrite these code without jQuery while keeping the code clear and correct?

lafriks commented 2 years ago

If we would rewrite dynamic parts to Vue we would not need such constructs

wxiaoguang commented 2 years ago

If we would rewrite dynamic parts to Vue we would not need such constructs

I agree new frameworks won't need such problems.

The reality is, Gitea uses a lot jQuery and Fomantic-UI. Everyone knows "what to do" is better, but it doesn't help now. I still want to mention that recently some new code were written in Vue but it only makes the index.js more messy.

If we can clarify "how to do" ahead and make everyone work together, we could achieve the goal.

A question is: do most of us agree to only write Vue code in future and never touch jQuery and Fomantic-UI any more? If yes, we can write this agreement down and discuss how to start rewriting old code.

Matt-Deacalion commented 2 years ago

For end to end testing, Playwright and TestCafe are worth adding as options. Playwright may be able to write front end tests in Go.

miku86 commented 2 years ago

Does anyone have some data?

E.g.

wxiaoguang commented 2 years ago

How many pages use these old tools?

Almost all.

How many pages would we have to rewrite?

Almost all if we want to drop them.

lunny commented 2 years ago

Let me have some summary. Let's have some choices and votes.

~About end-to-end test tool~

All of us agree we need end-to-end test. The vote is pausing. 💯 cypress 🥈 selenium 🥉 Playwright 🎱 TestCafe

~About front-end refactor~

The vote is pausing.

🔆 Keep the current state 👍 Move all to Vuejs + TaliWind step by step. (which one Vue3/Vue2/?) And only accept new PRs which write with this tools. 🎉 Just remove jQuery and use Fetch

wxiaoguang commented 2 years ago

It's a little early to vote in my mind.

If we want to continue without keeping current state, the first thing we can do is: rewrite the view/diff page totally in Vue (or new frameworks). The view/diff page is the most complex, if we can succeed on it, then we have enough confidence to go ahead.

kdumontnu commented 2 years ago

It's a little early to vote in my mind.

If we want to continue without keeping current state, the first thing we can do is: rewrite the view/diff page totally in Vue (or new frameworks). The view/diff page is the most complex, if we can succeed on it, then we have enough confidence to go ahead.

Don't you think we need to implement the testing framework before any major refactoring?

I think we’re combining two issues here (jQuery + visual testing) and we should split them up.

@lunny can you hold off on the vote for visual testing framework? I’d like to create a separate Issue explaining the goals and all of the options. I can do that tomorrow.

wxiaoguang commented 2 years ago

Don't you think we need to implement the testing framework before any major refactoring?

Agree to implement a testing framework (maybe not before, but at the same time).

Testing framework is not a blocking choice in my mind, most frameworks should work (And Yes, we should choose it carefully! Looking forward to your issue for testing fraemworks).

I do not think we need to write tests for the old pages which are going to be totally rewritten, we could just write the tests for newly written pages. Quote my comment before: If we do framework level migration and refactoring, then the pages should be rewritten, then all old tests become invalid and should be rewritten too. So the tests had better to be written in a long-term & stable framework.

So I prefer to rewrite to new pages and implement testing framework at the same time.

lunny commented 2 years ago

We could add test framework and some tests for the Vue pages or components at first. So that a new coming PR could reuse the testing framework and make CI happy.

silverwind commented 2 years ago

This issue discusses like three different issues (jQuery, testing, Vue), it's getting hard to focus.

Regarding the test runner, I think we should keep jest and then execute playwright (or whatever other framework) within jest test cases.

I think a primary concern is speed. No one likes tests that run for minutes. They must be fast which will encourage developers to write more tests.

tboerger commented 2 years ago

I got good expressions from cypress from the past. It makes sense to add some frontend tests BEFORE rewriting the stuff to see any sideeffect on refactoring. The failing tests can be fixed as part of the PR for the rewrite.

oilipheist commented 2 years ago

I think that qwik and mitosis would be worth investigating. Qwik has a tiny footprint compared with anything else I have seen and mitosis is pretty darn flexible.

wxiaoguang commented 2 years ago

I would close this issue since there is no agreement, no progress and no plan, it's impossible to continue.

oilipheist commented 2 years ago

Fair enough but Mitosis is designed specifically to solve that particular problem. Qwik is designed to do it fast with minimal javascript overhead on the client side(1kb). Instead of getting stuck choosing the right™ framework you could write once and have flexibility to switch from Vue, React, Angular or regular old HTML at a whim later vs choosing one of those and then potentially having to rewrite things again from scratch in the future.

silverwind commented 2 years ago

There will be drawbacks with such additional abstractions, either size, performance or flexibility, thought. I think it's more suitable for library use cases.

lafriks commented 2 years ago

I don't see reason for us to switch frameworks on regular base so I don't think there is need for that... We have long ago agreed to use vue and are already partially using it

wxiaoguang commented 2 years ago

I don't see reason for us to switch frameworks on regular base so I don't think there is need for that... We have long ago agreed to use vue and are already partially using it

So, can we confirm that we stay with Fomantic UI/jQuery and Vue and write it into guidelines? I didn't see the necessary to switch frameworks either, but there were so many discussions before (during frontend refactoring, in the discord channel, dropping jQuery, etc), that's why the issue was created.

A real question is: since we are using Fomantic UI which needs jQuery, should the new code be written in jQuery, or in Native? I believe there is no agreement yet.

And here we have a real case: https://github.com/go-gitea/gitea/pull/18002/commits/ae5b39e9e82c68ed09c61e86eaca4b2cafc6a320 , the go-tmpl UI uses Fomantic Checkbox and Popup, without the FomanticUI+jQuery, it's difficult (impossible) to make them work correctly. As long as there are Fomantic UI code, we can not drop jQuery.

wxiaoguang commented 2 years ago

For the questions in the issue:

image

My answer is:

Does everyone agree?

jtran commented 2 years ago

From what I understand, we were postponing the decision of a frontend refactor until the testing issue #18346 was settled. (There's a draft PR for it #18442.) What this says to me is that this issue isn't resolved; it's blocked.

So, yes. Status quo, as far as I understand. No change in approach until further notice.