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
[Vue] add promise api to ajaxHelper and deprecate piwikApi service #18114
Conversation
Also migrated piwik-helper.spec.js + renamed piwik => matomo in as many places as possible in new code. |
Adding DOM sanitizer library as well so future PRs can just use it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to merge @diosmosis . Left a comment to maybe check if it makes sense to print an output in a command.
Description:
This PR is based off of #18106
I looked into migrating the piwikApi service, but since it's API is dependent on the angular library, it seemed too much work to create a different implementation, then an adapter for existing angularjs code. Instead I opted to expose the jqXHR object from jQuery in AjaxHelper, which gives it a promise API. New code in Vue will use this API, old jquery specific code can use the callback version of ajaxHelper, and old angularjs code can use piwikApi until Matomo 5. This seemed like the simplest solution and should have no BC breaks.
I also looked at using browser native
fetch()
function w/ a polyfill (using github's fetch polyfill) in AjaxHelper so we wouldn't have to use jQuery. The API, unfortunately, is too different from jQuery's, so it would be very difficult to provide BC. By Matomo 5, it could be possible to switch the implementation away from jQuery if desired.New Vue code should use ajaxHelper's promise API.
DOM Sanitization
Also added a new library to sanitize HTML when migrating uses of
ng-bind-html
since Vue doesn't include one by deafult. Using https://github.com/cure53/DOMPurify, tested with ExampleVue, and it works. ng-bind-html/v-html should ideally not be used, but that would be a large undertaking.Note: jump in size is due to dom sanitizer. Unavoidable, but when angularjs is removed, the file size will go down.
Review