To have the discussion started in #16580
Currently when FormRules and FormFields will be namespaced, we have to append a suffix "Rule" and "Field" respectively to the classname. This is because there are fields and rules which would match a reserved word in PHP 7. From what I see it's the boolean
rule and the integer
formfield that would face this issue.
While that is a valid issue to be solved, I think just adding a suffix to it is more of a hack around it.
I think it would be nice if the fieldtype we specify in the form XML matches the classname of the field. So when I specify type="categoryedit"
and the form/fieldset/field has a addfieldprefix="Joomla\Component\Categories\Administrator\Field"
set, I'd epect the class Joomla\Component\Categories\Administrator\Field\Categoryedit
to be loaded. Currently the class that is loaded is Joomla\Component\Categories\Administrator\Field\CategoryeditField
.
Same for rules.
Before #16396 this was actually already working like I would expect, but it was changed due to the conflict with the reserved word (boolean in that case).
Imho a better approach would be to rename the two (?) classes that have this issue. We could add temporary B/C code to deal with that rename into 3.8/3.9 and drop it from 4.0. That way extensions have to rewrite the types in their form XMLs for 4.0 (if they use one of those two classes), but it would be possible to write an extension which works both in J3.9 and 4.0.
Labels |
Added:
?
|
Labels |
Added:
?
|
Actually, looking up the reserved words, neither 'booleannor
integerare reserved words in PHP7.
booland
int` would be the new ones. I'm now confused. @laoneo and @wilsonge can you elaborate what the issue is we try to solve? Or where you just wrongly assuming like me that integer and boolean are reserved words?
There isn't an issue with suffixing class names in this way.
Not by itself, that is true. I just find it more logical to specify the classname in the XML, rather than just a part of it (without the suffix). Especially since we already specify in the XML the namespace of the classes being used.
In other places, like controllers or models we don't use any suffixes as well. So it would be consistent with other code.
I agree that echoo
looks like a mistake we should not do again
Have to admit - I didn't realise that they weren't reserved in PHP 7 - I mean I guess there's an argument that other people could be using bool
or int
. We definitely shouldn't have echoo
again. I'm not hugely fussed either way if I'm honest. Happy to go with the majority consensus
Looking further, list
would be a reserved word. So JFormFieldList
would become an issue. But it could as well be made a class select
to fix the issue and the name would even be more semantic
Category | ⇒ | Unit Tests |
As stated already in the other pr I prefer to have clear class names like TextField
instead of just Text
. But changes like this one are only about personal preference.
Then maybe we should make a codestyle decision and try to do it the same across the CMS. Because currently for example the controllers and models are named "Article", "Admin" and the like. Rules and Fields however have a suffix (FooRule and FooField). In the library it's currently a mix between stuff that is suffixed and others is not. But that is not the point here.
For me it is important that when I reference/load a class (like in the XML) it is not magically appended with a suffix. That's the part which bothers me and which I think is not logical. Especially if there is no technical reason for it.
As stated already in the other pr I prefer to have clear class names like TextField instead of just Text.
To be fair, the class name (FQCN) will be Joomla\CMS\Form\Field\Text
, not just Text
.
Looking further, list would be a reserved word. So JFormFieldList would become an issue. But it could as well be made a class select to fix the issue and the name would even be more semantic
Problem is then you somehow need to ensure that list
in the xml
file proxies to select
Problem is then you somehow need to ensure that list in the xml file proxies to select
Given that we need classmaps anyway (since we rename all classes) I'd say this should actually be easy to solve. If simple classmaps don't work, worst case would be to have have check in the formfield loader which changes "list" to "select" for B/C during J3.
If you change list to select in J3 - then you break anyone with a select form field I would have thought?
Same with 4.0. If there is a JFormFieldSelect
(or whatever the equivalent ends up as in namespaces since most devs are not using properly prefixed classes) then that's going to break.
4.0 you can break b/c though - so it's not such a big deal there I guess. But in 3 it could be
It's still a painful B/C break with no real reason when it can be avoided.
Keeping the class name suffix avoids imposing an arbitrary restriction or running into issues if PHP reserves additional keywords in later releases. For parts of the API that have specific class name conventions because we're building class names, this becomes a fair bit more important.
Just because you can break b/c doesn't mean that you should unless there is a very good reason. The more you break the more pain you create.
I guess then an extension dev can't use the same xml file on 3 and 4 anymore if you rename the list to select or?
If we arbitrarily map list
to select
and not use class name suffixes, then developers who may have a select field would have to rename their fields to something else. If we use the class name suffixes, there is no need for a change.
If you change list to select in J3 - then you break anyone with a select form field I would have thought?
Yep, that's why I said we would have to have B/C code for that specific field in J3 if class mapping can't help here. And yes, it would mean a B/C break in 4.0 when extension developers have to switch it in their XML. However they need to change their XML anyway in 4.0 since the addFieldPath
will not work anymore and has to be changed to addFieldPrefix
(I think) which contains the namespace of the field.
I guess then an extension dev can't use the same xml file on 3 and 4 anymore if you rename the list to select or?
If you rename it in J3.8, (with B/C code for "list") then it would be possible to write an XML which works both in J3.8 and J4.0 by already using select as the type.
If we arbitrarily map list to select and not use class name suffixes, then developers who may have a select field would have to rename their fields to something else.
That's true. It would likely get funny sideeffects in that case. However same applies if we add another new type (which we did in the past) and it's something that is not covered by our Backward Compatibility promise.
If select
is a concern, we could as well name that single formfield "ListField", like we already do with the "ListModel" (vs Articles, Admin, Form, and other models). It was just an example how we could avoid the "echoo" trap.
But I see it looks I'm alone with my view that I find it suboptimal when I specify in the XML the field as type="Categoryedit"
with prefix addfieldprefix="Joomla\Component\Categories\Administrator\Field"
and the class loaded isn't Joomla\Component\Categories\Administrator\Field\Categoryedit
as I would expect but Joomla\Component\Categories\Administrator\Field\CategoryeditField
.
If that's fine and logical by all others, then lets close this and move on.
Yep, that's why I said we would have to have B/C code for that specific field in J3 if class mapping can't help here. And yes, it would mean a B/C break in 4.0 when extension developers have to switch it in their XML. However they need to change their XML anyway in 4.0 since the addFieldPath will not work anymore and has to be changed to addFieldPrefix (I think) which contains the namespace of the field.
But you can have both addFieldPrefix
and addFieldPath
at the same time at the moment which will give you compatibility between J3 and J4 (you need a one off change but you don't have to conditionally load XML files)
Yep. Depends what the migration promise is.
Imho we promise that an extension which is written for 3.9, will work in 4.0. Which would be the case here. You could already use the new name (eg select) in 3.9 and onward and you can use "list" up to 3.99. So no need for two different XML.
Imho we promise that an extension which is written for 3.9
Depending on your use of deprecated functionality of course (like if you're using the old JApplication classes or whatever).
The code we have now for loading fields/rules from a file path won't go away in 4.0 either. When dealing with namespaced classes, use the new attributes for specifying namespaces (this one requires you use namespaces which should inherently get included to the autoloader), but if you haven't namespaced your code you would still use the methods to register path lookups. 5.0 is the earliest we could get away with dropping that part of things.
You could already use the new name (eg select) in 3.9 and onward and you can use "list" up to 3.99.
How do you deal with extensions which have a select form field type? You break them if you arbitrarily map list
to select
and whatever its classname is. Using the class name suffixes avoids the need for a difficult B/C break. Class naming conventions are like coding styles, everyone has their own preferences. I don't think it's too difficult to explain that the class name must be suffixed with "Field" or "Rule" and the reasons why that convention was selected (in part it avoids collisions with reserved PHP keywords).
The code we have now for loading fields/rules from a file path won't go away in 4.0 either
I know, it is needed because we don't force namespaced extensions in J4. However in the example I gave with the categoryedit field it will not help at all. com_categories is namespaced in 4.0 and the old addFieldpath will no longer work for that field. You will need the addPrefix attribute at this point. Since most extensions probably use that field, it means most extensions will have to touch their XML manifest files.
How do you deal with extensions which have a select form field type?
I think I answered that above already. Basically the same we dealt with extensions that had the same formfield we added in the past. We can document it but it is not a B/C break and thus it's the risk of the extensions. Keep in mind that extensions could already "prefix" their own formfields to prevent exactly that scenario. Or of course if you think that risk is to big we could rename it to "listfield" same as we did with the "listmodel" (as I wrote already above). "select" was just an example to avoid "echoo".
I don't think it's too difficult to explain that the class name must be suffixed with "Field" or "Rule" and the reasons why that convention was selected (in part it avoids collisions with reserved PHP keywords).
The point is that you have to explain it, while otherwise it would be intuitive
If we remove the suffixes, we run into a hard B/C break by renaming a class and forcing downstream updates, and we also keep ourselves open to running into possible issues with PHP down the line if they choose to reserve more keywords. Keeping the suffixes prevents the need for a hard break and keeps us from having to (re)name things to avoid PHP core changes. To me that alone is worth the extra effort it'll take to write 2 minutes of documentation, all of which exists in this thread
If we remove the suffixes, we run into a hard B/C break by renaming a class and forcing downstream updates
We already did that by namespacing com_categories and its field but yeah, it would be another one amongst many.
we also keep ourselves open to running into possible issues with PHP down the line if they choose to reserve more keywords
Yep, same as with any other class which has no suffix, there are maaaaany of those, eg all controllers, models, tables, views as well as many library classes. Helpers on the other hand seem to be suffixed. If we want to avoid that issue, we should suffix all classes and don't leave it to "personal opinions" of the dev.
But as said, if you're fine with that, close this issue.
To be fair, I personally use suffixes all over the place so I don't have to do aliases within classes (i.e. use Joomla\CMS\Version as JVersion
). So I'd rather us consistently use them.
Status | New | ⇒ | Discussion |
@wilsonge can we have a decision here, would like to namespace the fields here https://github.com/joomla/joomla-cms/tree/3.8-dev/libraries/cms/form/field
Status | Discussion | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-06-22 18:43:41 |
Closed_By | ⇒ | Bakual |
As said, I seem to be in the minority. So go ahead.
I am not sure if this is the right place to ask this question, pardon me if it's not.
I am trying to make a plugin compatible with J4x. The plugin has custom fields of its own.
eg .
Custom fields are currently located at
joomla_root/plugins/myplugintype/mypluginname/fields/
joomla_root/plugins/myplugintype/mypluginname/fields/myfield.php
Plugin manifest is located at
joomla_root/plugins/myplugintype/mypluginname/mypluginname.xml
In the plugin xml, with Joomla 3.8.x I am giving fieldpath as
<fields name="params" addfieldpath="/plugins/myplugintype/mypluginname/fields">
and it works
What changes are needed for plugins to have its own fields, which resides in plugin folder itself and to make Joomla recognize those field paths with namespaces?
On Joomla 4, I tried this-
Field file located at:
joomla_root/plugins/myplugintype/mypluginname/Field/MyField.php
is having the code as
namespace Joomla\Plugins\Myplugintype\Mypluginname\Field;`
use Joomla\CMS\Form\FormHelper;
FormHelper::loadFieldClass('list');
class MyField extends \JFormFieldList
{
// Code here
}
Changed plugin xml located at
joomla_root/plugins/myplugintype/mypluginname/mypluginname.xml
to addfieldprefix, changed it as
<fields name="params" addfieldprefix="Joomla\Plugins\Myplugintype\Mypluginname\Field">
but, this does not work and custom field is not seen.
Has anyone faced similar issue with custom plugins/modules having its own fields on Joomla 4?
Sorry if this is a novice question related to the namespace. I am new to namespaces. Thanks.
Better to ask here https://groups.google.com/forum/#!forum/joomla-dev-general.
We already made this same mistake once when creating the Framework - https://github.com/joomla-framework/log/blob/master/src/Logger/Echoo.php - we can avoid making the same mistake again.