Skip to content

fix: align Media docblock with the nullable migration columns - #722

Merged
awcodes merged 1 commit into
5.xfrom
fix/nullable-media-columns
Aug 14, 2026
Merged

fix: align Media docblock with the nullable migration columns#722
awcodes merged 1 commit into
5.xfrom
fix/nullable-media-columns

Conversation

@awcodes

@awcodes awcodes commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Fixes #721.

The reported bug

Media's docblock declared four properties non-nullable that stubs/migration.stub creates as nullable():

property stub
directory $table->string('directory')->nullable()
width $table->unsignedInteger('width')->nullable()
height $table->unsignedInteger('height')->nullable()
size $table->unsignedInteger('size')->nullable()

Larastan believes the annotation, so correct null-handling in a consuming app gets reported as a defect — the reporter's expect($media->width)->toBeNull() for a sanitized SVG came back as pest.expectation.impossible.

All four now carry |null. I also corrected @property-read string $urlstring|null, since it resolves through Media::resolveUrl(), which is already declared ?string.

Per the issue's suggestion I checked the rest of the block against the stub. Everything else lines up: alt, title, description, caption, exif and curations already have |null; disk, name, path, type and ext are non-nullable columns and visibility has a default('public'); pretty_name is a nullable column but is in $appends with an accessor that always returns a string; the tenancy FK has a templated name, so there's nothing to annotate.

Two real crashes the annotation was hiding

Chasing the size call sites turned up two views that hand a null straight into sizeForHumans(int $size) and fatal:

  • resources/views/components/forms/details.blade.php — the details panel on the media edit page. The existing filled($record) guard checks the record, not the size.
  • resources/views/components/forms/picker.blade.php — the picker's list display.

Both are guarded now, with a regression test per site in tests/src/Feature/NullableMediaColumnsTest.php. Both tests fail on 5.x with Argument #1 ($size) must be of type int, null given and pass here.

The picker test needs the list markup, so PostResource gains an opt-in listDisplay() driven by a config flag — the other tests that render that fixture keep the default grid display.

Checked and deliberately left alone

  • MediaTable's size column looks like the same bug but isn't reachable: TextColumn short-circuits to the placeholder branch on blank state and never calls formatStateUsing. Verified by running a null-size record through the list page unpatched.
  • info-overlay.blade.php is also safe, for a different reason: Blade's @props default ('size' => 0) replaces an explicitly passed null, so it renders 0 B rather than throwing. Cosmetically wrong for an unknown size, but not a crash, and changing it would alter the grid's rendering — happy to do it separately if you want.

Notes

  • 3.x has a wider version of the same problem: its docblock also declares alt, title, description, caption, exif and curations non-nullable. Separate backport if you want it.
  • composer test is green: 201 passed, PHPStan clean, no Pint or Rector drift.

🤖 Generated with Claude Code

`directory`, `width`, `height` and `size` are created `nullable()` by
stubs/migration.stub, but the model's docblock declared them non-nullable.
Static analysis in consuming apps then flagged correct null-handling as a
defect — a test asserting `$media->width` is null for a sanitized SVG was
reported as an impossible expectation.

Also corrects `url`, which resolves through `Media::resolveUrl()` and is
already declared `?string` there.

Chasing the annotations turned up two views that really do hand a null
`size` straight to `sizeForHumans(int $size)`, fatalling the page: the
media edit page's details panel and the picker's list display. Both are
now guarded, with regression tests covering each.

Fixes #721

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@awcodes
awcodes merged commit 5dec320 into 5.x Aug 14, 2026
11 checks passed
@awcodes
awcodes deleted the fix/nullable-media-columns branch August 14, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Media docblock declares directory/width/height/size non-nullable, but the package's own migration creates them nullable

1 participant