inertiajs / inertia-rails

The Rails adapter for Inertia.js.
https://inertiajs.com
MIT License
529 stars 43 forks source link

BUG: Asset versioning broken since v1.4.0 #45

Closed ledermann closed 4 years ago

ledermann commented 4 years ago

38 introduced a serious bug within InertiaRails.version: It returns nil for every server response, so asset versioning is broken, which means the frontend will not be notified if the assets have changed.

Why? To me it seems to be caused by the thread-safe change. Now there is threadsafe_version, so changing the version in one thread is not shared with other threads. This seems to be intended, because there is an explicit test for this.

IMHO the thread handling is not correct. Typically, the version is set just one time within an initializer. Using a multi threaded application server like Puma spawns some new threads. So far as I know, spawning a new thread does not run the initializers again, so the Thread.current does not include the thread_safe_version for all running threads.

Try this is out in a Rails application using inertia_rails v1.4.0:

rails c
(irb)> InertiaRails.version.inspect
=> "1.0"
(irb)> thread = Thread.new { p InertiaRails.version.inspect }
"nil"

IMHO, the layout config has the same issue, but because it has a fallback, it will never be nil.

bknoles commented 4 years ago

thanks for finding this @ledermann ... the configuration in general is not super great. as you alluded to, mixing initializer configuration with runtime configuration is probably just not the right strategy. #37 is related to this.

@BrandonShar i think we need to take a step back and reconsider how we specify configuration, but in the meantime, let's merge the PR to address this and cut a new version... sound good to you?

taylorchu commented 4 years ago

it also happens for layout. we have a different value for inertia layout config.