Start prohibiting jQuery in Vue code #22353
Conversation
cconard96
changed the title
| "selector": "CallExpression[callee.object.name='$'][callee.property.name=/^(ajax|get|post)$/]", | ||
| "message": "Use axios for HTTP requests instead of jQuery AJAX methods." | ||
| }, | ||
| // { |
There was a problem hiding this comment.
To be uncommented in later PRs
| customfielddefinitions_id: custom_fields_id, | ||
| action: 'purge_field' | ||
| }, { | ||
| headers: { 'Content-Type': 'application/x-www-form-urlencoded' } |
There was a problem hiding this comment.
Axios defaults to JSON while jQuery defaulted to "application/x-www-form-urlencoded". To avoid needing to change server code, the content type is manually changed in each request. JSON would likely be better in the future, which is why this header isn't added in the axios instance.
| }); | ||
| // Add axios interceptors | ||
| axiosInstance.interceptors.request.use((config) => { |
There was a problem hiding this comment.
Near-copy of the jQuery ajaxSend and ajaxComplete handlers for recording requests sent by Axios.
js/src/vue/FuzzySearch/Modal.vue
Outdated
| all_menus.value = menus; | ||
| }); | ||
| } | ||
| const fuzzy_search = $('#fuzzysearch'); |
There was a problem hiding this comment.
Replaced by wrapping the component in a Teleport which makes Vue automatically "teleport" the component to another place in the DOM.
| const instance = axios.create({ | ||
| baseURL: CFG_GLPI.root_doc, | ||
| headers: { | ||
| 'X-Requested-With': 'XMLHttpRequest', | ||
| 'X-Glpi-Csrf-Token': window.getAjaxCsrfToken(), | ||
| } | ||
| }); | ||
|
|
||
| export function useAJAX() { | ||
| return { | ||
| ajaxGet: (url, config = {}) => instance.get(url, config), | ||
| ajaxPost: (url, data = {}, config = {}) => instance.post(url, data, config), | ||
| ajaxPut: (url, data = {}, config = {}) => instance.put(url, data, config), | ||
| ajaxDelete: (url, config = {}) => instance.delete(url, config), | ||
| axiosInstance: instance, | ||
| }; | ||
| } |
There was a problem hiding this comment.
IMHO, we should reexport the axios object itself, to be able to use it with import axios from './axios'; in our modules. Developers would just have to know that we have a specific import location for this module, and that it defines some common default values, but would be able to use it the same way as in any other project using it.
Defaults can be configured with
axios.defaults.baseURL = CFG_GLPI.root_doc;
axios.defaults.headers.common['X-Requested-With'] = 'XMLHttpRequest';
axios.defaults.headers.post['X-Glpi-Csrf-Token'] = window.getAjaxCsrfToken();There was a problem hiding this comment.
I think there are separate concerns here between Vue code and our regular ES modules. This composable will evolve with the Vue ecosystem and state management in mind. I am not sure it is something that should be started in this PR for code outside Vue.
Perhaps with GLPI 12 we can consider bundling our own plain ES module code together and then decide how to handle Axios in that context. f there is a ES script that re-exports Axios, it wouldn't be reliably usable everywhere as there are still non-module scripts loaded and inline on the page which would not have access. Even if the module had a side-effect of putting Axios on the window, it may not be available by the time the non-modules run.
Keeping the separation of Vue=Axios and everything else=jQuery AJAX may make it easier for other developers to understand as well in the short-term.
There was a problem hiding this comment.
We could keep it only for vue modules. My concern is about having specific methods naming that requires developers to learn how they behave while they probably already know how to properly use axios.
There was a problem hiding this comment.
I think this could eventually be replaced with the composable from VueUse which is a widely used set of utilities. I was just waiting for there to be more than one or two things I knew we could use it for before proposing adding it instead of having simple implementations of what is needed currently.
2 participants
Checklist before requesting a review
Description
Modern code deserves modern solutions.
Only affects Vue code at this time and is not completely done by this PR due to the number of changes.
fetchbut with some helpful functionality added like automatic JSON parsing and cleaner error handling..vuefilesAdditional PRs can address the remaining
$.eachand$(selector|HTML)usages.