AMBARI-25289 Upgrade Jquery and Bootstrap to latest versions
What changes were proposed in this pull request?
The current version of jquery and bootstrap has vulnerabilities which are then resolved in the latest versions of the libraries. There is a major version upgrade from 1.9.1 to 3.7.1 for jquery and Bootstrap 3 to bootstrap 5 for bootstrap. A lot of breaking changes were introduced between the versions. With this PR, deprecated methods are removed and are replaced with the newly supported methods. The reference has been taken from the changelog of the dependencies.
How was this patch tested?
Made the changes and tested the intended behaviour of the app by running test cases and performing manual checks.
Please review Ambari Contributing Guide before opening a pull request.
@vanshuhassija Great work. Due to many of Ambari's front-end libraries being outdated and no longer maintained, there's a need for gradual iteration and updating to newer versions. This PR is excellent. I'll first test this PR in my environment.
@vanshuhassija After applying this patch, running into errors (OS: Win10 PowerShell, Node: v4.5.0, Yarn: v0.23.2), seems to encounter some minor issues. Below are the steps I followed:
-
cd ambari-web -
yarn install --ignore-engines --pure-lockfile -
yarn build(use the script brunch build specified in package.json). - Use Live Server to serve the
publicdirectory.
Encountered errors include:
Uncaught ReferenceError: Backbone is not defined
...
...
Uncaught SyntaxError: Identifier 'tooltipTriggerList' has already been declared
The second error is likely caused by the similarity in content between ambari-web/vendor/scripts/bootstrap-popover.js and ambari-web/vendor/scripts/bootstrap-tooltip.js.
@zRains I have upgraded the Backbone to latest version now. Can you please recheck once?
@vanshuhassija Your PR still has some issues, and I'm not sure if they only occur when using Yarn separately. Here are some notable problems:
-
backbone.js isn't placed in the vendor folder. According to the configuration in the brunch file
'javascripts/vendor.js': /^vendor/, it won't be included in the final build result(vendor.js), leading to the Uncaught ReferenceError: Backbone is not defined error still occurring. -
The issue of vendor/scripts/bootstrap-popover.js and vendor/scripts/bootstrap-tooltip.js having identical content persists. Is this in reference to bootstrap-popover and bootstrap-tooltip?
-
There's a misalignment in the UI after running:
You can build and preview it follow the ambari-web/pom.xml and serve public folder by vite, my vite.config.js:
import { defineConfig } from 'vite'
export default defineConfig({
server: {
proxy: {
'/api': {
target: 'http://Your server addr',
changeOrigin: true
},
'/api/stomp/v1/websocket': {
target: 'ws://Your server addr',
changeOrigin: true,
ws: true
}
}
}
})
Can the front-end upgrade the charts to the latest ones? The old charts have outdated colors and can support more graphics, making page customization more flexible
@smileyboy2019 Upgrading the charts should be done after upgrading jQuery and other basic dependencies. IMO, both Node ^4.0 and Brunch.js are no longer maintained. It need to upgrade or replace dev/dependences, or, it will spend a significant amount of time dealing with compatibility and dependency issues.
@vanshuhassija Could you please split this commit into two PRs(Jquery upgrade/Bootstrap upgrade)? It would be very helpful for the code review.
@zRains The libraries are tightly coupled with each other. Upgrade of jquery would require upgrade of bootstrap as with the newer version of jquery, old bootstrap is not supported.
@zRains The testing team in the organisation has pointed out some UI deviations as well which will be raised as tickets on the board once this PR is merged.
@vanshuhassija Could u resolve conflict?
@vanshuhassija Could you please resolve this conflict first? After that, other community members can continue to push forward with the work on upgrading the frontend components.
@JiaLiangC The PR has no conflicts with the base branch. Any specific conflict you are referring to?
@JiaLiangC The PR has no conflicts with the base branch. Any specific conflict you are referring to?
@vanshuhassija I have already manually resolved the conflict.
@vanshuhassija Regarding this PR, I have concerns because it involves extensive changes. After applying this PR in the test environment, I noticed several bugs in the frontend. I'm worried that if we merge it, the community might not have enough frontend developers and contributors to address these issues. This could lead to many bugs remaining unresolved before the release. That's my main concern.
Hi @vanshuhassija ,
thanks for reporting this and working on this which is much awaited one.
Can you please resolve the commits.? Changes are looks to good to me apart from the concerns raised already here.
As this was gone through the cycles of review, I am thinking we can commit now and raise the follow up jira's.
@JiaLiangC and @zRains any thoughts on this.?
@brahmareddybattula resolved the conflicts
@vanshuhassija Thank-you for providing fix for JQuery and Bootstrap upgrade fixes seems to have been long pending. changes look good to me +1 can you check the PR build failure ?
@brahmareddybattula Sounds good. The main issue seems to be that the test cases for ambari-web have not yet been modified (144 failed test cases). This can be addressed gradually.
@vanshuhassija Please fix the CI failure. After it's fixed, I'll merge it. As @zRains mentioned, any remaining issues will be addressed in future PRs.
@JiaLiangC @brahmareddybattula @zRains @vishalsuvagia CI is now passing. Had to skip some test cases because of the known issues to be addressed in upcoming PRs. The test skip changes will be reverted with the upcoming fixes.
@JiaLiangC @brahmareddybattula @zRains @vishalsuvagia CI is now passing. Had to skip some test cases because of the known issues to be addressed in upcoming PRs. The test skip changes will be reverted with the upcoming fixes.
Thanks for your excellent work!
@JiaLiangC
LGTM. As discussed above, if there are no other objections, it will be merged in 24 hours. Any potential issues will be addressed in subsequent PRs.
@JiaLiangC Thanks
Can you help me take a look at this issue? It seems to be a front-end problem。 (https://issues.apache.org/jira/projects/AMBARI/issues/AMBARI-26148?filter=allissues)
@brahmareddybattula @vanshuhassija thi pr has introduced almost 30 bugs, many of which are critical, causing major functionalities to break and disrupting most of the unit tests. They haven't been fixed yet. Here is the bug list:https://issues.apache.org/jira/browse/AMBARI-26237. Could you take a look, and help fixing them?