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

filament/support subtree split incorrectly requires doctrine/dbal #11891

Closed
jackwh opened this issue Mar 16, 2024 · 3 comments
Closed

filament/support subtree split incorrectly requires doctrine/dbal #11891

jackwh opened this issue Mar 16, 2024 · 3 comments
Labels
bug Something isn't working low priority unconfirmed

Comments

@jackwh
Copy link
Contributor

jackwh commented Mar 16, 2024

Package

filament/filament

Package Version

v3.2.48

Laravel Version

v11.0.7

Livewire Version

No response

PHP Version

PHP 8.3

Problem description

Whilst updating to Laravel 11, I spotted a dependency oddity which seems accidental (though I'm not 100% sure). Here's a summary:

  1. PR Laravel 11 #10972 removed doctrine/dbal from Filament, in line with Laravel 11: filamentphp/support@9f37568
  2. Later, this commit (inadvertently?) restored it as a requirement: filamentphp/support@a5fb3a2
  3. Meanwhile, Filament's latest v3.2.50 release does not require this.

I noticed this as it prevents upgrading Filament beyond v3.2.40 when used alongside joelbutcher/socialstream, a popular package with Filament integration, which requires doctrine/dbal v4.0.

I assume this is unintentional but may be misunderstanding how the subtree split works.

Expected behavior

doctrine/dbal should not be required within the subtree split of filament/support.

Steps to reproduce

Try to install Filament alongside any project which requires doctrine/dbal v4 (e.g. https://github.com/joelbutcher/socialstream)

Reproduction repository

https://github.com/joelbutcher/socialstream

Relevant log output

composer require "filament/filament:3.2.48" --dry-run --with-all-dependencies
./composer.json has been updated
Running composer update filament/filament --with-all-dependencies
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - filament/support v3.2.48 requires doctrine/dbal ^3.2 -> found doctrine/dbal[3.2.0, ..., 3.9.x-dev] but these were not loaded, likely because it conflicts with another require.
    - filament/filament v3.2.48 requires filament/support v3.2.48 -> satisfiable by filament/support[v3.2.48].
    - Root composer.json requires filament/filament 3.2.48 -> satisfiable by filament/filament[v3.2.48].


Installation failed, reverting ./composer.json and ./composer.lock to their original content.
Copy link

Hey @jackwh! We're sorry to hear that you've hit this issue. 💛

However, it looks like you forgot to fill in the reproduction repository URL. Can you edit your original post and then we'll look at your issue?

We need a public GitHub repository which contains a Laravel app with the minimal amount of Filament code to reproduce the problem. Please do not link to your actual project, what we need instead is a minimal reproduction in a fresh project without any unnecessary code. This means it doesn't matter if your real project is private / confidential, since we want a link to a separate, isolated reproduction. That would allow us to download it and review your bug much easier, so it can be fixed quicker. Please make sure to include a database seeder with everything we need to set the app up quickly.

Also, it doesn't look like you've provided much information on how to replicate the issue. Please edit your original post with clear steps we need to take.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 16, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Roadmap Mar 16, 2024
Copy link

Thank you for providing reproduction steps! Reopening the issue now.

@danharrin
Copy link
Member

danharrin commented Mar 17, 2024

Filament used to use DBAL for resource generation.

That PR replaced DBAL with Laravel's own implementation of database analysis. The dependency was removed.

We then received reports of users using DBAL in their apps (#11429 (comment)). This is user error. They should be installing DBAL themselves instead of relying on the fact that Filament required it.

However, due to the fact that this was kind of a breaking change, we decided to re-require DBAL to prevent those apps from breaking. This was a gesture of good will from Filament that we didn't need to do since it was user error, but thought it would make the situation more stable.

So we will require DBAL until Filament v4 for users still depending on that incorrectly. If you want to allow DBAL v4 to be installed in the constraint, go ahead and make a PR.

@danharrin danharrin closed this as not planned Won't fix, can't repro, duplicate, stale Mar 17, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Roadmap Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working low priority unconfirmed
Projects
Status: Done
Development

No branches or pull requests

2 participants