-
Notifications
You must be signed in to change notification settings - Fork 74
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
🔥 Remove webpacker #3848
🔥 Remove webpacker #3848
Conversation
Removing webpacker easier than upgrading it, you are full of surprises. It is cool when it is about removing. |
e3706e1
to
9ed8a1b
Compare
895708f
to
f482902
Compare
f482902
to
732ba53
Compare
5c46503
to
d436efb
Compare
8b863a2
to
d38dd1e
Compare
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.
I'll take a better look to this tomorrow, but for now my question is: why not switching to jsbundling-rails instead of this? It's the official replacement for Webpacker in Rails 7 (announcement)
Guide: https://github.com/rails/jsbundling-rails/blob/main/docs/switch_from_webpacker.md
What I don't like about this approach is we might be inventing the wheel and implementing a homebrew solution, this has several problems. For instance, having to maintain it. A gem like webpacker gets updated, the CVEs are solved, etc. In this case we should do all of this ourselves, and we probably won't. Also it might require some extra work when we want to upgrade Rails.
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.
This works fine for me locally. Good job. A few comments though:
- I see many compile warnings about conflicting order, why is that?
- I tried to run this branch with
RAILS_ENV=production
and I can't load any screen because the fileerrors.css
is being requested as mimetype 'text/css' but the servers returns it as mimetypetext/html
, which causes and error 500 for some reason.
c7a72cb
to
d7135cd
Compare
d38dd1e
to
a7826ba
Compare
e7874fb
to
5ea1092
Compare
5ea1092
to
c5eb5ca
Compare
c5eb5ca
to
2116578
Compare
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.
Just some minor comments which aren't blockers. Besides that, I'm excited with the changes introduced here! 🚀
I've been testing it after compiling assets running |
e8970d8
to
d47b85d
Compare
1d21349
to
4b8b566
Compare
I've been checking @mayorova pages that were failing in production earlier, and the only one still failing is the SSO Integrations edit form, which fails both in I'd say
Besides that, both |
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 great to me! 🔥
Stellar job, @josemigallas 🌟
bdde485
to
68ca266
Compare
THREESCALE-7340: Upgrade to webpack@5
Special thanks to this blog post https://aristo.hashnode.dev/webpacker-to-jsbundling-rails-migration