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

deps: bump dependencies to latest #3552

Merged
merged 16 commits into from Mar 5, 2019

Conversation

jniles
Copy link
Contributor

@jniles jniles commented Jan 22, 2019

This PR bumps all dependencies to their latest version, except ui-router, which has major breaking changes for both an upgrade and a downgrade.

The major changes are:

  1. We removed all @bower_components and replaced them with their native NPM counterparts. This allows green-keeper to keep us up to date with different versions. The one exception to this is angular-ui-router, as mentioned above.
  2. gulp has been upgraded to version 4! If there was a performance increase, it was small. However, this allows us to rewrite our gulpfile for better clarity.
  3. A ton of unused CSS has been pruned. We don't use it anymore, so let us remove it.
  4. All vendor references have been removed. We no longer use a vendor folder without bower.

Super awesome changes:

  1. When using NODE_ENV=development, the application bundles the unminified dependencies. That means we have much better insight into what bugs are happening and less cryptic error message.

@jniles
Copy link
Contributor Author

jniles commented Jan 24, 2019

I jumped the gun a bit, and we are being blocked now by angular/angular.js#16814. We'll see if they release a new version with the bug patched soon.

@jniles jniles force-pushed the chore-bump-angular-deps branch 2 times, most recently from e9486fd to a8fe7a8 Compare February 4, 2019 13:49
@jniles jniles force-pushed the chore-bump-angular-deps branch 2 times, most recently from 2db4e3b to 97bf5dc Compare February 5, 2019 13:24
Angular just released a bugfix build and we've upgraded to it.  We've
also taken the opportunity to officially bump the rest of the depends up
to a supported version.
BREAKING CHANGE: a lot of third party files have been removed and put
into node_modules so they can be downloaded instead of versioned in our
repository.  This breaks all developer builds until you download and
re-install node_modules.

The gulp build is cleaned up to remove files that were never accessible,
adjust paths away from @bower_components to their npm version, and
finally remove fonts from assets so they are also downloaded with yarn.
This makes it easier to rationalize the package.json and to read
the gulpfile.  It also makes future upgrades to gulp (v4) easier to
manage.
The build now minifies all CSS in production mode.  Additionally, the
issues with the weird animations have been solved by concatenating the
scripts before minification, rather than after, so that the minifier
doesn't create variable collisions.  Finally, the ui-grid fonts now have
a special gulp task because they are referred to as specially in the css
directory.  They have a special place in our hearts.
This commit links the barcode and CSS to make sure wkhtmltopdf doesn't
report a network error when rendering.  Integration tests now pass.
This commit upgrades the gulpfile to use Gulp Version 4.

Closes IMA-WorldHealth#3327.
Adds the $applyAsync to tease out a performance boost on modules with
many $http queries.
Updates to an angular version that actually works! :).  Also fixes some
typos in the build script and test script.
This commit fixes the modal by removing the appended classes from the
ui-selects in the account reference modal.  We needed to work around
this by clicking to clear away from the context.
The ui-router master branch changes the order in which states are
entered, which in turn breaks a lot of BHIMA.  We'll need to do some
major adjustments to our routing to get up to the latest and greatest
ui-router.
The multiple depot component is no longer appended to the body to
prevent issues with testing.
The bhMultipleCashboxSelect append-to-body tag has been removed to
facilitate testing.
@jniles
Copy link
Contributor Author

jniles commented Feb 27, 2019

@mbayopanda, this is also finally ready for review.

Copy link
Contributor

@mbayopanda mbayopanda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a comment about the angular-file-upload-bower dependency.

And also, i have got this error with font-awesome :

image

