Skip to content

Address Zeitwerk reloading issue#3500

Merged
hectahertz merged 4 commits intoprimer:mainfrom
neilang:main
Aug 13, 2025
Merged

Address Zeitwerk reloading issue#3500
hectahertz merged 4 commits intoprimer:mainfrom
neilang:main

Conversation

@neilang
Copy link
Contributor

@neilang neilang commented May 22, 2025

I observed when calling reload! in a Rails console (after the gem is eager loaded) results in an error:

Uncaught exception: TypeError - superclass mismatch for class CustomCitiesForm

I suspect this occurs because Zeitwerk expects a single constant per file. Calling reload! will remove the constant ApplicationForm but not CustomCitiesForm - which makes it a subclass of a stale object. The next time the file is required, ruby assumes CustomCitiesForm is being redefined with a new superclass.

Steps to reproduce:

Zeitwerk::Loader.eager_load_all
reload!
Zeitwerk::Loader.eager_load_all

Copilot AI review requested due to automatic review settings May 22, 2025 06:59
@neilang neilang requested a review from a team as a code owner May 22, 2025 06:59
@neilang neilang requested a review from hectahertz May 22, 2025 06:59
@changeset-bot
Copy link

changeset-bot bot commented May 22, 2025

⚠️ No Changeset found

Latest commit: 484796f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

I have observed when calling `reload!` in a Rails console (after the gem
is loaded) results in an error:

```
Uncaught exception: TypeError - superclass mismatch for class CustomCitiesForm
```

I suspect this occurs because Zeitwerk expects a
[single constant per file](https://edgeguides.rubyonrails.org/upgrading_ruby_on_rails.html#one-file-one-constant-at-the-same-top-level).

Calling `reload!` will remove the constant
`ApplicationForm`, but NOT `CustomCitiesForm` - which is now the
subclass of a [stale object](https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#reloading-and-stale-objects).

The next time file is required, ruby assumes `CustomCitiesForm` is being
redefined with a new superclass.

Steps to reproduce:
```
Zeitwerk::Loader.eager_load_all
reload!
Zeitwerk::Loader.eager_load_all
```
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR nests form subclasses under their parent form classes to satisfy Zeitwerk’s one-constant-per-file requirement and prevent stale-constant reload errors.

  • Move the outer RadioButtonWithNestedForm and CheckBoxWithNestedForm definitions to wrap their nested form classes.
  • Remove duplicate top-level class definitions.
  • Convert the multi-line nested_form block to a single-line inline block in the checkbox form.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
app/forms/radio_button_with_nested_form.rb Define RadioButtonWithNestedForm at the top, nest FriendForm, remove duplicate class def
app/forms/check_box_with_nested_form.rb Define CheckBoxWithNestedForm at the top, nest CustomCitiesForm, inline nested_form block
Comments suppressed due to low confidence (3)

app/forms/radio_button_with_nested_form.rb:22

  • FriendTextAreaForm remains at the top level rather than nested under RadioButtonWithNestedForm. Nest this class inside RadioButtonWithNestedForm to ensure it reloads together with its parent and avoid superclass mismatch on reload!.
class FriendTextAreaForm < ApplicationForm

app/forms/radio_button_with_nested_form.rb:4

  • Add a regression test that calls Zeitwerk::Loader.eager_load_all and reload! in a Rails console context to verify the TypeError no longer occurs after nesting.
class RadioButtonWithNestedForm < ApplicationForm

app/forms/radio_button_with_nested_form.rb:1

  • [nitpick] Include a brief comment explaining that the nested class structure is required to satisfy Zeitwerk’s one-file-one-constant reload semantics.
# frozen_string_literal: true

Copy link
Contributor

@hectahertz hectahertz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@hectahertz hectahertz enabled auto-merge August 13, 2025 09:54
@hectahertz hectahertz added the skip changeset Pull requests that don't change the library output label Aug 13, 2025
@hectahertz hectahertz added this pull request to the merge queue Aug 13, 2025
auto-merge was automatically disabled August 13, 2025 10:12

Pull Request is not mergeable

Merged via the queue into primer:main with commit 4fea4a1 Aug 13, 2025
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip changeset Pull requests that don't change the library output

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants