ditch webpacker in favor of jsbundling/esbuild #2594
Conversation
… in esbuild yet though
…w up right again, then completely nuke webpacker
…nt into colin/jsbuild-better
| @@ -113,3 +113,6 @@ config/initializers/secret_token.rb | |||
| yarn-debug.log* | |||
| .yarn-integrity | |||
| yarn-error.log | |||
|
|
|||
| /app/assets/builds/* | |||
There was a problem hiding this comment.
result of rails:javascript:install
| @@ -0,0 +1,3 @@ | |||
| web: bin/rails server -p 3000 | |||
There was a problem hiding this comment.
Generated by rails:javascript:install; this is what bin/dev executes in three different processes
| @@ -1,4 +1,3 @@ | |||
| //= link_tree ../images | |||
| //= link_directory ../javascripts .js | |||
There was a problem hiding this comment.
Since these are now handled by cssbuild/jsbuild, they're packed up by those process and we don't need to include them in the manifest any longer. Sprockets still packs them up and delivers them though.
| @@ -0,0 +1,46 @@ | |||
| // Autosave on patient forms. | |||
There was a problem hiding this comment.
Converted these to es6 finally.
| @@ -0,0 +1,4366 @@ | |||
| // NOTE: This was generated from https://jqueryui.com/download | |||
There was a problem hiding this comment.
If we didn't vendor this the imports work really weird.
…r code, so simply cheat and always return 200. I know this sucks and I have decided I'm okay with it
colinxfleming
changed the title
| @@ -0,0 +1,25 @@ | |||
| // Major library config | |||
There was a problem hiding this comment.
This is basically a recreation of application.scss in webpacker, but also resolves the awful split we had between sprockets and npm.
| @@ -72,7 +72,6 @@ def update | |||
| else | |||
| error = @patient.errors.full_messages.to_sentence | |||
| flash.now[:alert] = error | |||
| response.status = :not_acceptable | |||
There was a problem hiding this comment.
this (and the few similar changes) are, I think, because UJS had an update such that it doesn't fire templates on 4XX response codes. So we (unfortunately) treat failures to save as 200s now. it is horrible but we aren't interacting with this through, like, a json api so I think it's okay. I promise I feel an appropriate amount of bad aboutit.
| class: [fontawesome_kit, "fa-#{event.icon}", 'event-item']), | ||
| entry_text(event) | ||
| ], ' ' | ||
| def fontawesome_classes(event) |
There was a problem hiding this comment.
no idea why this broke, but 🤷 more of this logic is in the view file now.
| @@ -1,4 +1,5 @@ | |||
| // For making bootstrap utilities available. | |||
| @import 'bootstrap/scss/bootstrap'; | |||
| // @import "rails_bootstrap_forms"; | |||
There was a problem hiding this comment.
Intentionally leaving this commented out right now. Eventually we'll be able to import it more cleanly but for right now it doesn't seem to make that much of a difference, either in terms of display or interaction.
|
Did a quick pass through, seems legit but honestly a PR with this many changes is hard to review. I would appreciate some documentation about this somewhere, ideally in this PR, but if not here then in a follow-up. This would be relevant for me personally, to get #2580 review-able and merge-able. |
lomky
changed the title
I rule and have completed some work on Case Manager that's ready for review!
Since webpacker 5 is no longer compatible with rails 7 on heroku, it's time to rip the bandaid off. Webpacker has unfortunately been rather a mess for us since we moved onto it, and so for better or worse we're going toward the new approach, jsbundling.
Roughly the way jsbundling works is that it runs
yarn build --watchas part of rails server, kinda like how webpacker ran something similar. But it's much more railsy in that it makes a lot of assumptions about how your project is laid out and what you're trying to do, most importantly including js transpilation.Because the webpack move (several years ago now!) was so rough there were still a few things in sprockets, including (most egregiously) dupe copies of bootstrap, jquery, jquery-ui, and some coffeescript that never got converted cleanly. Pleased to say this PR resolves all of those duplicate resources.
As @xmunoz noted this PR is an unreviewable mess. It's not fair to ask anyone to review 67 files straight up. I think we need to take this on the faith of our test suite that this works okay, and declare a new beginning.
There are a few followup things we need to do that I'll make issues for:
bin/devwhen running bare metalThis pull request makes the following changes:
no view changes
It relates to the following issue #s:
For reviewer:
featureif it contains a feature, fix, or similar. This is anything that contains a user-facing fix in some way, such as frontend changes, alterations to backend behavior, or bug fixes.dependenciesif it contains library upgrades or similar. This is anything that upgrades any dependency, such as a Gemfile update or npm package upgrade.