Skip to content

Refactor AbstractJsStrings and related classes to use constructor injection and Autowiring#5075

Open
sambhavp96 wants to merge 7 commits intovufind-org:dev-12.0from
sambhavp96:modernize-abstract-js-strings
Open

Refactor AbstractJsStrings and related classes to use constructor injection and Autowiring#5075
sambhavp96 wants to merge 7 commits intovufind-org:dev-12.0from
sambhavp96:modernize-abstract-js-strings

Conversation

@sambhavp96
Copy link
Contributor

No description provided.

Copy link
Member

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

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

@sambhavp96, I'm encountering test failures because theme.config.php still has the InvokableFactory set for the TransEsc view helper, so the plugin manager is trying to build the class without its dependencies. You might want to double-check that all of the helper factory settings are removed or updated as appropriate. Once you've had a chance to do that, I'll try the tests again.

Thanks for the progress; sorry for the bumpy road! :-)

Copy link
Member

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

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

Thanks, @sambhavp96, this is looking great so far. I pushed up a small change to introduce constructor property promotion to AbstractJsStrings (since it seemed faster to make the edit than to write up a comment suggesting that you do it).

Only one more thing I would suggest to finish this up: the JsIcons view helper is the other subclass of AbstractJsStrings, but it has not yet been revised to use property promotion and autowiring. It would probably make sense to take care of that one here so that all the related work is done together. I think it will be pretty straightforward.

@demiankatz demiankatz added this to the 12.0 milestone Feb 17, 2026
@demiankatz demiankatz added the architecture pull requests that involve significant refactoring / architectural changes label Feb 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

architecture pull requests that involve significant refactoring / architectural changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants