Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start decoupling from AngularJs #7718

Closed
17 tasks done
nathanwoulfe opened this issue Feb 25, 2020 · 39 comments · Fixed by #7738, #8501, #8505, #8538 or #8544
Closed
17 tasks done

Start decoupling from AngularJs #7718

nathanwoulfe opened this issue Feb 25, 2020 · 39 comments · Fixed by #7738, #8501, #8505, #8538 or #8544

Comments

@nathanwoulfe
Copy link
Contributor

nathanwoulfe commented Feb 25, 2020

Might be controversial, but here we go...

I'm proposing we start refining the backoffice javascript to remove/reduce the coupling to AngularJs. The quickest win here (IMO) is to remove all usages of Angular functions.

Most (if not all) of these exist as native JS methods in ES6, or are provided in Lodash. Umbraco currently takes a dependency on Underscore, of which Lodash is a superset. Benefit of Lodash is that it allows individual functions to be included, greatly reducing the size of the dependency. I don't see any issue in replacing a framework dependency with a library dependency.

So I guess this is two-fold - replace AngularJs fns with vanilla JS where possible, else import the appropriate Lodash method. At the same time, replace Underscore with vanilla, falling back to Lodash.

Why? It will need to happen eventually. Regardless of what direction the backoffice takes, the AngularJs functions will stop working. These changes can be addressed now as incremental improvements rather than leaving until later as part of sweeping backoffice changes.

TODO:

@anthonydotnet
Copy link
Contributor

Agree.

These are quick wins.

@c9mb
Copy link

c9mb commented Feb 26, 2020

I'll be even more controversial... migrate angularjs to vuejs - for which the above is a compatible step of an increment migration process. 😱

@nathanwoulfe
Copy link
Contributor Author

nathanwoulfe commented Feb 26, 2020

@c9mbundy even mentioning an alternate framework is waaaay out of scope for what I want to do here - these changes would be part of the required groundwork for shifting the backoffice to Vue, React, AngularX, JsDuJour * etc.

Really want this to be a completely stack agnostic conversation, focus on cleaning up/improving where possible to best facilitate whatever migration happens in future

* this does not exist. Yet. Consider this me staking my claim to the name!

@pgregorynz
Copy link
Contributor

pgregorynz commented Feb 26, 2020

Agree with this.

The less framework dependency the better.

In ideal world it would be really nice if plugins were completely agnostic of framework used but that there would be a simple interface for passing the model. Not sure how possible it is but dreams are free, although I know @PeteDuncanson did something like this a while back to run React in the back office?

@giuunit
Copy link

giuunit commented Feb 26, 2020

Agree with this. Slowly removing framework dependencies is the only way forward

@dawoe
Copy link
Contributor

dawoe commented Feb 26, 2020

Hi @nathanwoulfe

Same can be done for the usage of underscore.js. A lot of the functions in there are native as well.

Dave

@nathanwoulfe
Copy link
Contributor Author

@dawoe I know there's no native deep copy (lodash provides that) but outside of that, I think most everything is available native

@dawoe
Copy link
Contributor

dawoe commented Feb 26, 2020

As I understand the clone function from underscore is not a deep clone : https://underscorejs.org/#clone

I always use angular.copy for that

Dave

@nielslyngsoe
Copy link
Member

Hi @nathanwoulfe
This is great, and it's a great step towards moving forward.

Here are a few perspectives of why we think its right:

  • We are not ready or suited to write completely new code for BackOffice today.
  • Therefore we need to bring existing code up to speed.
  • We do know we need to get rid of AngularJS.
  • We do know that we want to be as little framework/library dependent in the future.
  • We do know we have a lot of code that could be using native technology instead of the framework/library.
  • And we do know we need to move forward in minor steps, to avoid making a mess and rapidly begin able to release stable versions.

I think you wrote a perfect description of the first goal for this journey:

Replace AngularJs functions with vanilla JS where possible, else import the appropriate Lodash method. At the same time, replace Underscore with vanilla, falling back to Lodash.

Feel free to get started, I would love to see a few minor PRs to ensure that we agree on the path. And afterward this could go for full-speed with up-for-grabs issues etc.

@anthonydotnet
Copy link
Contributor

This is a great initiative.

Now all we need is the blessing from HQ, and a few hackathons.

Then in a year we can discuss what ever hipster JS framework is around.

@nul800sebastiaan
Copy link
Member

And just to set some expectations; we don't have anyone dedicated to "new hipster JS framework" at the moment. The work resulting from this will be processed by the PR Team Core Collaborator team and it's easiest if this is happening in small PRs. The priority for evaluating incoming PRs is on fixing current bugs first.

And while I do love some good cleanup work, it's dangerous, due to lack of automated testing, so unit tests will help a lot in making getting changes evaluated and merged more quickly.

@nul800sebastiaan
Copy link
Member

Last note: while this is up-for-grabs, this issue will not be "fixed" by a single PR. It would be good if @nathanwoulfe kicks of with a few trial PRs and then to come up with a list of items that need improvement, so that other people can pick up various smaller bits to improve.

Please organize this so that the work is easy to share among a few people.
For inspiration, the accessibility team is really good at making a list of stuff like that: #5277

@umbrabot
Copy link

Hi @nathanwoulfe,

We're writing to let you know that we've added the Up For Grabs label to your issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly PR team bot :-)

@markadrake
Copy link
Contributor

markadrake commented Feb 26, 2020

  • We absolutely should replace what we can, code that is dependent on a specific framework or library, when native browser APIs can replace them.
  • Huge resounding H5YR for catching the potential savings LoDash offers.
  • Is there an opportunity to introduce tree-shaking? (Webpack? Parcel? Something else?)
  • Where browser adoption isn't the greatest, if we aren't already, there's no shame in using polyfills – or better yet ponyfills.
  • If Babel is not already part of the build process for the frontend (I am embarrassed for not knowing) it should be, and it would allow for us to write modern JavaScript and target a set of browsers for compatibility.
  • If a unit / end-to-end test does not already exist testing the functionality of code we update, we should take it upon ourselves to add it.

Just some thoughts as I read through this.

@PeteDuncanson
Copy link
Contributor

Wow. I'm excited to see this one get raised and get such a good reception. Bravo Nathan.

Small units of work would be the order of the day if we wanted to get this in pulled in. In my mind it would be super tiny changes per file (maybe even per method if its a big file to change) so that the CC team can easily eye ball it and pull it in. Plus if the changes are small enough you can get a few done in a lunch break and feel like you've achieved something and its gets HQ's PR count up so they can shout about that, win/win ;)

Testing is always lovely but its also difficult to create tests for some of the existing stuff the way its written (which is one of the reasons its not been done as yet). I think we would need someone brave to do a bit of a sprint into how best to do the testing and then show an example that others can follow. I don't think it should be a requirement though (swapping out one forEach implementation for another shouldn't require a test in my mind), but encouraged if possible.

For completeness for those that are hearing about work on tweaking the back office here are some of the older issues that contain lots of thoughts/ideas/plans on whatever we can do to polish up whats already there and get it in a fit state:

#4913
#4875
#4875
#4804
#4872

and the dreaded "Get TypeScript into the back office" thread #5215

@cssquirrel
Copy link
Contributor

Agree with all the agreeing. ;)

As has been already said, regardless of the backoffice's future form, it will definitely part ways from AngularJS at some point. This feels like a real approach to making an actual start to the process. Which is fantastic.

@protherj
Copy link
Contributor

#h5yr @nathanwoulfe, sounds like an excellent step to take.

@nathanwoulfe
Copy link
Contributor Author

Given this was a whole lot less controversial than I might have thought, I'm happy to take a lead on kicking off the work. Will follow the A11Y team's lead and go for a Trello board to manage work.

First though, will do some proper analysis of how much work is involved, where the quick-wins are hiding, and changes that may still require Lodash. From that, can map out a plan in Trello and away we go. Easy. Simple. 😃

More than happy to talk further about how to approach the work - by file, by function, by module etc.

@nathanwoulfe
Copy link
Contributor Author

Trello board for anyone interested. Feel free to add/change/move stuff. I'm not a PM.

leekelleher added a commit to leekelleher/umbraco-contentment that referenced this issue Mar 2, 2020
This, in part, is due to an Umbraco issue about reliance on
AngularJS functions, and the effort involved in future decoupling.

umbraco/Umbraco-CMS#7718
https://trello.com/b/wiDsUbly/umbracos-angularjs-divorce

