Repository navigation
fix: drop stale optional dependency on v17development/flarum-seo - #87
Conversation
The entry was added in FriendsOfFlarum#64 so that Sitemap booted after v17development/flarum-seo and could remove that extension's robots.txt route. FriendsOfFlarum#66 dropped the route removal for Flarum 2.0, and no SEO release for Flarum 2.0 registers a robots.txt route, so on this branch the entry no longer orders anything Sitemap needs. It now causes a circular dependency. SEO, now fof/seo, lists fof/sitemap in its own optional-dependencies, and Packagist still serves it under the old name. On a forum that installed SEO as v17development/flarum-seo, Flarum refuses to enable the second of the two extensions. The entry is not renamed to fof/seo, because that would create the same loop on every forum running both.
|
Note Automated review for @qin-yu. Multiple AI models with Flarum-specific knowledge reviewed this pull request for me, and a further agent verified their findings; I haven't read it myself yet. I'll look at this pull request personally as soon as I can. No findings that need fixing in this pull request. The change merges cleanly into This assessment is based on code inspection; I did not independently run tests or reproduce the forum upgrade. |
I ran into this while upgrading my forum from Flarum 1.8 to 2.0. Its
composer.jsonstill requiredv17development/flarum-seo, and after the upgrade Sitemap could no longer be enabled.Changes proposed in this pull request:
Removes
v17development/flarum-seofromextra.flarum-extension.optional-dependencieson the 2.x branch.The entry was added in #64 so that Sitemap booted after
v17development/flarum-seoand could remove that extension'srobots.txtroute. #66 dropped the route removal for Flarum 2.0, and no SEO release that supports Flarum 2.0 registers arobots.txtroute, so the entry no longer orders anything Sitemap needs.It now causes a circular dependency. SEO, now
fof/seo, listsfof/sitemapin its ownoptional-dependencies(added in FriendsOfFlarum/seo#124), and Packagist still serves it under the old name. On a forum that installed SEO asv17development/flarum-seo, which is the likely path when upgrading from 1.x, enabling Sitemap fails:Reviewers should focus on:
fof/seo. Renaming it would create the same loop on every forum running both extensions.robots.txtroute, so it still needs the entry for forums onv17development/flarum-seo2.0.9 or earlier. The same loop does happen on 1.x with SEO 3.x installed under the old name, but fixing it there needs a different approach, so it is not part of this PR.Confirmed
composer test).There are no frontend changes. How it was tested:
composer teston PHP 8.5.12 with MariaDB 11.4: unit 1 test, integration 56 tests (3 skipped), all passing, with the same results as unpatched 2.x.v17development/flarum-seo4.0.0-beta.16 enabled, enabling fof/sitemap 3.0.0 failed with the error above. With this branch installed through a Composer path repository, both extensions enabled, Sitemap first, and/sitemap.xml,/robots.txtand discussion pages answered 200.fof/seoinstead, as the SEO README describes, also avoids the loop, since the old name then matches no installed extension.