Use esbuild instead of webpacker #1957
Conversation
| render: function (data, type, row, meta) { | ||
| var api = new $.fn.dataTable.Api(meta.settings); | ||
| let selected = api.rows(meta.row, { selected: true }).count() > 0; | ||
| return `<input type="checkbox" ${selected ? 'checked' : ''}> ${selected ? 'checked' : ''}`; | ||
| } |
There was a problem hiding this comment.
I don't believe this is ever used or is useful. plus it adds the text checked to the innerHtml?
There was a problem hiding this comment.
files app test fine with this removed, so we should be good.
| // FIXME: confim modals don't work in esbuild. | ||
| // import 'data-confirm-modal'; |
There was a problem hiding this comment.
This may actually be OK because it's buggy: #665.
| window.$ = $; | ||
| import jQuery from 'jquery'; | ||
| import 'jquery-ujs'; | ||
| import datatables from 'datatables.net'; |
There was a problem hiding this comment.
We're trying to decouple. Why move the datatables here? if there is a piece of functionality that needs datatable access, shouldn't we do the same as we did in the files app?
There was a problem hiding this comment.
Another thought, by adding datatables import here, we're loading datatables even on pages that do not use datatables.
There was a problem hiding this comment.
That's true, but jquery plugins don't play nice when jquery has a conflict (i.e., some other definition of jquery). #1846 is an example of this. Indeed - tooltip is a global jquery plugin. Along with poppers that we have installed everywhere.
I figure if we buy in on jquery, we buy in on it's plugins too.
I can try to fix this but it didn't work easily so i'll bet we have to use jquery's noConflict.
There was a problem hiding this comment.
No need to fix. I was just asking the question.
|
|
||
| import '../images/OpenOnDemand_powered_by_RGB.svg'; | ||
| import '../images/OpenOnDemand_stack_RGB.svg'; | ||
| datatables(window, jQuery); |
There was a problem hiding this comment.
more coupling? Now, we're having to pass window and jQuery to the datatable. Seems counter-productive to the goal of decoupling?
There was a problem hiding this comment.
This deserves a comment which I'll give it, but this is here to initialize these plugins. They don't directly initialize just by being imported (because of javascript toolchain compilations and how old it is and how it's being invoked).
So I had to initialize them to tie them to this jquery otherwise they never invoke.
| render: function (data, type, row, meta) { | ||
| var api = new $.fn.dataTable.Api(meta.settings); | ||
| let selected = api.rows(meta.row, { selected: true }).count() > 0; | ||
| return `<input type="checkbox" ${selected ? 'checked' : ''}> ${selected ? 'checked' : ''}`; | ||
| } |
There was a problem hiding this comment.
files app test fine with this removed, so we should be good.
Fixes #1825 to move to
esbuildinstead ofwebpacker.I think it offers a few things
I believe this is ready for review. There is one drawback here where webpacker would automatically rebuild the assets whenever you modified them. This is no longer the case. Rails provides a simple
foremanmechanism, but I haven't gotten it to work within OOD, so that's something we'll need to look into. I've providedbin/recompile_jsas a helper to do what's required and tests automatically do what's required.┆Issue is synchronized with this Asana task by Unito