ocket8888 opened a new pull request, #7219: URL: https://github.com/apache/trafficcontrol/pull/7219
This PR adds types to the "Service Utilities". Those being: - CollectionUtils - DateUtils - DeliveryServiceUtils - FileUtils - FormUtils - LocationUtils - NumberUtils - PermissionUtils - ServerUtils - StringUtils - TenantUtils - TopologyUtils I refactored all of those to be proper ES2015 classes instead of using the pseudo-class function-with-a-namespace pattern that was common in the early 2010's. They also now all have strongly typed JSDoc annotations. I then added type annotations to the places they were injected, and removed any unused injections I found. I also added some development dependencies on typings for our libraries: AngularJS, jQuery, and MomentJS. One of those services injected `$uibModal`, which is provided by the `angular-ui-bootstrap` package. I tried really hard to get the typings from `@types/angular-ui-bootstrap` to work, but I think they are actually just broken, and if that's the case they'll probably never be fixed since that's a supplemental package for an abandoned extension to an end-of-life framework. So I copied the relevant portions of the typings into a place from which I can actually import, slightly modified them, added a notice about the license and location of the original code, and used that. I did not try to add typings for any of these libraries to any places I wasn't already editing to add service typings, because that would be perhaps a thousand files. As it is, this touches 187 files, which is quite a lot. But just under half of that is from a single service (`LocationUtils`) so I don't really think breaking it apart by service helps too much, because that's still massive. Plus in a most places they were injected, multiple of them were injected, which means a lot of duplicated tiny changes that would be an absolute nightmare to reconcile one at a time, I'd imagine. Finally, while updating dependencies I noticed that we were including two libraries but not using either of them (assuming the tests pass and prove me right) so I removed them, which was an 8-line change across a gulp config, a "shared libraries" file, and our package.json. Naturally this and the other dependency changes come out to 1.6k lines changed in `package-lock.json`, so keep that in mind. <hr/> ## Which Traffic Control components are affected by this PR? - Traffic Portal ## What is the best way to verify this PR? There should be no behavior changes, so all tests should still pass. As far as checking the correctness of typings, I just used my editor's built-in type checking to ensure things were working (UnderscoreJS/LoDash still has no typings since I hope to just get rid of them some day - I replaced some trivial uses with ES built-ins but they still cause issues in a few files where there isn't just an equivalent `Array.prototype` method). You could, theoretically, use a TypeScript compiler to check the files, something like maybe ```bash tsc --allowJs --allowUmdGlobalAccess --baseUrl . --checkJs --noEmit --noImplicitReturns --noImplicitThis --strictNullChecks --strictPropertyInitialization --target ES6 app/src/common/service/utils/*Utils.js ``` or using an equivalent `jsconfig.json` project file. But that only checks the typings within the utilities themselves, to check the typings on their usages like I did you have to expand the scope to include all of the files where they're referenced. Which means either including the entire TP project and seeing a billion errors I didn't fix (yet?), or figuring out where all of those references are and checking one-by-one. Frankly I wouldn't bother with that. I'd probably only check the typings in the actual utilities by opening them in my editor and not worry about the usages. Even if the typings are broken, that only affects developers and they can just fix it when it bites them. It's much more important that functionally there is no impact. ## PR submission checklist - [x] This PR uses existing tests - [x] This PR is documentation, more or less - [x] This PR doesn't need a CHANGELOG.md entry - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
