Extract LinkGenerator interface for replaceable/decoratable link generation#370
Open
jelen07 wants to merge 6 commits intonette:masterfrom
Open
Extract LinkGenerator interface for replaceable/decoratable link generation#370jelen07 wants to merge 6 commits intonette:masterfrom
jelen07 wants to merge 6 commits intonette:masterfrom
Conversation
- Extract LinkGeneratorInterface with link(), createRequest(), requestToUrl(), withReferenceUrl(), getLastRequest() methods - LinkGenerator class implements LinkGeneratorInterface, remove final keyword - Presenter::getLinkGenerator() is no longer final, returns LinkGeneratorInterface - Presenter::injectPrimary() accepts optional ?LinkGeneratorInterface for custom injection - ApplicationExtension sets interface type for DI autowiring - Add getLastRequest() accessor (replaces direct $lastRequest property access in Presenter) - Change new self() to new static() in withReferenceUrl() for proper inheritance - Add tests for interface contract and decorator injection pattern Resolves nette#333
- Change $linkGenerator property from readonly to nullable with default null, preventing fatal error when neither custom generator nor router/factory pair is provided - Add missing use import for LinkGeneratorInterface in ApplicationExtension - Remove @internal from interface methods, add descriptive docs instead - Change requestToUrl() parameter from ?bool to bool (null had no distinct meaning) - Add PrefixingLinkGenerator test proving decorators can modify URLs - Add DI container autowiring test for LinkGeneratorInterface - Improve interface PHPDoc with explicit null-return documentation on link() - Clean up interface test: remove concrete class assertions, add createRequest reset test - Remove fragile hardcoded URL assertion from decorator test
- Replace Reflection-based injection test with behavioral assertion via Presenter::link() - Add PrefixingLinkGenerator test proving decorators can transform URLs - Add null-return preservation test for decorators (test mode) - Add edge case tests: getLastRequest after failed link, withReferenceUrl immutability, createRequest and requestToUrl direct usage - Add LinkGeneratorInterface assertion to ApplicationExtension.basic.phpt - Add DI container autowiring test for LinkGeneratorInterface - Remove unnecessary PresenterFactory from test constructors - Fix empty catch blocks with proper Assert::exception usage
- Rename interface from LinkGeneratorInterface to LinkGenerator (Nette uses plain nouns for interfaces) - Rename class from LinkGenerator to DefaultLinkGenerator - Update all references in Presenter, ApplicationExtension, LinkBaseNode - Update all existing tests to use DefaultLinkGenerator for instantiation - Update static method calls: DefaultLinkGenerator::parseDestination(), DefaultLinkGenerator::applyBase() - Update Latte bridge compiled template output assertions
- DefaultLinkGenerator docblock: "Default link generator." to distinguish from interface - ApplicationExtension: add use import for DefaultLinkGenerator, use short name in setFactory
- Remove public visibility from interface methods (Nette interfaces use bare function) - Fix use import ordering in Presenter: DefaultLinkGenerator before Helpers (alphabetical) - Add missing str_starts_with to use function imports in DefaultLinkGenerator - Restore @internal on createRequest() and getLastRequest() in interface - Improve interface docblock to "Generates links to presenter actions." - Remove redundant DI test from decorator test file (already covered in ApplicationExtension.basic)
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.
Problem
Since v3.2.3, the link generation logic was moved from Presenter to the
LinkGeneratorclass (which is a great improvement). However, the class isfinalandPresenter::getLinkGenerator()is alsofinal, which means there is no way to customize link generation behavior.Real-world use case
In our application, we use referenceable modals that can overlap each other. When a user arrives at a page with open modals (via direct link or after a page refresh), links generated in components below the modal incorrectly include persistent parameters of all opened modals.
To solve this, we needed to intercept link generation and nullify persistent parameters based on context. Since
LinkGeneratorisfinalandgetLinkGenerator()is alsofinal, we were forced to create a workaround that overrides four methods (link(),isLinkCurrent(),redirect(),redirectPermanent()) on every component via a trait, all full of@phpstan-ignore-linecomments because we access@internalmethods. This works but is fragile and not a clean solution.We believe this is a valid use case that the framework should support natively.
Solution
This PR extracts a
LinkGeneratorinterface from the existing class (renamed toDefaultLinkGenerator), following Nette's established naming convention (plain noun for interfaces, e.g.TemplateFactory,Response,Renderable).Changes
New files:
src/Application/LinkGenerator.php— interface withlink(),createRequest(),requestToUrl(),withReferenceUrl(),getLastRequest()src/Application/DefaultLinkGenerator.php— the original class, renamed, implements the interfaceModified files:
Presenter— property typed asLinkGenerator(interface),getLinkGenerator()is no longerfinaland returns the interface,injectPrimary()accepts optional?LinkGeneratorfor custom injectionApplicationExtension— registers the service with typeLinkGenerator(interface) and factoryDefaultLinkGeneratorLinkBaseNode— static calls updated toDefaultLinkGenerator::applyBase()How it enables decoration
Users can now create a custom
LinkGeneratorimplementation (decorator) and inject it via DI:Or override
getLinkGenerator()in a Presenter subclass for more specific control.BC impact
LinkGenerator $xtype hintsnew LinkGenerator(...)new DefaultLinkGenerator(...)(rare in userland, DI handles this)LinkGenerator::parseDestination()DefaultLinkGenerator::parseDestination()(@internal)LinkGenerator::applyBase()DefaultLinkGenerator::applyBase()(@internal)The breaking changes only affect direct instantiation and
@internalstatic method calls, which are not part of the public API and are extremely rare in userland code.Tests
LinkGenerator.interface.phpt— interface contract verificationLinkGenerator.decorator.phpt— decorator injection and URL modification patternsResolves #333
Thank you for your time and for the framework!