salesagility / SuiteCRM

SuiteCRM - Open source CRM for the world
https://www.suitecrm.com
GNU Affero General Public License v3.0
4.39k stars 2.06k forks source link

Unminify JavaScript in the SuiteCRM repository #7333

Open connorshea opened 5 years ago

connorshea commented 5 years ago

Earlier today I opened #7332 to fix some minified JavaScript that was in the FP_Events module, and that made me curious to see if there were any other files that were commited to the git repo with minification. It looks like there are quite a few.

A lot are in the include/ directory, and I won't include them in this list just because them being minified sort of makes sense (we should really still fix it, though).

For example

Note: I found these by searching for var in VS Code and then looking through the results for lines that looked like var this;var that;var them;var spaghetti;. This list probably isn't comprehensive.

Most (all?) of these are from the 'Initial Commit' to the repo 6 years ago. I'm not sure if the original SugarCRM files exist somewhere that we can use, or if they were minified in the Sugar repo as well. Assuming we can't recover them from a copy of the Sugar CE repo, the best solution would probably be to do what I did in #7332 and just reformat the files using Prettier.

Dillon-Brown commented 5 years ago

Agreed, ideally we shouldn't be committing generated source files. I think we'd need to add rebuilding the minified JS to the installer or have something in the command line so you can pull down and install from the repo.

connorshea commented 5 years ago

Interestingly, most of the files have counterparts in the jssource directory, but not all of them. I formatted most of the files in my pull request, but I'm not sure if it makes sense to even merge the PR given that the source files are already in the repo.

@Dillon-Brown would changing the installation system to minify the JS be particularly difficult? That'd allow us to just remove most of this code.

Dillon-Brown commented 5 years ago

@connorshea I imagine it shouldn't be too difficult. We already have all the functionality to do this already, just need to be called during installation. It looks like there are around 613 files to remove here :O

connorshea commented 5 years ago

O.O