Guard defining ComponentPreviewController on feature flag

[zachmargolis]

🎫 Ticket

slack discussion of error

🛠 Summary of changes

Why: we avoid loading lookbook gem unless the feature flag is on, so the classes to subclass and the mixins will fail due to load errors

How: I went with the sneaky trailing if to minimize diff noise

📜 Testing Plan

• Deploy to a lower env with the feature disabled (int)

[tomas-nava]

🤯 that's a new trick to me! I might suggest adding a comment to explain why it's there, but functionally looks good to me.

[zachmargolis]

🤯 that's a new trick to me! I might suggest adding a comment to explain why it's there, but functionally looks good to me.

trailing if statements work on blocks! so even each, loop, etc. Is 7ed4d9d what you were looking for?

[aduth]

I'm seeing a different error when testing this locally:

/path/to/gems/3.0.0/gems/zeitwerk-2.6.0/lib/zeitwerk/loader/callbacks.rb:25:in `on_file_autoloaded': expected file /path/to/identity-idp/app/controllers/component_preview_controller.rb to define constant ComponentPreviewController, but didn't (Zeitwerk::NameError)

[aduth]

ViewComponentsController is from the view_component gem so that should be defined regardless of feature flag status, and we could move the condition inside the class.

class ComponentPreviewController < ViewComponentsController
  if IdentityConfig.store.component_previews_enabled
    # ...
  end
end

Or, could it make sense to add a require 'lookbook' to the top of the file?

[zachmargolis]

ViewComponentsController is from the view_component gem so that should be defined regardless of feature flag status, and we could move the condition inside the class.

Oh yeah I like that better than my first instinct, 5603a0a, so I updated to do this in 5031d89.

I thought loading lookbook did bad things (like compiling YARD) and so that's why we left it out in the first place

[aduth]

👍