Upgrade to Rails 7 and latest JS/CSS Tooling #2290
Conversation
ErikSchierboom
force-pushed
the
rails-7
branch
from
92f50ef to
1f958b8
Compare
| @@ -13,6 +13,7 @@ | |||
| "strict": true, | |||
| "noEmit": true, | |||
| "allowSyntheticDefaultImports": true, | |||
| "resolveJsonModule": true, | |||
| "types": ["webpack-env", "app/javascript/declarations"] | |||
| }, | |||
| "exclude": [ | |||
There was a problem hiding this comment.
- Does this minify?
- Does it target the correct version of JS (and/or add polyfill for older browsers)?
- Anything else we should do? :)
There was a problem hiding this comment.
- Yes, as it is just like any other bit of JavaScript (as JSON is JavaScript)
- No polyfill needed, as JSON is just regular JavaScript.
- Nope.
FYI The reason this has to be enabled explicitly is that they want to discourage people from reading big JSON files in their projects (which is usually better done via an XHR request). In our case, it's fine.
There was a problem hiding this comment.
Ah, I meant the whole block here, sorry. I mean is esbuild minifying, targeting the correct version, etc.
|
Next up is getting the tests green I think. |
ErikSchierboom
force-pushed
the
rails-7
branch
2 times, most recently
from
38b4d91 to
3a3c42f
Compare
| authored_exercises = Exercise.all.excluding(iHiD.authored_exercises).sort_by{rand}[0,3] | ||
| iHiD.authored_exercises += authored_exercises |
There was a problem hiding this comment.
This fixes an intermittent duplicate key exception, which was caused by the track syncing happening before this where you were already authoring some exercises. The excluding method is new in Rails 7 BTW: https://blog.saeloun.com/2021/03/08/rails-6-1-adds-excluding-to-active-record-relation.html
db/migrate/20211218160303_remove_not_null_on_active_storage_blobs_checksum.active_storage.rb
Show resolved
Hide resolved
db/migrate/20211218160302_create_active_storage_variant_records.active_storage.rb
Outdated
Show resolved
Hide resolved
db/migrate/20211218160301_add_service_name_to_active_storage_blobs.active_storage.rb
Outdated
Show resolved
Hide resolved
| @@ -276,6 +289,10 @@ class ActionDispatch::IntegrationTest | |||
| include Devise::Test::IntegrationHelpers | |||
| include TurboAssertionsHelper | |||
|
|
|||
| def setup | |||
| host! URI(Rails.application.routes.default_url_options[:host]).host | |||
| end | |||
There was a problem hiding this comment.
@ErikSchierboom Always call super in setup. A load of tests were breaking because the ActiveSupport::TestCase#setup method that we rely on (defined further up) wasn't getting called.
|
|
||
| unless ENV["EXERCISM_CI"] | ||
| listener = Listen.to(Rails.root / 'app' / 'images') { write_files } | ||
| if Rails.env.development? |
There was a problem hiding this comment.
Lol, good thing I didn't suggest this in 4d69274#diff-d4c7ccc36eef0e673446020de7db1e851adc99a30f3a663e8034b05d5e232df8R32 :D
ErikSchierboom
force-pushed
the
rails-7
branch
from
aaa57ef to
59678cf
Compare
| : []), | ||
| ], | ||
| bundle: true, | ||
| sourcemap: true, |
There was a problem hiding this comment.
Do we want sourcemaps in production?
| ? ['./app/javascript/packs/test.tsx'] | ||
| : []), | ||
| ], | ||
| bundle: true, |
There was a problem hiding this comment.
Is bundling something we want in development?
There was a problem hiding this comment.
Yeah, I think so. Let's keep that consistent.
| bundle: true, | ||
| sourcemap: true, | ||
| format: 'esm', | ||
| splitting: true, |
There was a problem hiding this comment.
Is splitting something we want in development?
Erik
Exercism.config.website_assets_host) in the icon/graphical_icon helper files--watchflag foryarn build(there's various articles on this online)iHiD