futurism icon indicating copy to clipboard operation
futurism copied to clipboard

GlobalID errors

Open Matt-Yorkley opened this issue 3 years ago • 13 comments

Bug Report

Describe the bug

Trying out Futurism on a fresh app I'm getting fatal errors that seem to be a class-loading issue with GlobalID.

To Reproduce

  • Create a fresh app with Rails 7.0.1 and Ruby 3.0.3
  • Add Futurism
  • Try futurizing a partial
NameError - uninitialized constant Futurism::Resolver::Resources::GlobalID

The backtrace points to:

[...]/3.0.0/gems/futurism-1.2.0.pre9/lib/futurism/resolver/resources.rb:92:in `resolved_models' 

I notice in my Gemfile.lock I have globalid at version 1.0.0, which was added by default. If I downgrade and manually pin it to <= 0.6.0 instead, the issue is resolved.

Versions

Futurism

  • Gem: 1.2.0.pre9
  • Node package: 1.2.0-pre9

External tools

  • Ruby: 3.0.3
  • Rails: 7.0.1
  • CableReady: 5.0.0-pre8

Matt-Yorkley avatar Feb 08 '22 23:02 Matt-Yorkley

Huh that is weird, I haven't run into this in spite of doing a couple of fresh installs lately.

Will investigate...

julianrubisch avatar Feb 09 '22 05:02 julianrubisch

It's even more weird because the GlobalID changelog mentions no code changes between 0.6.0 and 1.0.0 🤔

julianrubisch avatar Feb 09 '22 05:02 julianrubisch

Results of debugging round two: it actually does work with 1.0.0, but only when I have globalid explicitly declared in my Gemfile. If I remove it (even though it's still in my Gemfile.lock and apparently present) I get the error. :thinking:

Matt-Yorkley avatar Feb 09 '22 08:02 Matt-Yorkley

Oh... imho that's even stranger... could you prepare an MVCE?

julianrubisch avatar Feb 09 '22 09:02 julianrubisch

Ahaaaa... okay I think I got it. Results of debugging round three:

The app I was working on was an MVP so I skipped a lot of modules when doing rails new.

Looking at the Gemfile.lock at which Rails modules use/depend on global_id, I can see it's action_text and active_job. I skipped / disabled both of those when creating the app.

Re-enabling either of those two core modules resolves the issue; because both of them explicitly contain require statements for global_id but Futurism doesn't.

Matt-Yorkley avatar Feb 09 '22 11:02 Matt-Yorkley

Great find 👏🏻

I think this would have surfaced earlier if futurism only included the respective rails submodules - which I intended to do, too 🙈

julianrubisch avatar Feb 09 '22 12:02 julianrubisch

Cf https://github.com/stimulusreflex/cable_ready/pull/175

julianrubisch avatar Feb 09 '22 12:02 julianrubisch

Confirmed adding require "global_id" in here resolves it: https://github.com/stimulusreflex/futurism/blob/8d352a54a3f35bc2f7856774d6edefbd521d4fbf/lib/futurism.rb#L1-L3

Shall I PR it? It's a dependency of rails already, so it doesn't feel like it warrants a change in the gemspec. It just doesn't necessarily get required.

Matt-Yorkley avatar Feb 09 '22 14:02 Matt-Yorkley

yeah, the question I'd like to pose is if we should actually move away from requiring the whole of rails and just require the necessary gems...

there were some second thoughts in CR that led to https://github.com/stimulusreflex/cable_ready/pull/175, thus excluding ActionMailbox from the slug size, for example...

julianrubisch avatar Feb 09 '22 14:02 julianrubisch

I think it makes sense to only have dependencies on global_id (and require it) and cable_ready. We also have some dependencies on the controller code, activerecord, etc.

Do you think this list would do it?

  gem.add_dependency "actioncable"
  gem.add_dependency "actionpack"
  gem.add_dependency "actionview"
  gem.add_dependency "activerecord"
  gem.add_dependency "activesupport"
  gem.add_dependency "railties"
  gem.add_dependency "global_id"
  gem.add_dependency "cable_ready"

rickychilcott avatar Feb 10 '22 14:02 rickychilcott

Yep. I'm not sure if ActiveModel should be in there. It's probably in every application anyway so we could just make it explicit?

julianrubisch avatar Feb 10 '22 14:02 julianrubisch

Is Futurism usable in non-Rails projects though? If it's not, then this extra granularity in the gemspec is moot, right?

Matt-Yorkley avatar Feb 10 '22 14:02 Matt-Yorkley

Not really, as pointed out in the referenced CR PR, because it blows up the memory slug size when you include action_mailbox and action_text by default, for example.

julianrubisch avatar Feb 10 '22 14:02 julianrubisch