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

Documentation on the migration path from laminas-mvc-console #85

Open
koenkivits opened this issue Nov 12, 2021 · 5 comments
Open

Documentation on the migration path from laminas-mvc-console #85

koenkivits opened this issue Nov 12, 2021 · 5 comments
Labels
Documentation Improvements or additions to documentation Enhancement

Comments

@koenkivits
Copy link

koenkivits commented Nov 12, 2021

Feature Request

Q A
New Feature yes
RFC no
BC Break no

Summary

We're migrating our (pretty big) application from laminas-mvc-console to laminas-cli. So far I genuinely like the new setup, but the migration hasn't been trivial for us so far: we've run into some issues that might be worth documenting. Here are some things I've gathered so far:

  • laminas-cli does not initialize a Laminas\Mvc\Application and therefore does not initiate the MVC application lifecycle. If your app depends on any MVC events (bootstrap, route, dispatch, etc) then this won't work for console commands.
  • because no MVC events are triggered, a module's onBootstrap method will also not be called, as it depends on MvcEvent::EVENT_BOOTSTRAP being triggered. Using onBootstrap is suggested by the Laminas documentation for doing lightweight application setup like setting up event listeners (we're now using ModuleEvent::EVENT_LOAD_MODULES_POST instead).
  • public/index.php is no longer the application entry point, so if you do any setup here then that will no longer work. The docs for the MVC skeleton suggest to add some error reporting here, so it might be good to explicitly mention warn about this in the laminas-cli docs.

There might be more, but this is what I could come up with now. I'd be willing to create a documentation PR by the way, I just figured it was better to file an issue first. :)

@froschdesign froschdesign added the Documentation Improvements or additions to documentation label Nov 12, 2021
@froschdesign
Copy link
Member

@koenkivits
A migration guide is definitely needed!

  • laminas-cli does not initialize a Laminas\Mvc\Application and therefore does not initiate the MVC application lifecycle. If your app depends on any MVC events (bootstrap, route, dispatch, etc) then this won't work for console commands.

Why do you need the same events for a CLI command and an application? What is being executed here? Can you give an example?

In a laminas-mvc based application the listeners can be registered via configuration (global, module, etc.), no need for a module extension:

return [
    'listeners' => [
        MyModule\Listener\MyListener::class,
    ],
    // …
];

I'd be willing to create a documentation PR by the way, I just figured it was better to file an issue first.

👍

@koenkivits
Copy link
Author

koenkivits commented Nov 12, 2021

@froschdesign thanks for your quick response!

  • laminas-cli does not initialize a Laminas\Mvc\Application and therefore does not initiate the MVC application lifecycle. If your app depends on any MVC events (bootstrap, route, dispatch, etc) then this won't work for console commands.

Why do you need the same events for a CLI command and an application? What is being executed here? Can you give an example?

We mostly used onBootstrap for some setup (mostly some logging setup). We also indirectly depended on some ViewManager initialization (which happens on bootstrap), although that was due to some flaky code that was probably very specific to our situation (and which has now been refactored).

We also logged the MVC events as event markers in Tideways. We now simply do the same with Symfony console events. Not a huge thing, but it was something we needed find out.

In a laminas-mvc based application the listeners can be registered via configuration (global, module, etc.), no need for a module extension:

return [
    'listeners' => [
        MyModule\Listener\MyListener::class,
    ],
    // …
];

You're right, we can probably improve our code here, thanks. :) This doesn't seem to be mentioned in the documentation though? (I could look at documenting that too, of course)

(by the way, I linked the wrong documentation. Here's the page that specifically recommends onBootstrap for things like this: https://docs.laminas.dev/laminas-mvc/examples/#bootstrapping)

@froschdesign
Copy link
Member

@koenkivits
Sorry for the very late response. 😞

We mostly used onBootstrap for some setup (mostly some logging setup).

Sounds like it doesn't belong there. Why not configuration and factories?

…although that was due to some flaky code that was probably very specific to our situation (and which has now been refactored).

Refactoring is good. 😃

We also logged the MVC events as event markers in Tideways. We now simply do the same with Symfony console events. Not a huge thing, but it was something we needed find out.

A topic for the migration guide.

This doesn't seem to be mentioned in the documentation though? (I could look at documenting that too, of course)

You are right, this part is missing the documentation of laminas-eventmanager. I added a note to the related project.


Thanks for your feedback and the examples! 👍

@koenkivits
Copy link
Author

@froschdesign

Sorry for the very late response. 😞

🤐 😅 (it's been busy here as well)

Sounds like it doesn't belong there. Why not configuration and factories?

This was in an older part of our code base so it's hard to tell, but I'm not sure if it was ever a very deliberate decision at all. My guess is that the decision at the time was only based on this line laminas-mvc docs:

Each Module class can have an optional onBootstrap() method. This method is used for module-specific configuration, and is the ideal location to setup event listeners for you module.

(emphasis mine)

In any case: I hope to be able to take the time to write the documentation soon. I've had a busy few weeks so I haven't gotten around to it yet. 🙂

@izub
Copy link

izub commented Oct 12, 2022

@koenkivits did you ever manage to write some documentation on the migration path from laminas-mvc-console? We also have a large application that needs migrating and some documentation or a list of the issues you ran into would be very useful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation Enhancement
Projects
None yet
Development

No branches or pull requests

3 participants