Add request detectors for mobile and tablet.

[ADmad]

Library Mobile_Detect has been added as a dependency.

[lorenzo]

👍

[markstory]

Looking at this code makes me which we didn't need that wrapping array. Totally tangential though.

[lorenzo]

Yeah, seems unnecessary

[ADmad]

If we skip BC we can just replace all that options keys juggling and only use closures.

[lorenzo]

Detecting with is_callable() is also an option

[ADmad]

Yes it is, but i find the current code for Request::is() with almost a dozen isset checks pretty ugly :)

[markstory]

Looks good to me 👍

[rchavik]

Several things:

• Can we have this as a "suggest" instead?
• Instead of hardwiring the setup in bootstrap, can we inject the setup code using composer's post-package-install ?
• Is it possible to have only one instance \Detection\MobileDetect instead of one for each check?

[rchavik]

Uh. I just noticed #3031 and several already mentioned "suggest" is preferred. I see @lorenzo's point, but I still think that we should ship with as little dependencies as possible.

[ADmad]

Can we have this as a "suggest" instead?

Composer just lists the suggested packages after installation. There's no switch to actually install suggested packages, you have to explicitly install them separately. So that's not good enough.

Instead of hardwiring the setup in bootstrap, can we inject the setup code using composer's post-package-install ?

How would that be better? If you don't need those detectors remove the lib from your composer.json and strip out relevant code from bootstrap. In majority of the cases you will use the app template just as a starting point and modify bootstrap and other files as per your needs.

Is it possible to have only one instance \Detection\MobileDetect instead of one for each check?

I could create an instance and then pass it into closures for both detectors using use but that's not necessary imo. This lib is pretty lightweight. I don't thing creating duplicate instance will cause any noticeable overhead.

I see @lorenzo's point, but I still think that we should ship with as little dependencies as possible.

This is the first external dependency we have added that too for app template not core.

[rchavik]

Composer just lists the suggested packages after installation. There's no switch to actually install suggested packages, you have to explicitly install them separately. So that's not good enough.

That's the point, I would not want suggested packages to be auto-installed.

How would that be better? If you don't need those detectors remove the lib from your composer.json and strip out relevant code from bootstrap. In majority of the cases you will use the app template just as a starting point and modify bootstrap and other files as per your needs.

I see it in the opposite side, why would we 'hardwire' things when it can be optional and automated? And people who really need these, would have the composer.json automatically updated by running composer require mobiledetect/mobiledetectlib anyway.

I could create an instance and then pass it into closures for both detectors using use but that's not necessary imo. This lib is pretty lightweight. I don't thing creating duplicate instance will cause any noticeable overhead.

Ok. I'm just asking because I don't really know the internals. Good to know that we can have an lighter method of setup anyway.

This is the first external dependency we have added that too for app template not core.

Not specifically related to this pull request, but where would we draw a line?

One thing that draws me to cake in early days was that it was a full stack framework. Granted, with 3.0 and composer, things can be more interchangable. And when we can, we should re-use code from externally libraries.

I'm okay with that, but we should make them a "suggest" and not "required".

Another question, how should we handle library change if a better lib is available? The way i see this is that if this was released in 3.0.0, we're tied to mobiledetectlib for the rest of 3.x lifecycle.

[jippi]

I think we should include/require as much as possible from the community we possible can.

Cake 3 is a good opportunity to kill the full monolithic "not invented here" framework and embrace what the community in general is doing, working with composer and having dependencies where it make sense.
Embracing the "new" PHP community way of doing things, will probably also appeal to a lot of new users that are more experienced and tech savvy than the users we currently have a majority of.

I don't see a problem at all to include 5-10 composer dependencies, if the components included is well maintained, and they make sense for the framework.

I think we should just ignore people who are afraid of composer and continue on the path of being a composed framework like everyone else in the community is doing :)

If a library don't suit us or the user later, it's trivial to update, people can change their composer file back or chose to follow the framework. I think thats a missed point of composer :)

Also, it's not CakePHP that depends on it, but the App version, so CakePHP internally don't care, it's only the actual application, and thus something people can remove, change or switch as they see fit.

[rchavik]

I think we should just ignore people who are afraid of composer and continue on the path of being a composed framework like everyone else in the community is doing :)

I'm sorry Jippi, that's just a low blow and lame argument.

[ADmad]

I see it in the opposite side, why would we 'hardwire' things when it can be optional and automated? And people who really need these, would have the composer.json automatically updated by running composer require mobiledetect/mobiledetectlib anyway.

The app template should just work out of the box. You shouldn't need to install anything separately. This is very essential for attracting newbies.

Not specifically related to this pull request, but where would we draw a line?

No where. We consider it case by case and keep pushing it as further as we want.

I'm okay with that, but we should make them a "suggest" and not "required".

We will have to just agree to disagree on that.

Another question, how should we handle library change if a better lib is available? The way i see this is that if this was released in 3.0.0, we're tied to mobiledetectlib for the rest of 3.x lifecycle.

I see no harm if 3.0 is tied to mobiledetectlib (which btw is the app template not the core). Even if the original authors stop supporting it would be way more up to date than that mobile detection we had in the core.

In general I believe the days of developing and maintaining everything ourselves are gone. We don't have the man power to do that. If there are existing libs which do a particular job nicely than we should use them instead of trying to reinvent the wheel, albeit a broken one.

[jippi]

@rchavik low how? there is always people opposing progress, and considering how cake 3 already depends on composer, I do not see any problem in requiring 3rd party modules.
Cake 3 is embracing everything modern from the community, including composer, that has been decided a long time ago.
For people who are composer afraid, we will provided a pre-packages version of app + cake they can build on, but the recommended way of using cake3 will be through composer

[josegonzalez]

@rchavik How do you currently manage application dependencies? Do you not use plugins? Are they committed to your repository, or do you use submodules? Just curious, as your advocacy against composer appears to be advocacy in general against application dependencies.

[markstory]

While I think composer is great it is not the only way to manage dependencies. I know many people use git submodules, which the new MobileDetect dependency is amenable to. Keep in mind that the mobile detectors can be removed by anyone not needing them, and that the code will be rolled into the zip file we generate in future releases.