@@ -203,6 +203,9 @@ function httpConfig($httpProvider) {

// register error handling interceptor
$httpProvider.interceptors.push('ErrorInterceptor');

// perf - applyAsync
$httpProvider.useApplyAsync(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this ?

I found this comment

Copy link
Contributor Author

@jniles jniles Mar 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear if this will help or not, but I can imagine three places where this will help:

  1. Typeaheads/validators (such as the hospital_no) will trigger less digest calls. We use these quite a bit in our application, shipping lots of small requests rapidly over a local network.
  2. Modules where we add new grid rows to Stock/Vouchers/Transactions that prompt HTTP requests. For example, when we add voucher tools, those often trigger several HTTP requests from the Voucher module. This will stack those requests to make the browser do fewer $digest calls.
  3. Each ui-grid cellTemplate seems to send an HTTP request. I'm not sure why - maybe we need to debug this somewhere, but for the moment, this might help.

Again, this might be a bad idea, but we have to try it to find out... from the documentation, it doesn't look like it will hurt anything, just might not help as much as we hope.


// default time to live of 3 seconds
var TTL = 3000;
const TTL = 1000000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-) i think the comment must also be updated.

A curiosity question, why this decision of using 1000 seconds now ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, you are right. This is a mistake, and I'll revert it.

"angular": "angular/bower-angular#1.7.5",
"angular-animate": "angular/bower-angular-animate#1.7.5",
"angular-dynamic-locale": "lgalfaso/angular-dynamic-locale#0.1.37",
"angular-file-upload-bower": "danialfarid/angular-file-upload-bower#12.2.13",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need the angular-file-upload-bower anymore ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. It has been renamed ng-file-upload and is still included in the package.json file.

This is the old repository. You can in the README that they link to "angular-file-upload", but that it redirects to ng-file-upload.

And ng-file-upload is in line 92 of this file.

@@ -296,7 +296,6 @@ function document(req, res, next) {
return item;
});

// console.log(assetTable);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -32,7 +32,7 @@ startfold "Running server Unit Tests ......" "server-unit"
endfold "server-unit" ;

# run end to end tests
startfold "Running Client Unit Tests..." "test-end-to-end";
startfold "Running Client End to End Tests..." "test-end-to-end";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 good catch

@jniles
Copy link
Contributor Author

jniles commented Mar 4, 2019

Thanks for the thorough review @mbayopanda.

About snyk - I'm not sure what is going on there, although I remember some issue with snyk in the past that was similar. Do the font-awesome icons load when you launch BHIMA?

@mbayopanda
Copy link
Contributor

I cannot run the application correctly :
image

@mbayopanda
Copy link
Contributor

With yarn there is not any errors during the yarn command

image

@mbayopanda
Copy link
Contributor

Everything seems to work correctly and also the file upload

@mbayopanda
Copy link
Contributor

Could you fix the mergin conflict so that i can merge it

@kwilu kwilu added the size: XL label Mar 5, 2019
@mbayopanda
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Mar 5, 2019
3552: deps: bump dependencies to latest r=mbayopanda a=jniles

This PR bumps all dependencies to their latest version, except ui-router, which has major breaking changes for both an upgrade and a downgrade.

The major changes are:
 1. We removed all `@bower_components`  and replaced them with their native NPM counterparts.  This allows green-keeper to keep us up to date with different versions.  The one exception to this is angular-ui-router, as mentioned above.
 2. `gulp` has been upgraded to version 4!  If there was a performance increase, it was small.  However, this allows us to rewrite our gulpfile for better clarity.
 3. A ton of unused CSS has been pruned.  We don't use it anymore, so let us remove it.
 4. All `vendor` references have been removed.  We no longer use a vendor folder without bower.

Super awesome changes:
 1. When using `NODE_ENV=development`, the application bundles the unminified dependencies.  That means we have much better insight into what bugs are happening and less cryptic error message.

Co-authored-by: Jonathan Niles <jonathanwniles@gmail.com>
@bors
Copy link
Contributor

bors bot commented Mar 5, 2019

Build failed

@mbayopanda
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Mar 5, 2019
3552: deps: bump dependencies to latest r=mbayopanda a=jniles

This PR bumps all dependencies to their latest version, except ui-router, which has major breaking changes for both an upgrade and a downgrade.

The major changes are:
 1. We removed all `@bower_components`  and replaced them with their native NPM counterparts.  This allows green-keeper to keep us up to date with different versions.  The one exception to this is angular-ui-router, as mentioned above.
 2. `gulp` has been upgraded to version 4!  If there was a performance increase, it was small.  However, this allows us to rewrite our gulpfile for better clarity.
 3. A ton of unused CSS has been pruned.  We don't use it anymore, so let us remove it.
 4. All `vendor` references have been removed.  We no longer use a vendor folder without bower.

Super awesome changes:
 1. When using `NODE_ENV=development`, the application bundles the unminified dependencies.  That means we have much better insight into what bugs are happening and less cryptic error message.

Co-authored-by: Jonathan Niles <jonathanwniles@gmail.com>
@bors
Copy link
Contributor

bors bot commented Mar 5, 2019

Build succeeded

@bors bors bot merged commit 1ac2a8e into IMA-WorldHealth:master Mar 5, 2019
@jniles jniles deleted the chore-bump-angular-deps branch March 5, 2019 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants