8000 [Proposal] Allow to override the recipes in project · Issue #185 · symfony/flex · GitHub
[go: up one dir, main page]
Skip to content

[Proposal] Allow to override the recipes in project #185

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.

Sign up for GitHub

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

Jump to bottom
Closed
francoispluchino opened this issue Oct 17, 2017 · 7 comments
Closed

[Proposal] Allow to override the recipes in project #185

francoispluchino opened this issue Oct 17, 2017 · 7 comments

Comments

@francoispluchino
Copy link

In the proposal #179, @Pierstoval proposes a command to reset the recipes, and explains these motivations. However, in the "Upgrade example" section, there are 2 other problems to consider: that of moving the app folder to src, template, translations, and the overwriting of the configurations by the recipes during the installation (not the case for the update), such as composer-scripts.

Before Symfony 4.0, the src folder allowed to put its code with a tree of type:

  • src/MyCompany/Component/*
  • src/MyCompany/Bundle/*

and the app folder was used to set the config, templates, translations, etc ... You can take the Sylius project in example.

However, with the new best practices, the src folder is used to put only the code of the application with the namespace App (ok, it can be modified). So, to transfer a Symfony 3.x project to Symfony 4.0, the basic idea is to move the src folder to src/App and modify the autoload. This works fine when the vendors are already installed, however when you perform a new installation, Flex copies all the files from the recipe.

The idea of creating a src folder and another folder src2 with a psr-4 "App\\": "src/App/" and "MyCompany\\": "src2/MyCompany/" is not the most relevant. We can also change the autoload to replace App\\ by MyCompany\\, but we will also have the Component and Bundle folders with Kernel.php, Entity, Controller , Repository, Migrations, etc ... which is also not pleasant.

And the idea of updating the autoload "App\\": "src/" by "MyCompany\\": "src/" is not relevant either, because the class namespace will be. it can be done, but it would be more pleasant to have these 3 namespaces:

  • MyCompany\App\Entity\Foo (and not MyCompany\Entity\Foo)
  • MyCompany\Component\Bar\Boo
  • MyCompany\Bundle\BazBundle\MyCompanyBazBundle

that to say, a tree that looks like this:

  • src/MyCompany/App/Kernel.php
  • src/MyCompany/App/Entity/Foo.php
  • src/MyCompany/App/Repositories/FooRepository.php
  • src/MyCompany/Component/Bar/Boo.php
  • src/MyCompany/Bundle/BazBundle/MyCompanyBazBundle.php

In addition, if you change the scripts.auto-scripts section to remove the "make warmup" : "script" command and replace it with a "cache: warmup": "symfony-cmd" (because your prod environment does not have the make command), Flex will add the "make cache-warmup": "script" command each time. Same for the copy of the files like Makefile that are copied each time, if you delete it.

So, what solution is there for these 2 situations, or is it simply possible to overload the configuration of the recipes in the project?

Note:
In this case, proposal #173 and the PR #174 will easily solve the problem. But a native system to override the configuration of the recipes in the project would be much more practical than creating a plugin for Composer and Flex.
@Pierstoval
Copy link
Contributor

It all depends on what you want to achieve.

When migrating a project to Flex, almost everytime you'll face the issue of the namespace changes in src/. If you don't want to use App because your old project was using more bundles, it's up to you, but indeed many recipes relying on this namespace will not take care of it.

I'm not sure about the maintainers, but I think it would be too overkill/problematic to make Flex aware of namespaces, because our PHP files would have to be modified when generated, and I personally would never like to have to handle this manually, because there are too many possible edge cases and security issues.

In the proposal #179, @Pierstoval proposes a command to reset the recipes, and explains these motivations. However, in the "Upgrade example" section, there are 2 other problems to consider: that of moving the app folder to src, template, translations, and the overwriting of the configurations by the recipes during the installation (not the case for the update), such as composer-scripts.

All of these are informations available in the recipe, I don't see why it's a problem.

In a personal project I'm migrating to Flex, I was using the Standard Edition since 2.3, and migrating to flex was a big work, but for namespaces, I just copy/pasted the sources in src/ from the old app to the new app, it's working like a charm, and I changed the Kernel namespace to the empty one and added it to autoload classmap manually. It's just about "classmap": ["src/Kernel.php"], and it works.
For the rest, this app is using 7 bundles, with least possible coupling (all of them depend on the src/UserBundle, but no other coupling), it's a big monolithic app, packages configuration was migrated easily (just composer req all your previous deps and update config files with old configuration).

About half a day to do it. And no namespace nor script issue.

@francoispluchino
Copy link
Author

Just like you, my biggest project has also been migrated from version 2.0 until last month with Flex. The migration was very fast, respecting the best practices (except for the src folder as explained), but I realized that Flex always copied the files and the configuration as soon as you reinstall your project.

Question of the namespace:

As I said before, simply changing the namespace of autoload (App to MyCompany, and not the path), having this type of tree below is also not disturbing:

  • src/MyCompany/Kernel.php
  • src/MyCompany/Entity/Foo.php
  • src/MyCompany/Repositories/FooRepository.php
  • src/MyCompany/Component/Bar/Boo.php
  • src/MyCompany/Bundle/BazBundle/MyCompanyBazBundle.php

On the other hand, what is disturbing is having a blocked namespace on App. All components and bundles are extracted to be standalone in read-only repositories, and reused in other small projects. Keeping the name of the vendor in the namespace is a mandatory prerequisite. And I'm far from being the only one.

By simply changing the App to MyCompany namespace with this structure, can Flex work as expected? Knowing that the namespace will need to be changed manually in each new PHP file.

I understand that you do not want to complexify Flex, but is not it possible to do as you did with the SensioGeneratorBundle. That is to say, to be able to put 8000 variables in the PHP file which consequently becomes a template?

For example, for Kernel.php:

<?php

namespace App;

// code

becomes:

<?php

namespace {{ app_namespace }};

// code

And Flex replace the variable during the copy. In this way you keep backward compatibility for current recipes, and for recipes that taking into account the variable, this capacity is automatically used. It is not necessary to use Twig like the made the SensioGeneratorBundle, because a simple regex is enough for variables.

I'm not sure about the maintainers, but I think it would be too overkill/problematic to make Flex aware of namespaces, because our PHP files would have to be modified when generated, and I personally would never like to have to handle this manually, because there are too many possible edge cases and security issues.

With my proposal, and a job you've already done on SensioGeneratorBundle, this would be automatic, so I do not see how it's overkill/problematic to make Flex aware of namespaces.

because there are too many possible edge cases and security issues.

I need you to develop this part because I do not see how this is problematic. SensioGeneratorBundle is not secure ??

But in the current case, what is the best practice for Flex with the namespace MyCompany\Component\* and MyCompany\Bundle\*Bundle in the src folder?

Is it better to create a 2nd folder src to place the reusable components and bundles (extracted in read-only repositories) with a 2nd configuration for the autoload?

Or, simply changing the App namespace to MyCompany is recommended?

Or, is it better to extract all components and bundles to another main repository (still with cutting in each repository in read-only)?

All my components and bundles are decoupled to the maximum, but I want to avoid having to use each repository to add/modify code, but keep the main repository for changes, and pusher the code in each repository for components and bundles (in the same way as symfony/symfony).

Question of the recipe configurations:

Currently, the problem of domain names is not an emergency, it is just not DX for big projects. Regarding the configuration of recipes:

All of these are informations available in the recipe, I don't see why it's a problem.

The problem, and I explained it with a concrete case on the composer's scripts. Very well, yesternight you deleted the support of Makefile here, here and here in the Symfony FrameworkBundle recipe.

So now, Flex does not attempt to add to each reinstall the make cache-warmup script replaced by a custom cache: clear command. On this subject, cache:clear without --no-warmup is no longer depreciated for version 3.3, but I haven't found anything about Flex and Symfony 4.0.

Indeed, with all the changes made, leave the possibility to the project modified recipes is no longer really useful. However, Flex interactively asks if we want to install the recipe during the first installation. For the first time, it's developer friendly, but if you answered no, the next time you reinstall the project, Flex will ask again the question.

Is it not possible to save the answers for each package in the extra section of the composer.json file?

@francoispluchino
Copy link
Author

By answering, I forgot the part of the environment variables. Currently, it is impossible to change the default values of environment variables because Flex overwrites each time the entire section of each package.

For example, for Doctrine ORM, it is configured on Mysql:

DATABASE_URL="mysql://root@127.0.0.1:3306/symfony?charset=utf8mb4&serverVersion=5.7"

So if you want to set another default value to set PostgreSQL, this is not possible. Do you have anything to fix this problem?

@Pierstoval
Copy link
Contributor

The "overwrite" part of flex exists only when doing a composer require if you don't have the package installed yet in your project. The "reset" part was just a proposal in #179, it has not even been discussed by anyone and is just a first shot, because Flex does nothing today about this.

For the interaction part about asking questions to the user, @fabpot expressed many times that he didn't want Flex to interact with the user for better DX, the only "accepted" interaction now is the fact that contrib recipes need user approval if you don't set it up in your composer.json file, which can be done once and for all by saying "yes and for all" when Flex asks the first time (which prevents further interactions). So any kind of interaction about packages, namespaces or whatever seem not to be a viable option for now, especially for DX.

Next, about namespaces, which seems to be your biggest need:

The namespace {{ app_namespace }}; part in recipe files doesn't make sense to me.

What is the "app" namespace? The default App one configured when creating the project with the skeleton? Or the MyCompany one? And what if you don't use this App namespace and just register namespaces manually via "Controller\\": "src/Controller" or (worse for performances, but valid) "": "src/"? What namespace will be used then?

And even for your app, it's a monolithic repo that creates a subsplit of every repo in read-only packages, it's great, but then your app is one specific case.
The one I have for the project I'm migrating to Flex I've talked before is different: I have not-standalone bundles that rely all on the UserBundle one, I just have 7 psr-4 rules that will register my namespaces and I deleted the old App one. This is another specific case.

The "default" for everyone is to use this App namespace because it's the easiest to understand. Having an empty namespace is not a good practice for autoloading performances reasons, and manually setting namespaces like Controller: src/Controller or Entity: src/Entity doesn't make sense because people might have different habits (like ADR pattern with Action instead of Controller, using NoSQL and Document instead of Entity, etc.).

Default ways of doing things is about recommendations, and bundleless apps have been a recommendation since a few years now (I think it has started at the same time the official Symfony installer release, IIRC).

I don't see why we should add too much complexity over simple already-configurable namespaces when we recommend user to use the default setup and customize it.

@francoispluchino
Copy link
Author

Since I notice annoyance on your part, and some confusion between my comments and your reactions, therefore, I suppose my English should not be very clear. In this case, I apologize, and you will understand that I try to be the most understandable and respectful when I express myself. However, my English is not equivalent to my French. So I'll try to be clearer :-).

What is the "app" namespace? The default App one configured when creating the project with the skeleton? Or the MyCompany one? And what if you don't use this App namespace and just register namespaces manually via "Controller\": "src/Controller" or (worse for performances, but valid) "": "src/"? What namespace will be used then?

I understand that there is a confusion between us, because I had an old version of Flex, and the behaviors explained are no longer effective with the 1.0.31 version, except for the first installation of Flex (but that's normal). Indeed, if we modify the autoload to take account of the custom namespaces in the src folder:

  • the namesapce App\\ in the folder src/App (for any code dedicated to the project)
  • the namesapce MyCompany\\ in the folder src/MyCompany (containing all reusable components and bundles)

Flex will copy the recipe files into src and not in src/App, as logically expected by the autoload configuration.

To be clearer, my current configuration of autoload is:

{
    "autoload": {
        "psr-4": {
            "App\\": "src/App/",
            "MyCompany\\": "src/MyCompany/"
        }
    },
}

By responding to you and noticing the difference in behavior with the latest version, I understand your astonishment about the variable in the recipe to change the namespace to the installation, and my proposal is useless if Flex is able to take into account the proper folder for the App namespace.

So, is it possible to take into account the folder where is the App namespace configured in the Composer autoload?This will copy the files into the correct folder. This is standard, and this makes it easier to use Flex by avoiding moving files every time for a custom autoload.

Also, do not consider my comment on the custom values overwriting environment variables in env.dist and phpunit.xml.dist, I did not have the latest version of Flex, and I no longer have this behavior with the version 1.0.31.

Having an empty namespace is not a good practice for autoloading performances reasons, and manually setting namespaces like Controller: src/Controller or Entity: src/Entity doesn't make sense because people might have different habits (like ADR pattern with Action instead of Controller, using NoSQL and Document instead of Entity, etc.).

As mentioned above, I use the App namespace as recommended: dedicated to the project, and it is very small compared to the rest (components and bundles), because there are the entity classes, the migration of the database, as well as the Kernel.php file. Twig templates and translations were recently moved to new folders (and so they are no longer in the src/App/Resources folder).

The "overwrite" part of flex exists only when doing a composer require if you don't have the package installed yet in your project. The "reset" part was just a proposal in #179, it has not even been discussed by anyone and is just a first shot, because Flex does nothing today about this.

I was just taking your use case. I noticed that it was only a proposal :-).

For the interaction part about asking questions to the user, @fabpot expressed many times that he didn't want Flex to interact with the user for better DX, the only "accepted" interaction now is the fact that contrib recipes need user approval if you don't set it up in your composer.json file, which can be done once and for all by saying "yes and for all" when Flex asks the first time (which prevents further interactions). So any kind of interaction about packages, namespaces or whatever seem not to be a viable option for now, especially for DX.

Sorry, I follow the conversations, but I have not read any information about it, so thank you for the info, and at the same time, I agree on this point.

Conclusion:

Hopefully you understand that I appreciate and respect the work of the entire Symfony team and the contributors of which I am a part, and that I am only asking questions or proposing solutions to concrete cases. But for this use case, wasn't really useful, given that this was not the behavior of the latest version of Flex.

I am aware that there was confusion, and to sum up, instead of taking my original proposal that is out of date, is it possible to take into account the namespace folder App configured in the autoload? And not arbitrarily using the src folder?

Note: This should probably be another issue.

Thank you for your help.

@francoispluchino
Copy link
Author
francoispluchino commented Oct 19, 2017 •
edited
Loading

I have looked at the Flex code in more detail, and there are options that are not used, including "src-dir":"src". This would fix this issue if this option were implemented.

@francoispluchino francoispluchino mentioned this issue Oct 19, 2017
Closed
@francoispluchino
Copy link
Author

The option src-dir work fine, so this issue can be closed.
5907

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@francoispluchino @Pierstoval
0