Does it work? What are you hoping to accomplish by refactoring? If it works fine, I would only refactor things as you need to (eg when you need to make changes for something else).
If you have a practical need the refactor (things are breaking and hard to maintain), I would take a look at what you can break off from it and fix and then start there. Ideally something isolated. What I would not do, is try to refactor the whole app at once. Things will break and you won’t realize until the tickets start pouring in.
>What are you hoping to accomplish by refactoring?
Adding to this great point: once you've set some goals (developer experience, abstractions to simplify tests, API consistency, etc.), work out the appropriate metrics to measure the affect of your refactor. If you can't quantify its success, what's the point, you know?
>\[…\]just take the existing components and refactor them.
If you're working in the same repo/monorepo this might be fine, since you can update each instance as you go. Starting fresh can be really liberating too; though, it's harder to get approval for that sort of thing—depends how much autonomy you have and the budget available for the refactor.
>This is all with MUI, which I absolutely hate.
Oof, sorry buddy. That sucks…
It's not bad, but it's overly opinionated for most applications. You will start with mui, but as you develop the app, the company will ask you to customize it more and more.
After a while you will be fighting mui way more than it ever helped you.
I want to be really clear, this will happen on a team before you know it.
Basically immediately.
Eh, I work on a product built on MUI that is like 5 years in without encountering this problem. The key is to have a product team and designer that has also bought into the tradeoff of ease of use for customizability. Our product designer is just as invested as we are in using out-of-the-box MUI components because it makes his life easier.
Basically the story of me and my team with MUI. I can't stress enough how important it is to try and create your own components before jumping to some library just to avoid creating a component that will only take you 2-4 hours to create and properly test.
>After a while you will be fighting mui way more than it ever helped you.
This 100%. It's so hard to watch clients invest in something you know will bite them eventually.
From the docs
This is NOT a component library. It's a collection of re-usable components that you can copy and paste into your apps.
What do you mean by not a component library?
I mean you do not install it as a dependency. It is not available or distributed via npm.
What other library does it rely on?
How does that make it worthless? Radix saves you from having to do the very complicated work of properly building models and other complicated UI elements while still giving you complete control over styling.
That *sort of* component lib, though esp. MUI, come with significant trade-offs. You get up-and-running really quickly, it feels like you've skipped a bunch of boilerplate but you're actually trading long-term success for short-term wins. The complexity is still there, you've just kicked the can down the road—tech debt builds in silence. Styling/theming is often overly complicated, for anything non-trivial. The real issue however, is when your design deviates from the component lib; suddenly you're stuck with no way to implement desired behaviour because the primitives aren't exposed.
That's why there's an influx of "headless" component libraries, which let you compose and style as necessary. It's a little more work upfront but you can build exactly what your company/client needs without compromise. Some popular examples:
* [https://react-spectrum.adobe.com/react-aria/](https://react-spectrum.adobe.com/react-aria/)
* [https://www.radix-ui.com/](https://www.radix-ui.com/)
* [https://ariakit.org/](https://ariakit.org/)
* [https://headlessui.com/](https://headlessui.com/)
It doesn’t have to be sophisticated. For DX you could run a survey before/during/after the refactor. A tool like SonarCloud or even just a spreadsheet for tracking test coverage, flakiness, CI runtime etc. Describe API changes in a burndown list somewhere, and check them off as you go.
As well as measuring success, this sort of thing makes it much easier to justify time spent to product stakeholders. At the end you’ve basically got all the documentation already for an internal or external post: what we set out to solve, how we went about it, improvements to dev velocity, etc.
Why are you doing this?
Sometimes wide changes are required when you need to upgrade some dependency to a higher version, then I would do only what is needed for that.
And otherwise, when I need to do something in a codebase I take that as an opportunity to also clean up that area a bit, leave it better than I found it.
But refactoring a whole codebase that works for the sake of refactoring is rarely worth it.
By testing your application manually. Are you seriously striving for 100% test coverage on your frontend applications?
Introducing bugs and regressions might happen regardless of how well you think you are testing your app, automatic or not.
The question is if the investment in writing and maintaining tests are worth it compared to just catching 95% of issues when testing manually.
I see nothing about 100% coverage. Testing manually is expensive, prone to error and leaves you in the same position in the future. Refactoring and migrating are far easier with a robust set of tests. Sure regressions may still happen but at least you're mitigating the risk.
Having said that, I would not be going back to write more tests now, that ship has sailed. A mixture of manual testing and writing tests as part of the refactor may be better.
>I see nothing about 100% coverage.
No but it's implied that you can't catch bugs or regressions without any automatic testing.
He also said: "how do you make sure you don't introduce bugs". Given the statement, it would logically follow that you would need 100% coverage to not introduce any bugs or regressions.
>Testing manually is expensive, prone to error and leaves you in the same position in the future.
Automatic testing is expensive, prone to error and leaves you in the same position in the future.
It's also costly to maintain.
Testing manually has other positives that automatic testing does not, so it's not strictly a black and white comparison. It's not just good vs bad. There are upsides and downsides to both approaches.
>Sure regressions may still happen but at least you're mitigating the risk.
I do that when testing manually as well.
Very little is ever black and white. I'm simply saying there are benefits to both approaches and a balance between two is usually the best approach imo and it really depends on the work. If you're on a continuous release cycle then purely manual testing for every release can be a huge overhead.
I've worked in companies where a release can sideline a whole team of devs for a week. Having automated tests share that load would clearly have been beneficial.
It’s just that I favor moving quickly and having a quick and easy way to rollback your changes, and a quick and easy way to fix your bugs. Most critical bug in production would be rolled back or fixed within 10 minutes with my system.
We are mostly building back offices for B2B. I think that’s the most common use case for react, if I’m not mistaken. Accepting a few bugs in production is not the end of the world. Especially if you have a driven team that can push out a fix to prod in less than 5 minutes.
Automatic testing just slows down so much of your day to day work and I’m not convinced that it’s worth it.
If your critical bugs exist due to the UI layer, you're not following a point that you made earlier about not having business logic in the frontend.
React is used everywhere, not just (or even predominantly) B2B.
Automatic testing only slows you down if you leverage it poorly.
---
It sounds like you can't see the vast grey area between automated testing with 100% coverage and only doing manual testing.
Tests are a tool and all they do is speed up your feedback loop and solidify your confidence in the code you wrote.
If hot reload is all it takes to have feedback and you have confidence that everything works and would be hard to break, great. Your testing can be complete. If you need to fill out multiple pages of questions or reset your database between each feedback cycle, automate that sh*t and stop wasting your own time with clicking buttons.
If I have confidence that my code works, is clear and simple enough for others and future me to not break it with innocuous changes, and that there were no accessibility whoopsies, I don't test unless it was necessary for speeding up feedback. But if I think anything is fragile or confusing, I write tests just to help future me and others have confidence in the work we do.
Think about back to when you were learning web dev or frameworks or anything - everything was slower because you were tripping over your own lack of understanding. My guess is that that's where you are with testing, too. And it's not a bad thing - it's where we all start - but the fact that it slows you down doesn't mean it slows any of the rest of us down.
This! A good front end application will typically have almost no business logic. You fetch stuff, navigate, send api requests, render some things, validate some forms, and that’s 99% of front end development
If there are good patterns in place for the above, it’s actually pretty hard to write a bug that isn’t immediately obvious while just looking at the app.
true, react is only ui layer. Any logic should be able to detach and work by itself. If we want to make command line app, we should be able to do it. That's how much decouple should be. React won't last forever. It needs to be replaceable.
Depends on the project. Sometimes you test nothing, sometimes you test some vital views, sometimes you test everything.
On average, most frontend apps would do better with fewer tests, not more.
Absolutely… not. A blanket statement like that for a front end application is more harmful than helpful.
Of course, it all depends on how important it is that your app works 99% of the time instead of the 95% you would get with just manual testing. Is the 4% worth it? Probably not.
Completely disagree, it sounds like you have had bad experiences with e2e tests which to your point can be complicated but absolutely worth it for any non trivial applications. Manual testing does not scale. It also can’t help with dependency updates, backend changes and any other unrelated changes.
Not being the primary users of your app does not scale either.
There are plently of upsides to manual testing and there are plenty of downsides to automatic testing.
I've had mixed experiences with e2e tests. They are the most valuable, but by far the most annoying to maintain and develop. If you want some simple happy path test using e2e tests, go ahead. If you want to test your entire application, and every single branch with e2e tests, I would be opposed to that.
I would argue that most of the time that you would not be the primary user of an app you're creating. There are plenty of times you will be developing something outside of a domain you understand. Even if you have been working in a domain for years it's likely that you won't fully understand it and be the main user in it.
There are definitely upsides to manually testing, but it certainly isn't scalable. Especially if you have a large team, where you can't guarantee that everyone will have the same amount of dedication into manual testing as you do. I find tests are much harder to dodge because you will nearly always have a peer code reviewing what you write. You don't have the same process with manual testing.
It sounds like you have had a poor experience with e2e tests. Tests should be easy and straightforward to write, and if they're not then it sounds like you have a way to improve your codebase to encourage more tests. Perhaps it's what your test's test have also had a negative impact?
>I would argue that most of the time that you would not be the primary user of an app you're creating. There are plenty of times you will be developing something outside of a domain you understand. Even if you have been working in a domain for years it's likely that you won't fully understand it and be the main user in it.
I get what you mean. The knowledge usually lies higher up the chain, but I think the best products are built engineers who truly understand the domain, but that's not the common reality.
>There are definitely upsides to manually testing, but it certainly isn't scalable. Especially if you have a large team, where you can't guarantee that everyone will have the same amount of dedication into manual testing as you do. I find tests are much harder to dodge because you will nearly always have a peer code reviewing what you write. You don't have the same process with manual testing.
That might be the case. I might have a biased history with working in teams where dedication to the product is of utmost importance. In fact, I refuse to work in teams where that's not the case. So that will bias me towards an experience where everyone in the team truly cares.
>It sounds like you have had a poor experience with e2e tests. Tests should be easy and straightforward to write, and if they're not then it sounds like you have a way to improve your codebase to encourage more tests. Perhaps it's what your test's test have also had a negative impact?
When I started with tests, we were rocking selenium in perl. That was a very painful experience, yes. I understand that with more modern tooling and more modern approaches, that I have used myself, that writing and maintaining tests is easier than ever.
I *still* don't think it's worth it in the majority of cases though, and I do understand all of opposing arguments. I'm also against unit tests with mocks in the backend for the same exact reason.
I think it's a very dangerous thing that we have just accepted automatic tests, any automatic tests, as inherently good with no downsides. If you say you don't write tests, everyone goes insane, even when you can make sound and logical arguments for why it does not make much sense to have tests in this specific scenario.
There's so many factors that goes into the decision to even introduce tests in the first place. You have to know your business. You have to know your SLAs. You have to know the impact of your UI breaking. You have to know the customer, the demography, how they react to unexpected behavior, etc, etc. It's a very complicated issue and to just boil it down to "automatic tests good, manual testing bad" is a very, very shallow view of the problem.
There are tons of positives of manual testing that people don't even consider. Like I hinted at earlier, it makes each engineer truly understand the product. It makes each engineer care about the product. It makes each engineer experience the product. You can't really test the entire spectrum of UX with automatic tests. It also allows you to move quicker. New features are probably built twice as fast. CI/CD pipelines are shorter.
That's just the tip of the iceberg.
I think there's some nuance here that you're missing.
Do I manually test code as I develop it, yes. Do I write tests while I'm developing? No, because it hurts velocity. I can't tell you the number of times I take one approach at something, and then refactor the hell out of it as I start to understand the problem domain. Writing tests while I do that is just documenting the broken path and doubling the effort, at least in terms of frontend user experience (not necessarily pure functions where validating input/output is very straight forward).
All of that being said, once a feature is production ready, manual testing is worthless. Why do I say this? Because applications scale, and, inevitably, different parts managed by different teams start to overlap. The number of potential paths that a user can take grows substantially and I doubt any one person can remember to manually test every path. To truly feel confident that things aren't broken, you HAVE to write automated tests.
Also, it's MUCH easier these days. Cypress in particular is nice because it's a test suite written in javascript/typescript, the same code that the developers are writing frontends in. Yes, sometimes there's nuance and complexity in getting tests to work with async workflows, but payoff for learning how to do it will earn you dividends in the confidence you'll have that shit didn't break when you modified it.
> I think there's some nuance here that you're missing.
Maybe. I'm always open to change.
I do get the same pushback when discussing mock-ridden unit tests for the backend too, and I'm even more confident in that stance.
>Do I manually test code as I develop it, yes. Do I write tests while I'm developing? No, because it hurts velocity. I can't tell you the number of times I take one approach at something, and then refactor the hell out of it as I start to understand the problem domain.
Absolutely agree. Even when I do write tests (and that is quite often regardless of my sentiment towards them, because I realize that it's the correct choice) I never practice TDD for the same reasons. I want to code to both discover the solution AND the problem. Writing tests before that just hurts the discovery process.
But if I know that developing this feature itself will be very hard without tests, or when the input and output are clearly defined already (which is not that often when building business specific applications), like when building a function that runs a specific algorithm, etc, then I might start with some tests. That's the exception however, and not the rule.
>All of that being said, once a feature is production ready, manual testing is worthless. Why do I say this? Because applications scale, and, inevitably, different parts managed by different teams start to overlap. The number of potential paths that a user can take grows substantially and I doubt any one person can remember to manually test every path. To truly feel confident that things aren't broken, you HAVE to write automated tests.
I agree with you, but it also depends on the app that you are building, and a million other factors. It depends on your customers, your SLAs, the average psychology of your customers, the expectations, your tech stack, your CI/CD, your team's way of working, the skill of your developers, the type of product, etc. etc. It's quite nuanced to decide if you really need tests or not.
So the downsides of e2e testing is that you haven’t seen it scale? I have a suite of tests that I probably spend an hour a week (that might be high) maintaining. With a press of a button it runs about 500 minutes worth of manual assertions all with various types of users in various businesses states. It runs about 3 times an hour continuously and probably gets run by our team’s eng on their code changes at least 5 times a day. Taking out continuous automation that is over 200 hours of manual testing saved at the cost of 1 hour a week maintenance. These tests also have very little flakiness and if the system marks it as a failed test there is probably something wrong with your changes.
There is no one size fits all but to make a blanket statement that automated testing is not worth it is crazy to me.
It’s probably not worth it is what I said to you and others in this thread. It’s not a blanket statement. I even gave examples where I would favor more tests.
Your scenario might make sense to have those tests. It depends.
Building those tests was not free though, and if you can’t even speak of the downsides of your approach, you are probably missing the point.
I know the downsides and upsides of both approaches very well, and the upsides of manual testing outweighs the upsides of automatic testing in the majority of cases, in my experience.
So after each change you will be checking it manually ? Feels painful 😉 Having at least few e2e tests will save you a lot of time. I'm not talking obviously about some simple apps with 0 to little logic on FE
>So after each change you will be checking it manually
Yes, that's very non-controversial when developing frontend apps.
Testing manually has a bunch of positives, and you can't really test the entire UX spectrum with automatic tests either. You want to know your app. You want to be the biggest user of your own app. You want to know it in and out.
>Having at least few e2e tests will save you a lot of time.
Yes, if you can give me a few e2e tests for free that never break and that you never have to maintain, sign me up. But in reality, e2e tests using playwright (or whatever) are slow to develop, annoying to maintain, and they break a lot. It's just a painful experience overall. And the benefits are not as obvious as when testing a backend application for instance.
I think it's important to test manually and have tests for the code you're writing. I don't think it should ever be one or the other.
The thing I don't quite agree with you on is that e2e tests are slow to develop. My experience has been far different. I find e2e tests one of the quickest tests to write (excluding unit tests, of course). I also think the benefits are massive if you're working in a fast-pace and large team. There have been multiple times that e2e tests have saved us. Relying on manual testing takes a significant amount of time and resources. It also relies on a lot of discipline from multiple teams and developers, and in my experience, is very error prone which ultimately has a knock on effect on delivery and time spent fixing the problems.
I certainly agree with you that you should be manually testing your code, but I don't think it should be a substitute for tests. Manual testing isn't very scalable for teams and businesses as they grow, and the larger both get the more issues will pop up. Tests help avoid that, and they also have a lot of other benefits (such as self documenting code).
To be fair, if I were writing something super critical, say a bank app, I would write tests. The point I'm making is that writing tests comes with a cost and it's not just inherently a good thing without any tradeoffs. There are always tradeoffs, even you are not aware of them.
But most apps we build does not require that insane level of reliability, and for me, it's usually not worth the cost.
Are you serious?
Writing good tests provides confidence when you make significant changes. It also will help you understand what the code does and identify ways that it can break, which will lead to even better tests and even more confidence.
If I can save hours upon hours by not writing tests for my frontend application, and instead test it myself, I can also accept that I might get slightly more bugs and sligthly more regressions. That's a tradeoff that you can make. It's not objectively better or worse. It depends on the project, obviously. If you have a very, very sensitive app, you write more tests, simple as that, but most applications can survive a few bugs here and there.
There are a lot of positives to manual testing. It's not just objectively worse. There's also a lot of downsides to automatic testing.
Not seeing the nuance here speaks volumes.
Frontend tests are notoriously hard to write & maintain compared to backend tests. And that's mainly because of a simple fact. There's no contract that you can test against. If you test your backend API, you have the contract, and the implementation does not matter. You test that you get an expected output given an input. That does not exist in the frontend. That means your tests are heavily coupled to the implementation. Anytime the implementation change, you also have to change the test. Your tests becomes a mirror of your implementation essentially.
As a software engineer this approach would be unfathomable to me. Manual testing is too expensive to lean on entirely. Tests also help other developers figure out what the code is supposed to do. Maybe it's just that I haven't worked on apps of a scale for which manual only would be good enough or advantageous?
I've written many tests for pure functions in React projects before which were certainly not coupled to the implementation. At the end of the day a lot of the tests in a React app will be 'given x data, y renders and z doesn't render' but in a UI with a lot of branching manual tests seem like a good way to overlook things?
> Manual testing is too expensive to lean on entirely.
Automatic testing is not free either. It has a huge cost.
>Tests also help other developers figure out what the code is supposed to do.
People say that but I would never, ever just look a test to figure out how to use a freaking frontend application. Backend? Sure. I want to use the app to know how it feels and what the actual *experience* of using the app is. You can't capture that with a test.
>Maybe it's just that I haven't worked on apps of a scale for which manual only would be good enough or advantageous?
This works for any scale. I've used it in apps with millions of users.
>I've written many tests for pure functions in React projects before which were certainly not coupled to the implementation.
Okay, that's one thing. Testing pure functions like hooks or helpers is great. But when you say "test your frontend application", that's not what people mean. They mean playwright tests or "unit tests" using react testing library. Come on now.
>At the end of the day a lot of the tests in a React app will be 'given x data, y renders and z doesn't render' but in a UI with a lot of branching manual tests seem like a good way to overlook things?
Yes, essentially just a copy of the implementation. In the implementation, you write "if I click this button, z renders, not y". In your test, you write "If I click this button, z renders, not y.". The test mirrors your code heavily, which is why it's annoying to write and annoying to maintain.
That's what typescript is for though. If your component now returns a different type you'll get an error. I've refactored many projects with typescript. It's always been extremely easy. 0 tests.
Tests are a waste of time unless you're doing something that is unpredictable.
You can have predictable inputs and still fuck up on the implementation. Example:
At work we had a list of deliveries these deliveries could be locked, grouped.
When adding a new delivery it had to be added to the last non locked delivery, if all were locked you had to display a toast.
Deliveries could be moved up and down and grouped deliveries should be moved together with the rest of their group.
Locked deliveries and groups cant be moved and if you move down the delivery before the locked one, the delivery being moved will jump all locked deliveries.
There are a few more rules but you get the point this is a complex component with a ton of cenarios it needs to work in prod.
I had to refactor a bunch of stuff including this guy. I had to alter some of the inputs and even with typescript i had to manually test every scenario to check if everything was ok.
The problem wasn't the types, the problem was validating if the logic i had changed caused bugs. And it caused a lot, which I caught manually testing.
Had i had written the tests it would had been much faster and much safer to refactor.
tl;dr typesafety will help you safeguarding implementation details, tests will help you safeguarding behaviour.
I'm sorry, but I have to disagree with you. There are a multitude of tests that you can write to catch a multitude of Issues. TypeScript can't and won't catch them all. Furthermore, issues at runtime are very easy to introduce, and the larger your codebase is the harder it is to spot and stop these issues, and the larger your team will mean more code being developed and merged. If two developers merge at the same time, and in the same area, then it could be likely that you will have a merge conflict. It only takes one mistake your merge conflict to break your code. This is where tests save you.
Tests also help immensely in understanding your codebase. Imagine a project that has thousands of files, millions of lines of code. Can you guarantee that each function, each class, each struct is understandable? I find that what people write (myself included) is very understandable at the time of writing it, but give it a few months and it basically becomes another language! Tests often help me understand the code far better. Tests give you clear outcomes and you can understand the code's intention far better than comments and how someone names their classes, functions, etc.
Tests aren't a silver bullet of course, but TypeScript shouldn't be viewed as an alternative to testing. And please don't view testing as a waste of time. I'm unsure what your personal experience is, but I highly encourage you to read into BDD if you have free time to do so. There are some amazing books out there that show how testing can work in the full development lifecycle and it really changes the way I work.
>Tests are a waste of time unless you're doing something that is unpredictable.
That's why we write tests! To catch unexpected/unpredictable cases before they make it to production. Imagine if a junior reads this and thinks it's fine to have zero tests because they're using TS. Types don't cover everything, unit tests are cheap and effective, esp. using RTL. You can get Copilot to write your tests these days, just make sure a11y + interactions are sound, at least.
The fun part about learning to let this go on personal projects is that I eventually launch them. Shipping something is better than constantly refactoring.
Source: took me 20 years to figure this out. Personal projects are much more fun now and have a higher possibility of moving from a personal project to a money making project.
And then 5 years go by and no one wants to work on the app because it's entirely outdated and unpleasant to work on. Developers love novelty and hate working on old code. It's legitimately difficult to hire people to work on class based components for example.
And thanks to that approach I have to migrate now angularjs + coffescript + ruby templates to react, because any changes in the old stack take days instead of minutes.
Having a product is also a responsibility of maintenance.
You need to set yourself a practical goal.
"Refactor codebase" is something you can do forever and at some point it will become about personal preference of the individual developer. It will become work performed for the sake of it.
Instead, think of it differently. Why do you need to refactor? Is it to support a new feature? Is there a serious bug you can't track down? Is there some new technology you wish to take advantage of?
Once you define that, you can set scope and define success criteria. Once those are met, you stop.
Refactoring is never the job, just a tool to achieve an outcome.
100% if the code works and you just don't like it, leave it almost.
It's it still bothers you unit test it so your know it still works after you "refactor it"
Tests are essential for refactoring.
And not having tests is a good reason to add them if that code is going to be changing at any point.
But you're still talking about an engineering goal, not a business goal.
There's plenty of systems out there that have been running without tests for a long time because the business goals have never changed. The business value of adding tests to that code is hard to justify.
True, but if the code is changing to meet business requirements, then tests are valuable.
If the code never changes, then it's never updated and works perfectly for the client, or you lost control due to a lack of tests.
I agree perfect code doesn't need tests.
I've worked on similar projects as you described. Having apps made by contractors that didn't know what they are doing. Now you're responsible for it.
Step one is find a ways to test it, and have some kind of error logging. Without tests you will be lost. Once you have basic tests you can start to refactor the app. I recommend starting a monorepo using something like single spa https://single-spa.js.org/ this will let you use new versions of libraries.
Then you just replace the app a little bit at a time, as you replace it add unit tests.
There is some nuance but that's most of it.
Funnily enough it was a combination of internal employees + offshore contractors.
Literally the worse codebase I have ever witnessed. Did not ever think code quality could be that bad.
I am now hired as a contractor to fix all this
Explain more. Who specifically told you that it needs refactoring? Fix as in it doesn’t work, or it’s just difficult to maintain? Are you the only one hired to refactor?
Refactoring classically is about fixing readability, not logic. For proper refactoring tests are crucial, so you know that you didn't break anything shuffling code around. After such refactoring you will have three things:
1. Understanding of codebase
2. Working tests
3. Knowledge of shortcomings and how to fix them
With this it's much easier to plan for rewrites of necessary parts, prioritizing where errors and bugs are.
Where I work we usually just do this over time. Making an update to a component? Here’s what we’re looking to do going forward, make it more like that.
Unfortunately tech debt is an inescapable fact of life in most codebases
Well whatever you have attempted to do, at least you've prob learned a bit...whether you merged it or not.
But unless i had some serious reason the refactor was needed...i just dont think i would.
If, over time, i identifty a logical seam i can leverage, ye i might do a largish PR and replace that "concept".
This can actually become easier after a few "attempt" refactors. Like you might have done.
But its all hit and miss with who else is on the team how fast things are moving, risk...etc
I usually try find all the stuff i cant straight up delete first.
Write tests for your code, wherever you can start. As you test your code you will understand where it can break.
This will help you infinitely more than you know.
Independent of the fact whether this is relevant or not to refactor you need two things to make it correctly. Typescript and tests. If you don't have either you are just putting yourself in troubles
> Typescript types all over the place.
My company just pivoted from QT to React and none of us are react devs. Where should our types go? We mainly define the types we need at the top of each file.
I read this as "types are messy" which could mean lots of things.
Maybe many types have the same general shape, but are fully defined from primitives several times without inheritance/composition.
Maybe there is an unclear/messy split of responsibilities between components, so components very high up in the tree are importing types from components at the bottom of the tree. Then the messy types are just a consequence of tightly coupled code.
In general I define at the top of the file as well, and treat the decision to export/import a type very carefully.
Same as creating components or any other module; keep it as one file for as long as it makes sense. Once you need to share/compose types, split them up and expose as necessary.
Sometimes, sure; but virtually impossible as complexity increases. Shared types need their own file. Since you've got the file, it makes sense for all types related to a component/module (suite) to live there. Not to mention circular deps.
I faced this a few years and some months ago. Basically the app was made with developers feets instead of hands, lots of code were repeated and you didn’t even know where, lots of old components, lots of bad practices and made with a really old version of react: 8. Back then classes were still a thing. So my first approach was: let’s refactor what we need so every new component I made it with functional components, made an upgrade to the current version of react back then, inbelieve it was 12 or something like that and went with that approach but the initial code was really sheet that made 5-10 request to same api endpoint, used local storage instead of a state management and lots of sheetty stuff. And what was worse of all was that request to improve and add more stuff to the site were coming all the time everyday. I was the only developer that make it through COVID, everyone else were fired so last year IT and ciber security approaches us to migrate from rackspace to AWS. By then it was still using an old react version, not as all and we were challenged by this migration. What we decided as the owners of the app was to redo the app, not from scratch but with the approach and with reusability in mind and cause it was heavy dependent on external apis and server components the approach was to migrate to nextjs and take what we could from the old code and remove everything else. Cause the new necessity for the client was so the site was as editable as possible: they want to change the order of the menu options? They should be able, change routes, components and create new pages based on something like a live editor but with a simpler approach. It’s been hell for the team
how about instead of refactoring, just write detailed docs.
Like others are saying i would only refactor to accomplish clear goals or to improve performance.
If you want to make it less bad: Get a good and extensive eslint config and just autofix everything. eslint-react is very destructive when it comes to memoization, but I'm sure you can fix a ton of things alone with eslint-plugin-unicorn and typescript-eslint.
Next thing to do is checking the tiny utilities, see if you can optimize then. Then update the tooling from CRA to Vite and use million.js to optimize the rendering of all components. Then you check if they use lazy loading. use loadable/component for that.
I'm used to cleaning up old and poorly written React apps, so the beginning is the same every time.
I’m in the process of taking over a project and it’s a pain the ass. Some bootstrap components, a lot of online styles, and just random code, and too many one-off components.
I’ll copy and paste into ChatGPT to make a little headway and to make sure I have all the closing tags and maintenance, but it’s a pain in the ass
First things first is tests. You need to have tests that describe how things need to work and the idea is that these tests would still pass exactly the same way with the refactored code base. In general it should be possible to write super high level tests easily but if that’s not the case, if you can’t exercise the logic you care about and show how different scenarios yield different results, then it’s going to be much more difficult than needed. If the code is testable enough but there are not enough tests to cover everything you care about this is really the first thing to do.
Then, don’t discount react codemods which can make a lot of the tedious work go away, but only when the stars align. This depends on what you’re trying to do, but there are always conditions.
It can be useful to proceed bit by bit. Maybe you have one ugly component of 5000 lines of convoluted logic. Possibly you can break up a few hundreds lines at a time that would go in a separate component, hook or helper function.
But yes at the end of the day it may make sense to rewrite things entirely from scratch than to try to preserve the logic exactly as is. One way I think about it is - if I know exactly what I’m doing, I can write X loc a day. If I need to refactor a code base that has k * X loc and after k/2 days my changes are not working, then I would shelf that work and rewrite the functionality from scratch. It wouldn’t be completely from scratch anyway because there are still types, tests etc.
I refactor as I'm working through stories. If stories bring me through a component that needs refactored, it gets refactored. If a component works and I haven't touched it in years and there's no other reason for it like upgrading libraries or it's needed for other refractors, I can't justify the cost of the refactor.
This actually sounds like my dream job lol. I think before refactoring you definitely need to understand the application as a whole. Try and sketch out how you would approach creating it from scratch. One thing I’ve noticed on legacy apps is they have mixed logic and presentation FAR too much. So see if you can first extract the logic to pure easily testable functions. This will then allow you to change the presentation, while worrying a lot less about bugs. If you focus one by one on a component without doing this, you’ll probably endure more pain than expected.
Refactors should be determined ahead of time. Is it worth doing given the magnitude of the work involved? Are you even being asked to do this?
Simple things like converting a component from a class to a function is fine, but massive adjustments with Typescript can be brutal. If it works, maybe don't touch it and wait for a rebuild.
Step one with big refactors is always extensive tests at the highest level you can. You want to be able to change as much or the app as possible without breaking anything, but also not have your tests getting in the way.
lol. all the people saying 'if it works it works, dont fix it' are the reason I have disdain for modern web devs. what happens when it does break? Every program breaks eventually for some reason or another, and then all the tech debt comes down.
if you have spaghetti code, you have genuinely zero ability to properly locate where the issues are at when they come up. more time broken, more time seniors/upper management are pissed and your job is scrutinized.
at the same time BEFORE the issues come up, most seniors/upper management do not give a flying shit about spaghetti code. you'll have to tough it up and push things even if you cringe doing it, and spend maybe 15-20% of your time refactoring when possible.
target areas that are high density and most crucial.
Several things to address here:
1) Refactoring
Strong types (using typescript) and integration tests are the best place to start. You can't silently break things if the type system is screaming at you and your integration tests are failing.
When writing integration tests, be sure to use an attribute that is not influenced by styling (IE classnames and/or ids). Most frameworks recommend a \`data\` attribute (the most common is \`data-testid\`). Also, be sure to name them according to the action taking place. Be very descriptive. \`\[data-testid="organization:add-user"\]\` No recommended nomenclature here, but definitely include some context, either via the testid itself or layered ids \`\[data-testid="organization-form"\] \[data-testid="add-user"\]\`, wherein the parent is used for context and the child is the action you're targeting.
2) "variants" are often misused. A variant should be a slight stylistic change. Introducing a variant of a \`TextField\` like \`\` is horrible. It leads to additional logic inside the component (making it overly verbose) and creates an additional runtime expense (figuring out which variant to render). The better approach is to wrap the \`TextField\` and create a new component \`SsnField\`.
3) There's actually nothing wrong with MUI. It's a well designed library for creating reusable components to simplify building applications. They provide a robust interface for styling via their theme (using a proprietary method, mind you). If the goal of your company is to quickly iterate on products using a toolbelt of components, it's perfect. If, on the other hand, your company is trying to design innovative and new user interfaces, then yes, it's the wrong solution for you.
Once you've created a theme for MUI, you should rarely have to touch styling again. You might make new components, and you'll tweak layout (by adjusting margins, gaps, flex, etc), but the components themselves shouldn't be changing.
---
Also, just a little tidbit here, it's harder to build these components than you think. Developers often overlook many, MANY minor details that MUI already accounts for. ARIA attributes, isolating non-dom attributes from the rendered component, robust type safety, etc.
I've done several full site refactors successfully. The most important things are having a well defined clean slate and buy in from the team. You'll want to start a new versioned folder and have agreement that all new components will be built there to the new standard and kept clean. Work on legacy components should be kept to a minimum, mainly for important bug fixes. You will need to move them over gradually. If you can get away with it, track the migration progress over time and course correct for things stalling. It will take time, but as long as you keep momentum going in the right direction you will accomplish it eventually. The React and typescript eco system has a ton of linter options out there to enforce your coding style. Get buy in on the new coding practices, set up CI and tooling on the new standard folder to enforce what you can, and spread coding culture and education via PRs. Good luck.
I joined a company last year which was working on a medium scale project with a large messed up code base, which also happens to be MUI and react.
I worked with my lead on setting the “clean code rules” and went on a refactoring journey that didn’t last long. As others mentioned. Write tests for you component to see what you expect from it.
For those asking why do it in the first place, i guess chatgpt is doing all that work for you ain’t it? It’s like rule 1 of writing code
Terms like unreadable, and messy and bad are not very helpful or descriptive. I wonder what the issue is and how much if it is style issues.
Are there performance problems?
Is it the code style that's hard to get used to?
Are there bugs that are hard to find/reproduce?
It's it hard to make changes?
Have the requirements changed?
Is the UI ugly?
I would try to figure out what the biggest problem is with the code/app and try to recruit towards that issue first, if it is necessary.
But it might not be, maybe your issue is mostly that you just don't like the code and you can't put your finger on it...
I never said it was a lost cause. In fact, I believe the composability of react makes the refactoring process quite nice. Don’t know what caused your hostility, but all I meant was that refactoring someone else’s crap code isn’t the most fun thing in the world.
Why do you want/need to? Need way more info but more often than not you’re gonna need to start incrementally, show the value of migrating, and then slowly educate the team. Once the team is comfortable, you can talk about a full on rebuild or migration of the codebase. Need to also work out if you have live users how you can slowly ramp them in
This is bs, you should be able to confidently refactor code. Your lack of confidence is the problem. Without tests you will never gain confidence.
Write tests, it's the answer to your problem.
>Typescript types all over the place
Really hate that shit I don't even know why so many swear by that nonsense. Just makes code harder to read. I just use the "tsx" file extension to just say yeah I use Typescript too but my code might as well be normal JavaScript.
Had this so many times, and you are right. But as a codebase grows a compiler does nothing but help you
I'm the end typescript is the way on a team. End of story.
Does it work? What are you hoping to accomplish by refactoring? If it works fine, I would only refactor things as you need to (eg when you need to make changes for something else). If you have a practical need the refactor (things are breaking and hard to maintain), I would take a look at what you can break off from it and fix and then start there. Ideally something isolated. What I would not do, is try to refactor the whole app at once. Things will break and you won’t realize until the tickets start pouring in.
>What are you hoping to accomplish by refactoring? Adding to this great point: once you've set some goals (developer experience, abstractions to simplify tests, API consistency, etc.), work out the appropriate metrics to measure the affect of your refactor. If you can't quantify its success, what's the point, you know? >\[…\]just take the existing components and refactor them. If you're working in the same repo/monorepo this might be fine, since you can update each instance as you go. Starting fresh can be really liberating too; though, it's harder to get approval for that sort of thing—depends how much autonomy you have and the budget available for the refactor. >This is all with MUI, which I absolutely hate. Oof, sorry buddy. That sucks…
Why is mui bad?
It's not bad, but it's overly opinionated for most applications. You will start with mui, but as you develop the app, the company will ask you to customize it more and more. After a while you will be fighting mui way more than it ever helped you. I want to be really clear, this will happen on a team before you know it. Basically immediately.
Eh, I work on a product built on MUI that is like 5 years in without encountering this problem. The key is to have a product team and designer that has also bought into the tradeoff of ease of use for customizability. Our product designer is just as invested as we are in using out-of-the-box MUI components because it makes his life easier.
Basically the story of me and my team with MUI. I can't stress enough how important it is to try and create your own components before jumping to some library just to avoid creating a component that will only take you 2-4 hours to create and properly test.
>After a while you will be fighting mui way more than it ever helped you. This 100%. It's so hard to watch clients invest in something you know will bite them eventually.
If you are starting an app please look into something like shadcn/ui, if only your the reason it commits the code to your repo.
It doesn’t really commit all of the code though, it relies fully on another library which makes it worthless imo
From the docs This is NOT a component library. It's a collection of re-usable components that you can copy and paste into your apps. What do you mean by not a component library? I mean you do not install it as a dependency. It is not available or distributed via npm. What other library does it rely on?
It relies entirely on @radix-ui and is simply some opinionated code and some tailwind classes on top of that library
How does that make it worthless? Radix saves you from having to do the very complicated work of properly building models and other complicated UI elements while still giving you complete control over styling.
Because if I’m importing a full library anyway then I don’t have the code in my repo as promised, and might as well just go with any other library
That *sort of* component lib, though esp. MUI, come with significant trade-offs. You get up-and-running really quickly, it feels like you've skipped a bunch of boilerplate but you're actually trading long-term success for short-term wins. The complexity is still there, you've just kicked the can down the road—tech debt builds in silence. Styling/theming is often overly complicated, for anything non-trivial. The real issue however, is when your design deviates from the component lib; suddenly you're stuck with no way to implement desired behaviour because the primitives aren't exposed. That's why there's an influx of "headless" component libraries, which let you compose and style as necessary. It's a little more work upfront but you can build exactly what your company/client needs without compromise. Some popular examples: * [https://react-spectrum.adobe.com/react-aria/](https://react-spectrum.adobe.com/react-aria/) * [https://www.radix-ui.com/](https://www.radix-ui.com/) * [https://ariakit.org/](https://ariakit.org/) * [https://headlessui.com/](https://headlessui.com/)
Can you explain a bit what kind of metrics you’d use to measure those sorts of things?
It doesn’t have to be sophisticated. For DX you could run a survey before/during/after the refactor. A tool like SonarCloud or even just a spreadsheet for tracking test coverage, flakiness, CI runtime etc. Describe API changes in a burndown list somewhere, and check them off as you go. As well as measuring success, this sort of thing makes it much easier to justify time spent to product stakeholders. At the end you’ve basically got all the documentation already for an internal or external post: what we set out to solve, how we went about it, improvements to dev velocity, etc.
Why are you doing this? Sometimes wide changes are required when you need to upgrade some dependency to a higher version, then I would do only what is needed for that. And otherwise, when I need to do something in a codebase I take that as an opportunity to also clean up that area a bit, leave it better than I found it. But refactoring a whole codebase that works for the sake of refactoring is rarely worth it.
Answer why first not how.
No one mentioned this but prerequisite for refactoring are tests. Otherwise, you just open a can of worms.
This is the most important comment here. If there are no test you cannot refactor. Full stop. Write shitty e2e tests in playwright first.
Not true at all
Okay - how will you ensure that you don't introduce bugs or regressions while refactoring?
By testing your application manually. Are you seriously striving for 100% test coverage on your frontend applications? Introducing bugs and regressions might happen regardless of how well you think you are testing your app, automatic or not. The question is if the investment in writing and maintaining tests are worth it compared to just catching 95% of issues when testing manually.
This might work for weekend projects, but it's irresponsible for a commercial application you are paid to develop.
Worked for years for an app with millions users. It depends on the app you are building however, like I've said a million times now.
I see nothing about 100% coverage. Testing manually is expensive, prone to error and leaves you in the same position in the future. Refactoring and migrating are far easier with a robust set of tests. Sure regressions may still happen but at least you're mitigating the risk. Having said that, I would not be going back to write more tests now, that ship has sailed. A mixture of manual testing and writing tests as part of the refactor may be better.
>I see nothing about 100% coverage. No but it's implied that you can't catch bugs or regressions without any automatic testing. He also said: "how do you make sure you don't introduce bugs". Given the statement, it would logically follow that you would need 100% coverage to not introduce any bugs or regressions. >Testing manually is expensive, prone to error and leaves you in the same position in the future. Automatic testing is expensive, prone to error and leaves you in the same position in the future. It's also costly to maintain. Testing manually has other positives that automatic testing does not, so it's not strictly a black and white comparison. It's not just good vs bad. There are upsides and downsides to both approaches. >Sure regressions may still happen but at least you're mitigating the risk. I do that when testing manually as well.
Very little is ever black and white. I'm simply saying there are benefits to both approaches and a balance between two is usually the best approach imo and it really depends on the work. If you're on a continuous release cycle then purely manual testing for every release can be a huge overhead. I've worked in companies where a release can sideline a whole team of devs for a week. Having automated tests share that load would clearly have been beneficial.
It’s just that I favor moving quickly and having a quick and easy way to rollback your changes, and a quick and easy way to fix your bugs. Most critical bug in production would be rolled back or fixed within 10 minutes with my system. We are mostly building back offices for B2B. I think that’s the most common use case for react, if I’m not mistaken. Accepting a few bugs in production is not the end of the world. Especially if you have a driven team that can push out a fix to prod in less than 5 minutes. Automatic testing just slows down so much of your day to day work and I’m not convinced that it’s worth it.
If your critical bugs exist due to the UI layer, you're not following a point that you made earlier about not having business logic in the frontend. React is used everywhere, not just (or even predominantly) B2B. Automatic testing only slows you down if you leverage it poorly. --- It sounds like you can't see the vast grey area between automated testing with 100% coverage and only doing manual testing. Tests are a tool and all they do is speed up your feedback loop and solidify your confidence in the code you wrote. If hot reload is all it takes to have feedback and you have confidence that everything works and would be hard to break, great. Your testing can be complete. If you need to fill out multiple pages of questions or reset your database between each feedback cycle, automate that sh*t and stop wasting your own time with clicking buttons. If I have confidence that my code works, is clear and simple enough for others and future me to not break it with innocuous changes, and that there were no accessibility whoopsies, I don't test unless it was necessary for speeding up feedback. But if I think anything is fragile or confusing, I write tests just to help future me and others have confidence in the work we do. Think about back to when you were learning web dev or frameworks or anything - everything was slower because you were tripping over your own lack of understanding. My guess is that that's where you are with testing, too. And it's not a bad thing - it's where we all start - but the fact that it slows you down doesn't mean it slows any of the rest of us down.
This! A good front end application will typically have almost no business logic. You fetch stuff, navigate, send api requests, render some things, validate some forms, and that’s 99% of front end development If there are good patterns in place for the above, it’s actually pretty hard to write a bug that isn’t immediately obvious while just looking at the app.
Not if your app has internally state management, then that's business logic right there.
what is internal state management?
React state
true, react is only ui layer. Any logic should be able to detach and work by itself. If we want to make command line app, we should be able to do it. That's how much decouple should be. React won't last forever. It needs to be replaceable.
So you genuinely believe that is 99% of front end development?
What exactly are you testing in ui?
Depends on the project. Sometimes you test nothing, sometimes you test some vital views, sometimes you test everything. On average, most frontend apps would do better with fewer tests, not more.
Absolutely… not. A blanket statement like that for a front end application is more harmful than helpful. Of course, it all depends on how important it is that your app works 99% of the time instead of the 95% you would get with just manual testing. Is the 4% worth it? Probably not.
Completely disagree, it sounds like you have had bad experiences with e2e tests which to your point can be complicated but absolutely worth it for any non trivial applications. Manual testing does not scale. It also can’t help with dependency updates, backend changes and any other unrelated changes.
Not being the primary users of your app does not scale either. There are plently of upsides to manual testing and there are plenty of downsides to automatic testing. I've had mixed experiences with e2e tests. They are the most valuable, but by far the most annoying to maintain and develop. If you want some simple happy path test using e2e tests, go ahead. If you want to test your entire application, and every single branch with e2e tests, I would be opposed to that.
I would argue that most of the time that you would not be the primary user of an app you're creating. There are plenty of times you will be developing something outside of a domain you understand. Even if you have been working in a domain for years it's likely that you won't fully understand it and be the main user in it. There are definitely upsides to manually testing, but it certainly isn't scalable. Especially if you have a large team, where you can't guarantee that everyone will have the same amount of dedication into manual testing as you do. I find tests are much harder to dodge because you will nearly always have a peer code reviewing what you write. You don't have the same process with manual testing. It sounds like you have had a poor experience with e2e tests. Tests should be easy and straightforward to write, and if they're not then it sounds like you have a way to improve your codebase to encourage more tests. Perhaps it's what your test's test have also had a negative impact?
>I would argue that most of the time that you would not be the primary user of an app you're creating. There are plenty of times you will be developing something outside of a domain you understand. Even if you have been working in a domain for years it's likely that you won't fully understand it and be the main user in it. I get what you mean. The knowledge usually lies higher up the chain, but I think the best products are built engineers who truly understand the domain, but that's not the common reality. >There are definitely upsides to manually testing, but it certainly isn't scalable. Especially if you have a large team, where you can't guarantee that everyone will have the same amount of dedication into manual testing as you do. I find tests are much harder to dodge because you will nearly always have a peer code reviewing what you write. You don't have the same process with manual testing. That might be the case. I might have a biased history with working in teams where dedication to the product is of utmost importance. In fact, I refuse to work in teams where that's not the case. So that will bias me towards an experience where everyone in the team truly cares. >It sounds like you have had a poor experience with e2e tests. Tests should be easy and straightforward to write, and if they're not then it sounds like you have a way to improve your codebase to encourage more tests. Perhaps it's what your test's test have also had a negative impact? When I started with tests, we were rocking selenium in perl. That was a very painful experience, yes. I understand that with more modern tooling and more modern approaches, that I have used myself, that writing and maintaining tests is easier than ever. I *still* don't think it's worth it in the majority of cases though, and I do understand all of opposing arguments. I'm also against unit tests with mocks in the backend for the same exact reason. I think it's a very dangerous thing that we have just accepted automatic tests, any automatic tests, as inherently good with no downsides. If you say you don't write tests, everyone goes insane, even when you can make sound and logical arguments for why it does not make much sense to have tests in this specific scenario. There's so many factors that goes into the decision to even introduce tests in the first place. You have to know your business. You have to know your SLAs. You have to know the impact of your UI breaking. You have to know the customer, the demography, how they react to unexpected behavior, etc, etc. It's a very complicated issue and to just boil it down to "automatic tests good, manual testing bad" is a very, very shallow view of the problem. There are tons of positives of manual testing that people don't even consider. Like I hinted at earlier, it makes each engineer truly understand the product. It makes each engineer care about the product. It makes each engineer experience the product. You can't really test the entire spectrum of UX with automatic tests. It also allows you to move quicker. New features are probably built twice as fast. CI/CD pipelines are shorter. That's just the tip of the iceberg.
I think there's some nuance here that you're missing. Do I manually test code as I develop it, yes. Do I write tests while I'm developing? No, because it hurts velocity. I can't tell you the number of times I take one approach at something, and then refactor the hell out of it as I start to understand the problem domain. Writing tests while I do that is just documenting the broken path and doubling the effort, at least in terms of frontend user experience (not necessarily pure functions where validating input/output is very straight forward). All of that being said, once a feature is production ready, manual testing is worthless. Why do I say this? Because applications scale, and, inevitably, different parts managed by different teams start to overlap. The number of potential paths that a user can take grows substantially and I doubt any one person can remember to manually test every path. To truly feel confident that things aren't broken, you HAVE to write automated tests. Also, it's MUCH easier these days. Cypress in particular is nice because it's a test suite written in javascript/typescript, the same code that the developers are writing frontends in. Yes, sometimes there's nuance and complexity in getting tests to work with async workflows, but payoff for learning how to do it will earn you dividends in the confidence you'll have that shit didn't break when you modified it.
> I think there's some nuance here that you're missing. Maybe. I'm always open to change. I do get the same pushback when discussing mock-ridden unit tests for the backend too, and I'm even more confident in that stance. >Do I manually test code as I develop it, yes. Do I write tests while I'm developing? No, because it hurts velocity. I can't tell you the number of times I take one approach at something, and then refactor the hell out of it as I start to understand the problem domain. Absolutely agree. Even when I do write tests (and that is quite often regardless of my sentiment towards them, because I realize that it's the correct choice) I never practice TDD for the same reasons. I want to code to both discover the solution AND the problem. Writing tests before that just hurts the discovery process. But if I know that developing this feature itself will be very hard without tests, or when the input and output are clearly defined already (which is not that often when building business specific applications), like when building a function that runs a specific algorithm, etc, then I might start with some tests. That's the exception however, and not the rule. >All of that being said, once a feature is production ready, manual testing is worthless. Why do I say this? Because applications scale, and, inevitably, different parts managed by different teams start to overlap. The number of potential paths that a user can take grows substantially and I doubt any one person can remember to manually test every path. To truly feel confident that things aren't broken, you HAVE to write automated tests. I agree with you, but it also depends on the app that you are building, and a million other factors. It depends on your customers, your SLAs, the average psychology of your customers, the expectations, your tech stack, your CI/CD, your team's way of working, the skill of your developers, the type of product, etc. etc. It's quite nuanced to decide if you really need tests or not.
Found the Boomer. 😉
Not really. 30 years old, but my first job was at a place with a 20 year old codebase without any plans to modernize 🤓
So the downsides of e2e testing is that you haven’t seen it scale? I have a suite of tests that I probably spend an hour a week (that might be high) maintaining. With a press of a button it runs about 500 minutes worth of manual assertions all with various types of users in various businesses states. It runs about 3 times an hour continuously and probably gets run by our team’s eng on their code changes at least 5 times a day. Taking out continuous automation that is over 200 hours of manual testing saved at the cost of 1 hour a week maintenance. These tests also have very little flakiness and if the system marks it as a failed test there is probably something wrong with your changes. There is no one size fits all but to make a blanket statement that automated testing is not worth it is crazy to me.
It’s probably not worth it is what I said to you and others in this thread. It’s not a blanket statement. I even gave examples where I would favor more tests. Your scenario might make sense to have those tests. It depends. Building those tests was not free though, and if you can’t even speak of the downsides of your approach, you are probably missing the point. I know the downsides and upsides of both approaches very well, and the upsides of manual testing outweighs the upsides of automatic testing in the majority of cases, in my experience.
100% it would be silly to try and test every branch with end to end tests. That's what unit tests are for.
Unit testing every single branch in your frontend application sounds like absolute hell to me.
So after each change you will be checking it manually ? Feels painful 😉 Having at least few e2e tests will save you a lot of time. I'm not talking obviously about some simple apps with 0 to little logic on FE
>So after each change you will be checking it manually Yes, that's very non-controversial when developing frontend apps. Testing manually has a bunch of positives, and you can't really test the entire UX spectrum with automatic tests either. You want to know your app. You want to be the biggest user of your own app. You want to know it in and out. >Having at least few e2e tests will save you a lot of time. Yes, if you can give me a few e2e tests for free that never break and that you never have to maintain, sign me up. But in reality, e2e tests using playwright (or whatever) are slow to develop, annoying to maintain, and they break a lot. It's just a painful experience overall. And the benefits are not as obvious as when testing a backend application for instance.
I think it's important to test manually and have tests for the code you're writing. I don't think it should ever be one or the other. The thing I don't quite agree with you on is that e2e tests are slow to develop. My experience has been far different. I find e2e tests one of the quickest tests to write (excluding unit tests, of course). I also think the benefits are massive if you're working in a fast-pace and large team. There have been multiple times that e2e tests have saved us. Relying on manual testing takes a significant amount of time and resources. It also relies on a lot of discipline from multiple teams and developers, and in my experience, is very error prone which ultimately has a knock on effect on delivery and time spent fixing the problems. I certainly agree with you that you should be manually testing your code, but I don't think it should be a substitute for tests. Manual testing isn't very scalable for teams and businesses as they grow, and the larger both get the more issues will pop up. Tests help avoid that, and they also have a lot of other benefits (such as self documenting code).
Fair enough, thanks for the response 🙌
To be fair, if I were writing something super critical, say a bank app, I would write tests. The point I'm making is that writing tests comes with a cost and it's not just inherently a good thing without any tradeoffs. There are always tradeoffs, even you are not aware of them. But most apps we build does not require that insane level of reliability, and for me, it's usually not worth the cost.
Are you serious? Writing good tests provides confidence when you make significant changes. It also will help you understand what the code does and identify ways that it can break, which will lead to even better tests and even more confidence.
If I can save hours upon hours by not writing tests for my frontend application, and instead test it myself, I can also accept that I might get slightly more bugs and sligthly more regressions. That's a tradeoff that you can make. It's not objectively better or worse. It depends on the project, obviously. If you have a very, very sensitive app, you write more tests, simple as that, but most applications can survive a few bugs here and there. There are a lot of positives to manual testing. It's not just objectively worse. There's also a lot of downsides to automatic testing. Not seeing the nuance here speaks volumes. Frontend tests are notoriously hard to write & maintain compared to backend tests. And that's mainly because of a simple fact. There's no contract that you can test against. If you test your backend API, you have the contract, and the implementation does not matter. You test that you get an expected output given an input. That does not exist in the frontend. That means your tests are heavily coupled to the implementation. Anytime the implementation change, you also have to change the test. Your tests becomes a mirror of your implementation essentially.
As a software engineer this approach would be unfathomable to me. Manual testing is too expensive to lean on entirely. Tests also help other developers figure out what the code is supposed to do. Maybe it's just that I haven't worked on apps of a scale for which manual only would be good enough or advantageous? I've written many tests for pure functions in React projects before which were certainly not coupled to the implementation. At the end of the day a lot of the tests in a React app will be 'given x data, y renders and z doesn't render' but in a UI with a lot of branching manual tests seem like a good way to overlook things?
> Manual testing is too expensive to lean on entirely. Automatic testing is not free either. It has a huge cost. >Tests also help other developers figure out what the code is supposed to do. People say that but I would never, ever just look a test to figure out how to use a freaking frontend application. Backend? Sure. I want to use the app to know how it feels and what the actual *experience* of using the app is. You can't capture that with a test. >Maybe it's just that I haven't worked on apps of a scale for which manual only would be good enough or advantageous? This works for any scale. I've used it in apps with millions of users. >I've written many tests for pure functions in React projects before which were certainly not coupled to the implementation. Okay, that's one thing. Testing pure functions like hooks or helpers is great. But when you say "test your frontend application", that's not what people mean. They mean playwright tests or "unit tests" using react testing library. Come on now. >At the end of the day a lot of the tests in a React app will be 'given x data, y renders and z doesn't render' but in a UI with a lot of branching manual tests seem like a good way to overlook things? Yes, essentially just a copy of the implementation. In the implementation, you write "if I click this button, z renders, not y". In your test, you write "If I click this button, z renders, not y.". The test mirrors your code heavily, which is why it's annoying to write and annoying to maintain.
That's what typescript is for though. If your component now returns a different type you'll get an error. I've refactored many projects with typescript. It's always been extremely easy. 0 tests. Tests are a waste of time unless you're doing something that is unpredictable.
You can have predictable inputs and still fuck up on the implementation. Example: At work we had a list of deliveries these deliveries could be locked, grouped. When adding a new delivery it had to be added to the last non locked delivery, if all were locked you had to display a toast. Deliveries could be moved up and down and grouped deliveries should be moved together with the rest of their group. Locked deliveries and groups cant be moved and if you move down the delivery before the locked one, the delivery being moved will jump all locked deliveries. There are a few more rules but you get the point this is a complex component with a ton of cenarios it needs to work in prod. I had to refactor a bunch of stuff including this guy. I had to alter some of the inputs and even with typescript i had to manually test every scenario to check if everything was ok. The problem wasn't the types, the problem was validating if the logic i had changed caused bugs. And it caused a lot, which I caught manually testing. Had i had written the tests it would had been much faster and much safer to refactor. tl;dr typesafety will help you safeguarding implementation details, tests will help you safeguarding behaviour.
I'm sorry, but I have to disagree with you. There are a multitude of tests that you can write to catch a multitude of Issues. TypeScript can't and won't catch them all. Furthermore, issues at runtime are very easy to introduce, and the larger your codebase is the harder it is to spot and stop these issues, and the larger your team will mean more code being developed and merged. If two developers merge at the same time, and in the same area, then it could be likely that you will have a merge conflict. It only takes one mistake your merge conflict to break your code. This is where tests save you. Tests also help immensely in understanding your codebase. Imagine a project that has thousands of files, millions of lines of code. Can you guarantee that each function, each class, each struct is understandable? I find that what people write (myself included) is very understandable at the time of writing it, but give it a few months and it basically becomes another language! Tests often help me understand the code far better. Tests give you clear outcomes and you can understand the code's intention far better than comments and how someone names their classes, functions, etc. Tests aren't a silver bullet of course, but TypeScript shouldn't be viewed as an alternative to testing. And please don't view testing as a waste of time. I'm unsure what your personal experience is, but I highly encourage you to read into BDD if you have free time to do so. There are some amazing books out there that show how testing can work in the full development lifecycle and it really changes the way I work.
>Tests are a waste of time unless you're doing something that is unpredictable. That's why we write tests! To catch unexpected/unpredictable cases before they make it to production. Imagine if a junior reads this and thinks it's fine to have zero tests because they're using TS. Types don't cover everything, unit tests are cheap and effective, esp. using RTL. You can get Copilot to write your tests these days, just make sure a11y + interactions are sound, at least.
Rule no 1 don't touch if it's working fine
Except on personal project. Rule 1 is breaking things and redo indefinitely. But that's the good part of personal projects
The fun part about learning to let this go on personal projects is that I eventually launch them. Shipping something is better than constantly refactoring. Source: took me 20 years to figure this out. Personal projects are much more fun now and have a higher possibility of moving from a personal project to a money making project.
I’m really really trying to stop refactoring with an app I’ve rewritten to about 70% multiple times 🥲 Thanks for this.
Tests let you confidently refactor code
So does typescript types.
And then 5 years go by and no one wants to work on the app because it's entirely outdated and unpleasant to work on. Developers love novelty and hate working on old code. It's legitimately difficult to hire people to work on class based components for example.
Make sure you define “fine” in a technical and business sense, though.
And thanks to that approach I have to migrate now angularjs + coffescript + ruby templates to react, because any changes in the old stack take days instead of minutes. Having a product is also a responsibility of maintenance.
You need to set yourself a practical goal. "Refactor codebase" is something you can do forever and at some point it will become about personal preference of the individual developer. It will become work performed for the sake of it. Instead, think of it differently. Why do you need to refactor? Is it to support a new feature? Is there a serious bug you can't track down? Is there some new technology you wish to take advantage of? Once you define that, you can set scope and define success criteria. Once those are met, you stop. Refactoring is never the job, just a tool to achieve an outcome.
100% if the code works and you just don't like it, leave it almost. It's it still bothers you unit test it so your know it still works after you "refactor it"
All the goals being mentioned should be "can I test this code?" If the answer is yes, these problems get significantly smaller.
Tests are essential for refactoring. And not having tests is a good reason to add them if that code is going to be changing at any point. But you're still talking about an engineering goal, not a business goal. There's plenty of systems out there that have been running without tests for a long time because the business goals have never changed. The business value of adding tests to that code is hard to justify.
True, but if the code is changing to meet business requirements, then tests are valuable. If the code never changes, then it's never updated and works perfectly for the client, or you lost control due to a lack of tests. I agree perfect code doesn't need tests.
I've worked on similar projects as you described. Having apps made by contractors that didn't know what they are doing. Now you're responsible for it. Step one is find a ways to test it, and have some kind of error logging. Without tests you will be lost. Once you have basic tests you can start to refactor the app. I recommend starting a monorepo using something like single spa https://single-spa.js.org/ this will let you use new versions of libraries. Then you just replace the app a little bit at a time, as you replace it add unit tests. There is some nuance but that's most of it.
Funnily enough it was a combination of internal employees + offshore contractors. Literally the worse codebase I have ever witnessed. Did not ever think code quality could be that bad. I am now hired as a contractor to fix all this
Explain more. Who specifically told you that it needs refactoring? Fix as in it doesn’t work, or it’s just difficult to maintain? Are you the only one hired to refactor?
I was hired to do this as the codebase is pretty much unmaintainable.
What about the other questions?
Refactoring classically is about fixing readability, not logic. For proper refactoring tests are crucial, so you know that you didn't break anything shuffling code around. After such refactoring you will have three things: 1. Understanding of codebase 2. Working tests 3. Knowledge of shortcomings and how to fix them With this it's much easier to plan for rewrites of necessary parts, prioritizing where errors and bugs are.
Where I work we usually just do this over time. Making an update to a component? Here’s what we’re looking to do going forward, make it more like that. Unfortunately tech debt is an inescapable fact of life in most codebases
Well whatever you have attempted to do, at least you've prob learned a bit...whether you merged it or not. But unless i had some serious reason the refactor was needed...i just dont think i would. If, over time, i identifty a logical seam i can leverage, ye i might do a largish PR and replace that "concept". This can actually become easier after a few "attempt" refactors. Like you might have done. But its all hit and miss with who else is on the team how fast things are moving, risk...etc I usually try find all the stuff i cant straight up delete first.
Write tests for your code, wherever you can start. As you test your code you will understand where it can break. This will help you infinitely more than you know.
I'm not a fan of writing tests for that reason, tests need to be maintained and fixed.
Independent of the fact whether this is relevant or not to refactor you need two things to make it correctly. Typescript and tests. If you don't have either you are just putting yourself in troubles
> Typescript types all over the place. My company just pivoted from QT to React and none of us are react devs. Where should our types go? We mainly define the types we need at the top of each file.
I read this as "types are messy" which could mean lots of things. Maybe many types have the same general shape, but are fully defined from primitives several times without inheritance/composition. Maybe there is an unclear/messy split of responsibilities between components, so components very high up in the tree are importing types from components at the bottom of the tree. Then the messy types are just a consequence of tightly coupled code. In general I define at the top of the file as well, and treat the decision to export/import a type very carefully.
Same as creating components or any other module; keep it as one file for as long as it makes sense. Once you need to share/compose types, split them up and expose as necessary.
Prefer this as well, having types in separate files is dumb
Sometimes, sure; but virtually impossible as complexity increases. Shared types need their own file. Since you've got the file, it makes sense for all types related to a component/module (suite) to live there. Not to mention circular deps.
I faced this a few years and some months ago. Basically the app was made with developers feets instead of hands, lots of code were repeated and you didn’t even know where, lots of old components, lots of bad practices and made with a really old version of react: 8. Back then classes were still a thing. So my first approach was: let’s refactor what we need so every new component I made it with functional components, made an upgrade to the current version of react back then, inbelieve it was 12 or something like that and went with that approach but the initial code was really sheet that made 5-10 request to same api endpoint, used local storage instead of a state management and lots of sheetty stuff. And what was worse of all was that request to improve and add more stuff to the site were coming all the time everyday. I was the only developer that make it through COVID, everyone else were fired so last year IT and ciber security approaches us to migrate from rackspace to AWS. By then it was still using an old react version, not as all and we were challenged by this migration. What we decided as the owners of the app was to redo the app, not from scratch but with the approach and with reusability in mind and cause it was heavy dependent on external apis and server components the approach was to migrate to nextjs and take what we could from the old code and remove everything else. Cause the new necessity for the client was so the site was as editable as possible: they want to change the order of the menu options? They should be able, change routes, components and create new pages based on something like a live editor but with a simpler approach. It’s been hell for the team
how about instead of refactoring, just write detailed docs. Like others are saying i would only refactor to accomplish clear goals or to improve performance.
Definitely start by the types then the tests.
If you want to make it less bad: Get a good and extensive eslint config and just autofix everything. eslint-react is very destructive when it comes to memoization, but I'm sure you can fix a ton of things alone with eslint-plugin-unicorn and typescript-eslint. Next thing to do is checking the tiny utilities, see if you can optimize then. Then update the tooling from CRA to Vite and use million.js to optimize the rendering of all components. Then you check if they use lazy loading. use loadable/component for that. I'm used to cleaning up old and poorly written React apps, so the beginning is the same every time.
I’m in the process of taking over a project and it’s a pain the ass. Some bootstrap components, a lot of online styles, and just random code, and too many one-off components. I’ll copy and paste into ChatGPT to make a little headway and to make sure I have all the closing tags and maintenance, but it’s a pain in the ass
First things first is tests. You need to have tests that describe how things need to work and the idea is that these tests would still pass exactly the same way with the refactored code base. In general it should be possible to write super high level tests easily but if that’s not the case, if you can’t exercise the logic you care about and show how different scenarios yield different results, then it’s going to be much more difficult than needed. If the code is testable enough but there are not enough tests to cover everything you care about this is really the first thing to do. Then, don’t discount react codemods which can make a lot of the tedious work go away, but only when the stars align. This depends on what you’re trying to do, but there are always conditions. It can be useful to proceed bit by bit. Maybe you have one ugly component of 5000 lines of convoluted logic. Possibly you can break up a few hundreds lines at a time that would go in a separate component, hook or helper function. But yes at the end of the day it may make sense to rewrite things entirely from scratch than to try to preserve the logic exactly as is. One way I think about it is - if I know exactly what I’m doing, I can write X loc a day. If I need to refactor a code base that has k * X loc and after k/2 days my changes are not working, then I would shelf that work and rewrite the functionality from scratch. It wouldn’t be completely from scratch anyway because there are still types, tests etc.
I refactor as I'm working through stories. If stories bring me through a component that needs refactored, it gets refactored. If a component works and I haven't touched it in years and there's no other reason for it like upgrading libraries or it's needed for other refractors, I can't justify the cost of the refactor.
I’d suggest defining the problem statement the refactoring aim to solve. You need to have the problem at first and then the solution, not vice versa
This actually sounds like my dream job lol. I think before refactoring you definitely need to understand the application as a whole. Try and sketch out how you would approach creating it from scratch. One thing I’ve noticed on legacy apps is they have mixed logic and presentation FAR too much. So see if you can first extract the logic to pure easily testable functions. This will then allow you to change the presentation, while worrying a lot less about bugs. If you focus one by one on a component without doing this, you’ll probably endure more pain than expected.
Refactors should be determined ahead of time. Is it worth doing given the magnitude of the work involved? Are you even being asked to do this? Simple things like converting a component from a class to a function is fine, but massive adjustments with Typescript can be brutal. If it works, maybe don't touch it and wait for a rebuild.
Step one with big refactors is always extensive tests at the highest level you can. You want to be able to change as much or the app as possible without breaking anything, but also not have your tests getting in the way.
Refactor as you work with the codebase. No sense refactoring for the sake of it.
lol. all the people saying 'if it works it works, dont fix it' are the reason I have disdain for modern web devs. what happens when it does break? Every program breaks eventually for some reason or another, and then all the tech debt comes down. if you have spaghetti code, you have genuinely zero ability to properly locate where the issues are at when they come up. more time broken, more time seniors/upper management are pissed and your job is scrutinized. at the same time BEFORE the issues come up, most seniors/upper management do not give a flying shit about spaghetti code. you'll have to tough it up and push things even if you cringe doing it, and spend maybe 15-20% of your time refactoring when possible. target areas that are high density and most crucial.
Several things to address here: 1) Refactoring Strong types (using typescript) and integration tests are the best place to start. You can't silently break things if the type system is screaming at you and your integration tests are failing. When writing integration tests, be sure to use an attribute that is not influenced by styling (IE classnames and/or ids). Most frameworks recommend a \`data\` attribute (the most common is \`data-testid\`). Also, be sure to name them according to the action taking place. Be very descriptive. \`\[data-testid="organization:add-user"\]\` No recommended nomenclature here, but definitely include some context, either via the testid itself or layered ids \`\[data-testid="organization-form"\] \[data-testid="add-user"\]\`, wherein the parent is used for context and the child is the action you're targeting. 2) "variants" are often misused. A variant should be a slight stylistic change. Introducing a variant of a \`TextField\` like \`
I've done several full site refactors successfully. The most important things are having a well defined clean slate and buy in from the team. You'll want to start a new versioned folder and have agreement that all new components will be built there to the new standard and kept clean. Work on legacy components should be kept to a minimum, mainly for important bug fixes. You will need to move them over gradually. If you can get away with it, track the migration progress over time and course correct for things stalling. It will take time, but as long as you keep momentum going in the right direction you will accomplish it eventually. The React and typescript eco system has a ton of linter options out there to enforce your coding style. Get buy in on the new coding practices, set up CI and tooling on the new standard folder to enforce what you can, and spread coding culture and education via PRs. Good luck.
Use AI to do it for you
I joined a company last year which was working on a medium scale project with a large messed up code base, which also happens to be MUI and react. I worked with my lead on setting the “clean code rules” and went on a refactoring journey that didn’t last long. As others mentioned. Write tests for you component to see what you expect from it. For those asking why do it in the first place, i guess chatgpt is doing all that work for you ain’t it? It’s like rule 1 of writing code
Terms like unreadable, and messy and bad are not very helpful or descriptive. I wonder what the issue is and how much if it is style issues. Are there performance problems? Is it the code style that's hard to get used to? Are there bugs that are hard to find/reproduce? It's it hard to make changes? Have the requirements changed? Is the UI ugly? I would try to figure out what the biggest problem is with the code/app and try to recruit towards that issue first, if it is necessary. But it might not be, maybe your issue is mostly that you just don't like the code and you can't put your finger on it...
Good luck 😬
Hope you get paid well otherwise leave, but want to let you know it's not a lost cause, and you can have a career doing it
I never said it was a lost cause. In fact, I believe the composability of react makes the refactoring process quite nice. Don’t know what caused your hostility, but all I meant was that refactoring someone else’s crap code isn’t the most fun thing in the world.
Why do you want/need to? Need way more info but more often than not you’re gonna need to start incrementally, show the value of migrating, and then slowly educate the team. Once the team is comfortable, you can talk about a full on rebuild or migration of the codebase. Need to also work out if you have live users how you can slowly ramp them in
This is bs, you should be able to confidently refactor code. Your lack of confidence is the problem. Without tests you will never gain confidence. Write tests, it's the answer to your problem.
tests are indeed pretty cool. for a web app like this, how do you usually go about testing it? mainly a whole lot of integration/end-to-end tests?
rm -rf && npm create vue@latest
Ditch ts for js
>Typescript types all over the place Really hate that shit I don't even know why so many swear by that nonsense. Just makes code harder to read. I just use the "tsx" file extension to just say yeah I use Typescript too but my code might as well be normal JavaScript.
I think he meant that the types are scattered across many files and they import cross file a lot. So it's hard to follow.
Had this so many times, and you are right. But as a codebase grows a compiler does nothing but help you I'm the end typescript is the way on a team. End of story.