Errors with RockMigrationsConstants Traits

[gebeer]

Hi @bernhard,

I already fell in love with the new config migrations feature in RockMigrations > 6.0.0. I am currently on v6.0.1

So much cleaner to have migrations organized in this way.

I just tried to implement the part about the constant traits, described at https://www.baumrock.com/en/processwire/modules/rockmigrations/docs/config-migrations/#class-constant-traits

First it would not work at all as described in the docs. I noticed that my file at site/modules/Site/RockMigrations/RockMigrationsConstants.php never got picked up because of searching for those files did not recurse into module subdirectries. Fixed in PR#70 commit  https://github.com/baumrock/RockMigrations/pull/70/commits/faba5475a008ab14415c0e35f2c5c78d2e91f0b1.

First problem:

After fixing that, I get the Compile Error
Traits cannot have constants

Actually, constants are only allowed in Traits since PHP 8.2.

So this would change the module dependencies from PHP>=8.0 to PHP>=8.2

I don't think this was intended. But it should be updated or at least documented that the automated constants thingy requires PHP>=8.2.

Second problem:

After upgrading to PHP8.2 I ran into the next issue:

This is caused by the constant name "template_basic-page" containing a hyphen. As it seems, they are not allowed in constant names in PHP at all (https://www.php.net/manual/en/language.constants.php):

Quote

The name of a constant follows the same rules as any label in PHP. A valid constant name starts with a letter or underscore, followed by any number of letters, numbers, or underscores. As a regular expression, it would be expressed thusly: ^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$

For years I have naming conventions for templates and fields in PW. Template names always contain hyphens like in "basic-page". Field names always contain underscores like in "my_field". This is also helpful when looking at code to discern templates from fields on the first glance.

I fixed this by replacing - with _ in constant names. See https://github.com/baumrock/RockMigrations/pull/70/commits/a06c6062c61b5264932a8044cf25bac0c689f8cf

The issue with PHP8.2 still remains to be resolved. I suggest adding the requirement only to docs  about RockMigrationsConstants.php because other than that RockMigrations seemed to work fine on 8.1 in my tests.

 

[bernhard]

Hey @gebeer thanks for trying out Config Migrations so early 🙂 💪

10 hours ago, gebeer said:

I already fell in love with the new config migrations feature in RockMigrations > 6.0.0.

Same here 😍 

10 hours ago, gebeer said:

First it would not work at all as described in the docs. I noticed that my file at site/modules/Site/RockMigrations/RockMigrationsConstants.php never got picked up because of searching for those files did not recurse into module subdirectries. Fixed in PR#70

The file should not be in /site/modules/Site/RockMigrations/... but should live side by side with Site.module.php! I just had a look into the docs and you are right - the instructions there are like you say. I have to look into what you said tomorrow, but if you have time and read this in the meantime, then I'd appreciate if you could try to create the file /site/modules/Site/RockMigrationsConstants.php and see if that maybe fixes some other of your mentioned issues?

10 hours ago, gebeer said:

The issue with PHP8.2 still remains to be resolved. I suggest adding the requirement only to docs  about RockMigrationsConstants.php because other than that RockMigrations seemed to work fine on 8.1 in my tests.

Thx a lot! I'll likely do exactly that 🙂 As of 2 days ago we have PHP8.4 released and PHP8.1 gets security fixes only - so I think it would even be fine to have PHP8.2 as a requirement for the module. Any other votes?

[gebeer]

7 hours ago, bernhard said:

hen I'd appreciate if you could try to create the file /site/modules/Site/RockMigrationsConstants.php and see if that maybe fixes some other of your mentioned issues?

This solves only the first problem that I described.

The error caused by hyphens in constant names still persists, even after switching to PHP8.3. Do you name all your templates with underscores like "basic_page" to avoid that?

On a side note, after reading up on PHP constants, there seems to be a convention to name them in capital letters.

[bernhard]

7 hours ago, gebeer said:

The error caused by hyphens in constant names still persists, even after switching to PHP8.3. Do you name all your templates with underscores like "basic_page" to avoid that?

Yes I always use underscores. I had problems with hyphens several times. I can't remember any more where exactly, but I remember that avoiding them was the easiest solution.

7 hours ago, gebeer said:

On a side note, after reading up on PHP constants, there seems to be a convention to name them in capital letters.

Any sources for that? PW definitely uses camelcase constants like Inputfield::collapsedHidden which I think looks nicer than Inputfield::COLLAPSEDHIDDEN. But I'd be happy to get some links to read.

[gebeer]

6 minutes ago, bernhard said:

Any sources for that? PW definitely uses camelcase constants like Inputfield::collapsedHidden which I think looks nicer than Inputfield::COLLAPSEDHIDDEN. But I'd be happy to get some links to read.

https://www.perplexity.ai/search/php-constants-naming-conventio-g4mLG8sbQyu34r4mGXmx6w 

Although the source for this is not php.net directly. I wanted to read up on that over there but php.net currently throws a 502 error.

Anyways, that was just a side note.

8 minutes ago, bernhard said:

Yes I always use underscores

That explains why you never saw that error. But imo you should not assume that everybody uses underscores so I think, the renaming logic in my PR at https://github.com/baumrock/RockMigrations/pull/70/commits/a06c6062c61b5264932a8044cf25bac0c689f8cf should be implemented.
Otherwise people who use hyphens in their template names would have to rename all their templates just to be able to use the RockMigrationsConstants magic.

[da²]

On 11/24/2024 at 3:31 AM, gebeer said:

On a side note, after reading up on PHP constants, there seems to be a convention to name them in capital letters.

Usually languages uses this convention for constants, caps and underscore to separate words : Inputfield::COLLAPSED_HIDDEN

I add a doubt but yes, PHP 8.2 supports constants in traits.

Quote

Traits can, as of PHP 8.2.0, also define constants.

<?php
trait ConstantsTrait {
    public const FLAG_MUTABLE = 1;
    final public const FLAG_IMMUTABLE = 5;
}

class ConstantsExample {
    use ConstantsTrait;
}

$example = new ConstantsExample;
echo $example::FLAG_MUTABLE; // 1
?>

About the hyphen in variable name, I bet most languages would refuse it, because hyphen is an operator.

Edited 8 hours ago by da²

[bernhard]

7 minutes ago, da² said:

<?php
trait ConstantsTrait {
    public const FLAG_MUTABLE = 1;
    final public const FLAG_IMMUTABLE = 5;
}

class ConstantsExample {
    use ConstantsTrait;
}

$example = new ConstantsExample;
echo $example::FLAG_MUTABLE; // 1
?>

Not sure what you are trying to say, but that's exactly how the new config traits work.

I've added a note about PHP8.2 to the docs: https://github.com/baumrock/RockMigrations/commit/4e776f5185ce19866fd853c87a9acec47f4e7485

[da²]

6 minutes ago, bernhard said:

Not sure what you are trying to say

This is an example from PHP documentation. Just saying that I discovered right now that constants are now allowed in traits since 8.2. ^^