@ConcreteOwl did you make a Github issue about this?
No @Myq I thought it would be picked up by Andrew by responding to his post.
A new Github issue has now been created.
Ok, I’ve installed RC2, installed my package - seems some Illuminate exceptions are gone.
Following this doc:
seems to have fixed the errors so far. Will keep trying.
I have an exception “Unable to get permission key for allowed_file_extensions” using the following:
$fs = FileSet::getGlobal(); $fp = $this->app->make(Checker::class, ['fs' => $fs]); $extensions = $fp->getAllowedFileExtensions();
So how do I fix that for v9?
@linuxoid try with this code
use Concrete\Core\File\Set\Set; use Concrete\Core\Permission\Checker; $fp = new Checker(Set::getGlobal()); $extensions = $fp->getAllowedFileExtensions();
@mlocati , I know that works. But the PRB told me to avoid the use of ‘new’ instantiations and to move everything to ‘app->make’. So I’m wondering how to instantiate all classes with various variable names there.
Checker class can be created by using
If you still want to use the “make” approach, you have to:
- check the constructor of
- there you can see that it accept one parameter, named
- So, you have to call
$app->make(Checker::class, ['object' => $fs])
@mlocati , So the key name must be the same as the constructor variable name, not the passed variable name! Got it. Thank you.
Yep: that’s the general rule when you use
make: check the argument names in the class constructor (
function __construct(...)), and use them (without the leading
Well, with the app->make out of the way, now I’ve got another exception but with useless log info. It doesn’t say what caused it, which file, which line. I have no clue where to even start looking:
With these release candidates I’m noticing that there’s something a bit misaligned with the full error/whoops page, the error line highlighted appears too high on the page, and you can’t scroll up to see it.
I thought it might have just been me experiencing this, but linuxoid’s screenshot above is a good example of this happening.
I am also seeing the whoops page being misaligned, with the important part being off the top rather than in the middle.
Perhaps a BS3->BS5 style issue on the way the code block is vertically centered.
I’ve actually reported that as a bug on github
Just going to reply with some hopefully helpful advice:
There’s really nothing too magical about $app->make(). You don’t have to use it. In general, it’s useful when you have some dependencies that are globally registered that you don’t want to have to retrieve yourself in order to pass in.
So, for example, let’s say we want to use the
Concrete\Core\Filesystem\FileLocator class, in order to search the file system for some element or block templates. The class requires
Concrete\Core\Application\Application in order to do its business, and it obtains those classes in the constructor.
Instead of having to somehow instantiate the Filesystem and Application classes in order to pass them into your new FileLocator class, you can just run
Which will then auto-magically pass in the appropriate versions of those classes, either by just automatically instantiating them or by looking into one of the service provider classes and then figuring out how to build the classes.
I mention this because if there’s a class that you need to use that only takes an object that you always have on hand, there’s absolutely no reason to use $app->make(). The permissions checker class is a perfect example of this. The checker class only takes a single object – whatever object you want to check permissions on. So it doesn’t make sense to run it through $app->make() because it’s just adding overhead. This
$permissions = new Checker($block);
Is exactly the same as this
$permissions = $app->make(Checker::class, ['object' => $block]);
But the second one is a lot harder to read.
Just thought I’d mention this. If you want to read up more on why we use this approach, this document still holds up (even though it talks about Core::make() a lot – that’s exactly the same as $app->make(), it’s just you don’t have to have a $app object to work on.)
Just wanted to let everyone know that a new release candidate has been posted. Here’s a new thread:
Issue [v9.0.0rc3] Whoops right pane crops error line off top of screen · Issue #9745 · concrete5/concrete5 · GitHub
@andrew I don’t fully agree with you when it comes to using $app->make() not being useful.
Without it you’re not taking advantage of Laravel’s container. I built several packages extending on Concrete CMS in ways that were only possible because the classes I was building on were instantiated using $app->make() and I also found myself unable to do something because the core was using new ClassName() and it was impossible to override that in any way.
@mnakalay That’s true – and that’s certainly something we’ve taken advantage of in the past – but I think at this point I would argue that that’s really more a side effect of the container architecture than a feature that should be actively used. Certainly in the past I’ve been all over trying to use these types of approaches for extension, but I think at this point I’d argue that if you’re trying to extend a class in some way, it should be built into the architecture in a more dependable way, like a driver-based approach, or some other method of extension - than just passing a class at runtime.
The exception to this, of course, is if interfaces are actually type hinted within a particular class. Then it’s kind of generally accepted that the underlying class doesn’t know anything more about the class it’s working with than the features of that particular interface, and as long as your custom class implements the interface properly this is a good use of the container.
tldr: I think the IoC container is best left to making it easier to test classes, and forcing developers to write less code or repeat less code in order to instantiate classes. But if a particular class or extension pattern requires you to use the IoC container, you’re doing something wrong.
(With that being said, I’m sure there’s many examples of us doing exactly this within the core)