8.0.0: PHP 8 attribute grid config, Symfony 7/8 support, and legacy cleanup#35
Open
mmucklo wants to merge 21 commits into
Open
8.0.0: PHP 8 attribute grid config, Symfony 7/8 support, and legacy cleanup#35mmucklo wants to merge 21 commits into
mmucklo wants to merge 21 commits into
Conversation
aed4ec4 to
dd3c885
Compare
Grid ScreenshotsAuto-generated from CI run |
90fc5c4 to
bd39e06
Compare
Grid configuration can now be expressed with native PHP 8 attributes
alongside (or instead of) Doctrine annotations:
#[Grid]
#[ShowAction]
#[DeleteAction]
#[Sort(column: 'name', direction: 'ASC')]
class Product {
#[Column(label: 'Name', sortable: true)]
private $name;
}
- Annotation classes gain constructors following Symfony's dual pattern:
a $data array first param (Doctrine annotation reader BC) plus named
params (PHP 8 attributes). All six are marked #[\Attribute(...)].
- ColumnSource reads #[Grid]/#[Column], plus class-level #[Action]
(and subclasses via IS_INSTANCEOF) and #[Sort]. Actions/sort can't be
nested inside #[Grid] because PHP attribute args must be constant
expressions. Sort is IS_REPEATABLE so multiple #[Sort] map to sortMulti.
- Shared buildColumnInfoFromGrid() backs both annotation and attribute
paths; attributes are tried first, then annotations, then reflection.
- Tests: constructor instantiation (PHP-7-safe + PHP-8 named-arg suite)
and an integration test driving getColumnSourceInfo against a fixture,
asserting columns, action column and sort resolve from attributes.
- phpunit.xml splits a php8 testsuite so PHP-8-only syntax never loads
on PHP 7.x. README documents attribute usage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A new CI job boots a real Symfony app (FrameworkBundle, TwigBundle, DoctrineBundle, DtcGridBundle) with a SQLite-backed Product entity configured via #[Grid]/#[Column], starts the PHP built-in server, and uses Puppeteer to screenshot the table and DataTables renderers (waiting for DataTables' AJAX to settle). Screenshots upload as artifacts and, for same-repo branches, are pushed to an orphan refs/screenshots/<branch> ref and embedded in a PR comment. Fork PRs skip the write steps. - Tests/App: minimal kernel, Product entity, YAML config (SQLite ORM), front controller (serves static bundle assets directly), DB+fixtures setup script. - Tests/Screenshots/screenshot.mjs: Puppeteer capture script. - require-dev: doctrine/doctrine-bundle, symfony/yaml (test app only). - DtcGridBundle::build() gains a : void return type for Symfony 8. - PHPStan excludes Tests/App and Tests/Screenshots; baseline regenerated. - .gitignore: node_modules/, Tests/Screenshots/output/. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The default config loaded dataTables.bootstrap.min.* (Bootstrap 3) while
the default theme is Bootstrap 4.4.1, so the length selector and search
box rendered beside the grid instead of above it. Switch defaults to
dataTables.bootstrap4.min.{css,js}. Caught by the new screenshot CI.
BC note: override dtc_grid.datatables.css/js to keep the old behavior.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2ce6c94 to
e987b7d
Compare
shouldIncludeColumnCache only ran the debug-mode timestamp check when an annotation reader was present; with no reader it trusted the cache. That left PHP 8 attribute-only entities (which can be read without an annotation_reader) serving stale columns in dev until a manual cache clear. Always re-check timestamps in debug mode regardless of reader. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pin Puppeteer to a fixed version and cache ~/.cache/puppeteer (the ~150 MB Chromium download) keyed on that version, so it isn't re-downloaded on every run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The screenshot job pushes throwaway refs that were never cleaned up. Add a workflow that deletes the ref on PR close (same-repo branches only). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
composer 2.10 blocks security-advisory-affected packages during dependency resolution, which makes the legacy PHP matrix (7.2-8.0) unresolvable: older twig/symfony versions get excluded, leaving only the newest twig which requires PHP 8.1+. These jobs passed on composer 2.9.x; pin to it to restore a resolvable graph for old PHP/Symfony. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous composer 2.9 pin did not help: both composer 2.9.8 and 2.10 block security-advisory-affected packages during resolution, which makes the legacy PHP matrix (7.2-8.0) unresolvable (older twig/symfony excluded, leaving only the newest twig which needs PHP 8.1+). The real fix is composer's audit config: set block-insecure=false so old PHP/Symfony combinations can install the (advisory-flagged but expected) legacy package versions. This only affects this bundle's own dev/CI — a library's config block is not read by downstream consumers. Revert the version pin back to composer:v2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
--prefer-lowest resolves phpunit to its 8.5.0 floor (kept for PHP 7.2),
which crashes on PHP 8.1+ ("Cannot acquire reference to $GLOBALS"). No
single phpunit constraint satisfies both PHP 7.2 and 8.4 under
prefer-lowest, so lowest-deps is tested on 7.2 only; 8.4 is covered by
8.4/highest. (This combination was previously masked by composer's
advisory blocking excluding the ancient phpunit.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Remove dtc:grid:source:generate plus the Generator classes and skeleton templates (deprecated since 3.0.0). The command relied on bundle shortcut notation and Doctrine entity-namespace aliases, both removed in modern Symfony/Doctrine; @Grid/#[Grid] auto-detection replaced this workflow. - Remove dtc:grid:source:list (deprecated since 3.0.0). Grid sources register lazily per request, so in a console process the list was always empty; its duplicate service definition also lacked the manager setter and would fatal if it won the name collision. - Declare GridSourceManager::$entityManager/$documentManager (their setters are public API; undeclared assignment is deprecated on PHP 8.2+) and drop the write-only $sources property. - Fix GenerateGridSourceCommand mongodb registry property typo (superseded by the removal), drop the dead Reader threading and call_user_func_array indirection in ColumnSource cache lookups, and prune the corresponding PHPStan baseline entries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Add AnnotationColumnSourceTest: drives getColumnSourceInfo through a real Doctrine AnnotationReader against an annotated fixture. The annotation path is the bundle's original public API and previously had no coverage at all, despite the annotation classes gaining constructors in this branch; this guards it across the doctrine/annotations ^1.13 || ^2.0 range and PHP 7.2-8.4. - Add ConfigPrecedenceTest + README note pinning the contract that attributes win over annotations when a class carries both. - Reduce Grid::__construct to the data-array form only. Its named params were unreachable: attributes can't pass actions/sort (constant-expression rule) and annotations use the array. Keeping them invited invalid #[Grid(actions: ...)] usage. - Fix sortGridColumns comparator to return int (<=>) instead of bool, deprecated for uasort since PHP 8.0. - Document why composer audit.block-insecure is disabled (CI comment). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 3.x column refactor (March 2020) moved column extraction into the ColumnSource service but left ColumnExtractionTrait behind with no remaining users — 469 lines dead since then. The trait was also the only reader that understood the "return false" negative-cache sentinel ColumnUtil::populateCacheFile still wrote for empty column sets. ColumnSource's stricter reader treats that file as corruption, so a Grid marker with no Column definitions (and reflection unavailable) poisoned the cache and failed every request with the misleading "Bad column cache". Now: - buildColumnInfoFromGrid throws a clear InvalidArgumentException naming the class and the real problem before anything is cached. - populateCacheFile always writes the full columns/sort structure, so an include can never return false. - ColumnlessGridTest pins the new error on both the attribute (PHP 8) and annotation (PHP 7) paths; ColumnUtilTest updated to the new cache-file contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Annotation/attribute API (root-cause fixes via @NamedArgumentConstructor):
- Annotation classes now declare natural constructors mapped by Doctrine's
@NamedArgumentConstructor (supported since annotations 1.12; the bundle
pins ^1.13). Positional attribute syntax #[Column('Name')] now works
instead of fataling with an opaque TypeError about an internal $data
parameter, and direct construction uses real parameters.
- Validation restored and improved: unknown annotation keys throw at the
DocParser layer on both 1.13 and 2.x, and the constructors type-check
values (e.g. sortable="false" throws instead of silently becoming a
truthy string). Grid validates and normalizes actions/sort/sortMulti,
so @grid(actions=@ShowAction) (no braces) and array-literal attribute
data can no longer flow garbage into the column cache.
- Nested actions/sort inside #[Grid] now work on PHP 8.1+ (the old
"constant expressions" rationale predates new-in-initializers); the
separate class-level attribute form remains for PHP 8.0. Comments,
README and CHANGELOG updated to match.
ColumnSource resolver (regression + behavior fixes):
- Half-migrated classes work again in both directions: #[Grid] with
@column docblock columns (which regressed to a 500/wrong columns) and
@grid with #[Column] properties. Each reader falls back to the other
source's column definitions when it finds a marker but no columns.
- The columnless-grid error moved from the reader to the end of the
resolver chain, after attributes, annotations, cache and reflection
have all been consulted.
- In debug mode, a stale cache for a class with no Grid marker (YAML
grids cached at container compile time) is served rather than being
silently replaced by reflection columns after an entity edit.
- Cached sort lists are now validated with the correct shape
(column => direction map); the old call misread the map and could
throw on a column literally named "column" while validating nothing
for everyone else.
- Built column info is returned in-memory (cache write is a side
effect), removing the write-then-re-include roundtrip; dead members
($cachedSort, setDebug, setCacheDir) removed; duplicated reader tails
and property-collection loops hoisted into shared helpers.
CI/workflows:
- Screenshot refs are archived (refs/screenshots/closed/pr-N) on PR
close instead of deleted, so images in merged PRs' comments survive
garbage collection; the screenshot SHA is passed between steps via
step output instead of implicit HEAD state; comment lookup paginates
past 30 comments; Puppeteer browser/npm cache saves even on job
failure; the screenshots job gained the composer cache the tests job
already had.
Tests: shared ColumnSourceTestCase base (removes ~60 duplicated lines),
plus regression tests for half-migration (both directions), annotation
validation, stale-cache fallback, and PHP 8.1 nested attributes. Dead
autoDiscoverColumns references removed from five doc files.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
doctrine/annotations 1.13.3+/2.x wrap annotation-constructor throws in AnnotationException; 1.13.0 (the lowest-deps floor) propagates them raw. Assert the contract (loud failure, clear message) rather than the wrapper class. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restructure the ColumnSource resolver so config-source composition lives in the orchestrator instead of inside each reader, which root-fixes three regressions in the previous round's fallback logic: - Columns merge per-property (collectColumns walks properties once, attribute winning per property), so a property-by-property migration no longer silently drops the unconverted columns. - Class-level #[Action]/#[Sort] attributes merge into a Grid marker from either source (collectGrid), so an @grid annotation class migrating its actions/sort to attributes no longer loses them. - Cache files carry a 'source' marker ('compile' for dtc_grid YAML built at container compile, 'runtime' for request-time reads). The timestamp-stale fallback only resurrects 'compile' caches, so a removed annotation/attribute Grid marker now takes effect instead of the stale runtime cache rendering deleted config forever. Also: - An empty-column cache is treated as a miss, not a silent zero-column grid; the no-columns configuration error moved into buildColumnInfoFromGrid (before the action column is added) so an actions-only grid can't mask it. - ColumnUtil::populateCacheFile var_export()s keys (user-controlled YAML column names with an apostrophe previously produced an unparseable cache) and writes atomically via tempnam+rename so a concurrent request can't include() a half-written file. - phpunit.xml gains defaultTestSuite="default" so a bare `bin/phpunit` on PHP 7.x no longer loads the PHP-8-only suite and fatals on parse. - CI: drop always() from the Puppeteer cache save (it could persist a partial download under the immutable key); poll for the dev server instead of a fixed sleep; archive screenshot refs via FETCH_HEAD. Cleanups: gridConfigured flag removed; instantiateColumnInfo moved to ColumnUtil beside the cache writer; validateSortList delegates to validateSortInfo; Grid normalize/describe helpers collapse the duplicated actions/sortMulti validation; checkTimestamps made private; a single TempDir::remove replaces three recursive-delete copies. CHANGELOG notes the setDebug/setCacheDir removal; README documents per-property merge. Regression tests added for each fixed behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BC-1: restore ColumnSource::checkTimestamps() to public static. It was
public static and advertised since 3.3.0 ("expose compiler-pass logic as
a static method"); privatizing it was an undocumented break.
BC-2: getCachedColumnInfo treats any non-current-format cache file as a
miss (rebuild) instead of fataling — covers a pre-8.0 `return false`
sentinel, a corrupt/partial file, and any cache predating the 'source'
key. Removes the "Bad column cache" exceptions in favor of self-healing.
#1 (validation consistency): add a shared ValidatesArguments trait and
apply it across Column/Action/Sort/Grid so every grid annotation enforces
its argument types the same way. Sort now rejects a non-ASC/DESC
direction and Action a non-string label/route/etc. at the config boundary
(previously only Column validated; Action/Sort passed bad values through).
#2 (cache mechanism): stop generating PHP source. populateCacheFile now
var_export()s the column *spec* array (class + arguments), materialized on
read by the single instantiateColumnInfo() factory. This removes the
hand-written codegen, the dual materialization the spec required, and the
manual key-escaping (var_export handles it).
#3 (docs): README calltout for the PHP 8.0-vs-8.1 attribute syntax
difference. CHANGELOG documents the removed dtc_grid.command.* service IDs
and the doctrine/annotations soft-dependency; composer.json gains a
suggest entry for it.
Regression tests: Sort/Action validation, and legacy/source-less/corrupt
caches treated as a miss.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the inline attribute/annotation reading in ColumnSource (collectGrid/collectColumns with their PHP_VERSION_ID branches) with a GridConfigSourceInterface and two implementations: AttributeConfigSource (PHP 8 ReflectionAttribute) and AnnotationConfigSource (Doctrine reader). getColumnSourceInfo now builds the source list (attributes first, then annotations) and resolveGridConfig composes them: the Grid marker comes from the highest-precedence source that has one; class-level actions/sort are filled from the highest-precedence source declaring them separately; columns merge per property in declaration order. The precedence/merge rules — previously spread across two methods and the orchestrator — now live in one documented place, and adding a future config source is implementing the interface rather than threading new branches through the resolver. The ReflectionAttribute API reference also moves out of the always-loaded ColumnSource into the PHP-8-only source. Behavior is unchanged; all existing resolver tests (precedence, both half-migration directions, annotation-grid-borrows-attribute-actions) pass as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Key the screenshot ref by PR number (refs/screenshots/pr-<N>) instead of branch name and force-push it each run, so each PR keeps exactly one current set of screenshots. Because the ref is keyed by PR and never deleted, the merged PR's comment images keep working with no separate archive-on-close workflow — cleanup-screenshots.yml is removed. The ref still lives under refs/screenshots/*, which `git clone` does not fetch, so none of this touches consumer clones. Capture as WebP q75 instead of lossless PNG: ~20KB/set vs ~130KB, still rendered inline by GitHub. The push step is now gated to same-repo PRs (a default-branch push has no PR comment to attach images to). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Column formatter was wrongly type-validated as a string. It is a callable (invoked via call_user_func), which may be a string OR an array (['Class','method']) — both valid in annotations and attributes. The string assertion (added with the validation trait this cycle) rejected array callables; drop it. Regression test added. - An explicit empty actions list (#[Grid(actions: [])] / @grid(actions={})) built a stray, empty action column because buildColumnInfoFromGrid used isset($actions) — true for an empty array. Use a truthy check so empty actions behaves like no actions. Pre-existing; the masking phpstan baseline entry ("$actions in isset() always exists") is removed, since it was hiding exactly this smell. Regression test added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Sort constructor's assertOneOf(direction) duplicated the direction check that ColumnSource::validateSortInfo already performs at build time — and threw first, with a context-free message, so an invalid #[Sort] direction surfaced as '"direction" must be one of ASC, DESC' with no hint which entity was misconfigured. Remove the constructor validation (and the now-unused trait) so sort validation happens once, in the resolver, where the column list and entity class name are available; the error now reads '<Entity> - Grid's sort annotation direction "UP" is invalid'. Replace the constructor-throws unit test with a resolver-level test asserting the entity-qualified message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
Releases 8.0.0. Headline feature: entities can be configured with PHP 8 native attributes instead of (or alongside) Doctrine annotations. The release also modernizes the supported platform range, removes long-deprecated subsystems, and rebuilds CI.
What's in it
Grid configuration
#[Grid]/#[Column]native attributes, with#[ShowAction]/#[DeleteAction]/#[Action]/#[Sort]as class-level attributes. On PHP 8.1+ actions and sort can also be nested directly in#[Grid(...)].#[Column], some@Column; an@Gridwith class-level#[Sort]/#[ShowAction]) keeps all of its columns, actions, and sort.@NamedArgumentConstructor; misconfigured values (unknown keys, wrong types, an invalid sort direction) are rejected with a clear error instead of being silently ignored.GridConfigSourceInterfacestrategy (attribute + annotation sources), and the on-disk column cache is now a plainvar_export'd spec materialized by a single factory (no generated PHP source).Platform
^3.4 || ^4.4 || ^5.4 || ^6.0 || ^7.0 || ^8.0).sensio/framework-extra-bundledependency.Removals / cleanup
dtc:grid:source:generate/dtc:grid:source:listcommands, theDtc\GridBundle\Generatorclasses, the skeleton templates, and the orphanedColumnExtractionTrait.CI
@NamedArgumentConstructor: construct them with real parameters (new Column('Name')), not a values array (new Column(['label' => ...])). Misconfigured values (unknown keys, wrong types) now throw instead of being silently ignored.dtc_grid.datatables.css/jsif you depend on the old assets.dtc:grid:source:*commands and their service IDs,ColumnExtractionTrait, andColumnSource::setDebug()/setCacheDir().doctrine/annotations; the native attribute path needs no extra package.See CHANGELOG.md for the full itemized list.
Test plan
--prefer-lowest) through 8.4🤖 Generated with Claude Code