By getting into the mindset of agnostic JS functions (where applicable),
the code will (hopefully) be more portable (migratable).
@ronaldbarendse
Copy link
Contributor

While I absolutely agree we should get ready for a client-side framework transition (especially because the AngularJS LTS period ends on June 30, 2021), I'm a little sceptical whether any of these code changes will eventually be part of the new codebase and/or add any real value.

Sure, it doesn't hurt to rewrite some JS functions and having additional tests would be great, but I think that time would be better spend in converting existing code to TypeScript (as @PeteDuncanson linked in his comment). That has the potential to reduce/prevent a lot of bugs, make testing easier and while doing so, also swap out a lot of these functions at the same time!

@nathanwoulfe
Copy link
Contributor Author

@ronaldbarendse as much as I'm on board with a TS migration, that's an order of magnitude more work than getting these changes done. Regardless of where the backoffice goes in terms of framework/language, these changes will need to be made, so no time like the present.

Sure, some/all of this may be removed in the future, but until that happens, it's still an incremental improvement to reduce the coupling to AngularJs.

@anthonydotnet
Copy link
Contributor

Agree. In order to TypeScript the code, we need the code to actually be written in JS. Currently it's full of legacy AngularJS functions. You can't TypeScript those.

Moving to as much vanilla JS as possible is the first step to actually using TypeScript.

@nielslyngsoe
Copy link
Member

I think everyone is in for a TypeScript codebase. But I would say that we first need to make sure that we are in a good state for implementing TypeScript.

I would love to see us making an effort to decouple things as the main focus. Just like Nathan now proposed getting rid of utility functions from AngularJS.
The next step could be moving Services out of AngularJS and splitting some services up to minor, or just turning some functionality into functions. And in this process, it could make sense to convert to TypeScript as well. But I think we are better of not focusing on TypeScript as the goal but focusing on decoupling. Ensuring that our architecture becomes SOLID, or at least closer to those concepts.

@MMasey
Copy link
Contributor

MMasey commented Apr 10, 2020

PR for isString #7929

@kjac
Copy link
Contributor

kjac commented Jul 28, 2020

Bit more work on replacing angular.forEach in #8501

@kjac
Copy link
Contributor

kjac commented Jul 29, 2020

And more angular.forEach replacement in #8505

@nathanwoulfe
Copy link
Contributor Author

@kjac lovely stuff! I've picked up some similar changes from an issue @warrenbuckley created a while back re removing underscore, so have been chipping away at killing off _.each. Long story short hopefully we don't step on each other's toes.

I'll link the PRs here when I'm not on my phone. Thinking maybe a parent issue to link the angular and underscore stuff might be useful too

@kjac
Copy link
Contributor

kjac commented Jul 29, 2020

@nathanwoulfe I saw the PRs - #8475 and #8490 ... I think we're good 👍 the _.each-removal doesn't seem to interfere much if at all with the angular.forEach removal. And if it does, well... it should be merge-able for sure.

@nathanwoulfe
Copy link
Contributor Author

@kjac sounds good to me 👍👍

@kjac
Copy link
Contributor

kjac commented Jul 31, 2020

#8538 contains angular.forEach replacement for the remaining property editors.

@kjac
Copy link
Contributor

kjac commented Aug 1, 2020

#8552 contains angular.forEach replacement for filters and services

@emma-hq
Copy link
Contributor

emma-hq commented Mar 16, 2021

Aw this issue!!! I love it. Brings back some nice memories from last year and we all know they're in short supply. We have only one more to do and then PROFIT! 😆

Anyone from this team want to pick up .extend()? Go on. Go on go on go on go on go on.

Em

@bjarnef
Copy link
Contributor

bjarnef commented Mar 21, 2021

@emma-hq I couldn't find an open PR to replace angular.extend() so I have submitted a PR here: #10023 😁 👍

@emma-hq
Copy link
Contributor

emma-hq commented Mar 22, 2021

Aw thank you @bjarnef - you're a ⭐

@nathanwoulfe
Copy link
Contributor Author

CLOSED! Finally!

Thanks to all who contributed PRs/thoughts/prayers 😄

@nathanwoulfe
Copy link
Contributor Author

#10759 resolves some sneaky angular.forEach usages which managed to weasel their way back in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment