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

How to handle prefixed dependencies also used by dev-dependencies #81

Open
justlevine opened this issue Nov 21, 2023 · 9 comments
Open
Labels

Comments

@justlevine
Copy link

justlevine commented Nov 21, 2023

I've been struggling to understand the best way to handle prefixing a dependency that is in use by both the project itself and some of the project dev dependencies. I am unsure if this is a bug or a usage issue.

For example, over in this PR for axewp/wp-graphql-headless-login, we're namespacing several production dependencies that require guzzlehttp/guzzle. However, that package is also required by our dev-deps codeception/module-phpbrowser, which fails since it doesn't know to use the namespaced version of the classes, causing errors like:
PHP Fatal error: Uncaught Error: Class "GuzzleHttp\HandlerStack" not found in /var/www/html/wp-content/plugins/wp-graphql-headless-login/vendor/codeception/module-phpbrowser/src/Codeception/Lib/Connector/Guzzle.php:342 here.

What I've tried

  • delete_vendor_packages doesn't seem to prefix the usage of deps in non-namespaced packages, nor does it happen if the target_directory is set to vendor.
  • exclude_from_copy and exclude_from_prefix don't make sense here, since we explicitly want the package to be namespaced to prevent conflicts with other plugins.
  • Also tried setting delete_vendor_packages to false, and to conditionally require vendor/autoload.php, however I couldnt find an option for strauss to copy the PSR-4 autoloader over to vendor-prefixed/autoload.php, and since delete_vendor_packages doesn't seem to be configurable on a per-package level that means our distribution zip needs to include duplicates of all namespaced dependencies.
  • It doesnt seem like config.strauss provides a way to have separate commands for composer install --no-dev (e.g. delete copy from vendor) and composer install (e.g. dont delete copy from vendor), which would let me use both the strauss and composer autoloaders and still keep the duplicates out of the production build.

Is this a bug, known limitation, or just something wrong with how I'm approaching strauss?

@justlevine justlevine changed the title How to handle prefixed dependencies also used by dev-dependencies. How to handle prefixed dependencies also used by dev-dependencies Nov 21, 2023
@defunctl
Copy link

I just ran into this as well, with a prefixed Symfony library used by both the Codeception tests library and in our code base.

@BrianHenryIE
Copy link
Owner

BrianHenryIE commented Nov 23, 2023

I'm not sure why you're ever getting a PHP error – since your project code should be using the copied + prefixed class and the original should still exist in your vendor directory. I use WP Browser/Codeception pretty widely and haven't had a problem.

I see why it's a problem that the unprefixed package still remains after composer install --no-dev.

I'll break up the tool so commands can be run individually.

I'm already working on that as part of a feature to prefix the call sites in project files. Which would also give the option of doing all development using the true namespaces and only prefixing as a build step. 2bd1b3f

Another approach which might work (but I don't currently plan to do) would be for Strauss to have a class_alias() map which could be written on each run and loaded by a files autoloader. Edit: this is probably the neatest way to do it when target_directory is set to vendor

@justlevine
Copy link
Author

justlevine commented Nov 23, 2023

I'm not sure why you're ever getting a PHP error – since your project code should be using the copied + prefixed class and the original should still exist in your vendor directory. I use WP Browser/Codeception pretty widely and haven't had a problem.

The PHP error occurs when the namespaced dependency (GuzzleHttp/Guzzle) is deleted/replaced in situ, since the dev-dependency (Codeception) needs the nonprefixed copy. Codeception runs, tries to use a class from GuzzleHttp that doesn't exist, and fatals out.

If the original is not deleted, then I need to bundle both the original (for use by the dev deps) and the prefixed dependency (for use by project code) in production, since I need the PSR-4 autoloader from composer.json.
And since delete_vendor_packages is all-or-nothing, that means I need to include duplicates of all the renamed packages as well. (All of which become subject to dependency conflicts if my plugin is installed on a wp-composer setup)

Hope this clarify things. If not, happy to try again.

As far as possible solutions go, Im over my head and happily defer to what you think the ideal mechanism would be. Will note that an additional possible solve would be to have Strauss provide the option to bring over the composer.json "autoload" config so a production build could solely rely on vendor-prefixed/autoload.php, but no idea if that's a better/worse pattern than what you described.

@defunctl
Copy link

defunctl commented Nov 23, 2023

Same deal for me, with "delete_vendor_packages": true, I get:

Fatal error: Uncaught Error: Interface "Symfony\Contracts\Service\ResetInterface" not found in /var/www/html/wp-content/plugins/project/vendor/symfony/console/Application.php on line 62

Because Symfony\Contracts\Service\ResetInterface is also a dependency of another package in require: so it has been prefixed, moved and deleted, but symfony/console/Application.php still implements the original interface because its part of a require-dev: dependency.

@BrianHenryIE
Copy link
Owner

I've started a branch which will probably take me months to finish.

The idea is to track each file which is copied/deleted/prefixed and after changes have a list that can be queried. From that, I intend to build a file with a class_alias(...) list which should address the problem at the root of this issue – dev dependencies will look up the old namespace/classname and use the renamed one. There's a related files autoloader issue which this will help me fix too. Broadly, this will help move towards a Composer plugin structure where changes are all in-place and composer install --no-dev works smoothly too.

https://github.com/BrianHenryIE/strauss/compare/master...BrianHenryIE:strauss:use-files-objects-with-properties?expand=1

@defunctl
Copy link

@BrianHenryIE what do you think about adding a Strauss command line argument to configure delete_vendor_packages during execution?

Right now we get around this issue by setting that to false, but for a production zip, we'd like to delete the vendor packages, so like strauss.phar --delete-vendor-packages flag or option strauss.phar --delete-vendor-packages=true|false or something along those lines we could run to bypass the composer.json config.

We are just changing with jq for the time being, but an arg/option would be cleaner.

@BrianHenryIE
Copy link
Owner

Yeah, absolutely. I only recently added a first CLI option and started writing progress to the terminal. I'll try add that soon. It should be easy enough to add, but probably a pain to add a unit test given the current state of that class! I did look into it a bit before and there was something weird about handling both a --delete-vendor-packages and --delete-vendor-packages=true, which IMO should both work.

@BrianHenryIE
Copy link
Owner

OK, new feature, undocumented except in release notes, vendor/bin/strauss --deleteVendorPackages=false73d2d3e

Overrides the composer.json configuration.

Still plenty to do on this issue.

@defunctl
Copy link

@BrianHenryIE finally had a chance to test this, working well. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants