Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

calling a modx instance via getInstance #16571

Merged
merged 14 commits into from
May 29, 2024
Merged

Conversation

webnitros
Copy link
Contributor

@webnitros webnitros commented May 13, 2024

\MODX\Revolution\modX::getInstance(\MODX\Revolution\modX::class)

What does it do?

Calling a modx instance without having to pass it through __construct(modX &$modx)
this method will be useful in cases where you need to access an instance of the modX class from different parts of your application

Why is it needed?

The problem is reusing classes from composer as well as shortening for writing code in various classes

How to test

The feature was tested on real modx 2.8 projects with a high load and a large backend
\MODX\Revolution\modX::getInstance(\MODX\Revolution\modX::class)
phpunit tests pass successfully

Related issue(s)/PR(s)

#14555
#16569 (comment)

Functions app

I didn’t immediately add the app() function
to shorten the call code
I wanted to clarify where it would be better to add it?
It might be worth renaming it to appModx()
Although app() would obviously be better

if (!function_exists('app')) {

    /**
     * Get the available modx instance.
     * @return mixed|\MODX\Revolution\modX
     */
    function app()
    {
        return \MODX\Revolution\modX::getInstance(\MODX\Revolution\modX::class);
    }
}

Example

require 'index.php';

# Old
$example1 = $modx->getOption('site_name');
# New
$example2 = \MODX\Revolution\modX::getInstance(\MODX\Revolution\modX::class)->getOption('site_name');
$example3 = app()->getOption('site_name');

ide code highlighting works immediately

CleanShot 2024-05-14 at 01 47 01

\MODX\Revolution\modX::getInstance(\MODX\Revolution\modX::class)
Copy link
Member

@opengeek opengeek left a comment

Choose a reason for hiding this comment

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

This will work if you revert the change to modX::getInstance() and remove the class name string you are passing to it, passing null instead when additional parameters are involved.

core/src/Revolution/modX.php Outdated Show resolved Hide resolved
@opengeek
Copy link
Member

Thank you for your contribution, @webnitros! Before we can move forward, please sign the MODX Contributor License Agreement (CLA). This is to ensure MODX has your written permission to distribute projects containing your contributions under the appropriate license.

Please make sure the CLA has been signed for GitHub user(s): @webnitros

Once you've signed, please reply with a comment letting us know so I can verify. Then, I will work on getting this integrated.

@webnitros
Copy link
Contributor Author

Thank you for your contribution, @webnitros! Before we can move forward, please sign the MODX Contributor License Agreement (CLA). This is to ensure MODX has your written permission to distribute projects containing your contributions under the appropriate license.

Please make sure the CLA has been signed for GitHub user(s): @webnitros

Once you've signed, please reply with a comment letting us know so I can verify. Then, I will work on getting this integrated.

ready

@opengeek opengeek merged commit d34aa7e into modxcms:3.x May 29, 2024
5 checks passed
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.

None yet

2 participants