started preparing the API changes for the 5.0.0 release notes.
Should probably be moved to a change record at some point.
I'm planning to release a first beta version soon, thanks a lot for testing the alpha release (2 issues on that fixed already in 5.x dev).
Merged the small fixes in the pull request, thanks @almunnings!
I'm not sure that this will fix the cache issue here, please test if this helps.
Then I can make a new 4.x release soon.
merge request created.
Merged that one already, let me know if you still see a problem!
This should do it, please test!
Aha good point, I should have checked Drupal core plugins.
Is it enough to just add this property to the attribute classes? Or do we have to do anything on the plugin managers as well? Probably not, as you say the property is defined on the parent Plugin attribute class.
Thanks for reporting!
I had the same problem in our installation because the 2 int properties were missing in our graphql entity server config:
query_depth: 20
query_complexity: 0
I wrote a custom update function to work around that:
/**
* Set the missing query_depth and query_complexity values.
*/
function jobiqo_graphql_update_10103(): void {
\Drupal::configFactory()
->getEditable('graphql.graphql_servers.jobiqo_schema')
->set('query_depth', 20)
->set('query_complexity', 0)
->save();
}
The int values were introduced in 🐛 5.x fatal error creating new severs Active .
We have 2 options to solve this:
1. Implement constructor on the Server entity and always cast those 2 to int
2. Write an update function to update all graphql server entities that miss those 2 properties, where they are not int.
I'm inclined to go with option 2, so that we don't have to maintain workaround code in the constructor.
GraphQL 5.0.0-alpha3 released!
Mostly refactoring to use PHP attributes and some bug fixes.
Thank you Daniel Bosen, Al Munnings, Andrii Aleksandrov and Ben Stallings for contributing fixes!
https://www.drupal.org/project/graphql/releases/5.0.0-alpha3 →
I think we can do the first beta soon, let me know of any blockers you see.
Thanks, merged!
Thanks, merged!
Not the best test as it would be better to test a real entity, but I think we don't care.
merged.
Thanks, I think we need to expand the test coverage to test with string and TranslatableMarkup for the description.
Ah ok, I don't think we need a functional test, we can just use a real entity. Can you create a node in the test and use that instead of a mock? We have a database connection available in our kernel tests.
Thanks, that is surprising! Can you add a test case? Looks like test coverage was missing before, but we should ensure the code is tested.
Pull request ready.
This was the final prompt for AI: "convert the doc block annotation to php attributes. use statement: "use Drupal\graphql\Attribute\DataProducer;". For ContextDefinition use Drupal\Core\Plugin\Context\ContextDefinition. keep the top doc comment sentence. use named parameters for all ContextDefinition constructors, the data type property is called "data_type". Use TranslatableMarkup for @Translation. make sure the use statements on top of the file are sorted alphabetically. for each property of ContextDefintion use a new line. If the data type of a context definition starts with "entity:" then use the class EntityContextDefinition with use statement Drupal\Core\Plugin\Context\EntityContextDefinition. If the data type of a context definition is exactly "entity" then use the class ContextDefinition with use statement Drupal\Core\Plugin\Context\ContextDefinition. Make sure that all multiline array elements have a comma at the end. Ensure that all context definitions in "consumes" are present in the attribute conversion and have the same name. the number of entries in "consumes" needs to be the same. If the class already has an attribute then skip the file."
I took me 5 hours to create this pull request. Was I really faster with AI? Maybe a bit, but fiddling and reviewing changes also takes time. This might be a typical case where a developer thinks they are more productive with AI and suddenly 5 hours have passed.
At least I enjoyed the refactoring more as I did not have to type out or copy/paste all the modifications. I would estimate that 95% of the changes the AI created correctly and the 5% rest I had to fix manually.
I'm very thankful for the automated tests, which caught a lot of small mistakes that I then had to feed back into the prompt.
I used Github copilot in agent mode with model Gemini 2.5 Pro.
Merged.
Merged.
Merged.
Nice, thank you!
Merged.
I will try to convert the annotations to attributes with AI, as the Drupal rector tool did not work for me. Issue: 📌 Use attributes to define DataProducer plugins Active
Merged.
Is the rector code supposed to work with contrib plugin annotations and attributes supposed to work? Tried to convert graphql dataproducer plugins, but I get no output.
Rector config:
<?php
declare(strict_types=1);
use Drupal\graphql\Attribute\DataProducer;
use DrupalRector\Drupal10\Rector\Deprecation\AnnotationToAttributeRector;
use DrupalRector\Drupal10\Rector\ValueObject\AnnotationToAttributeConfiguration;
use Rector\Config\RectorConfig;
return static function (RectorConfig $rectorConfig): void {
$rectorConfig->ruleWithConfiguration(AnnotationToAttributeRector::class, [
new AnnotationToAttributeConfiguration('10.0.0', '10.0.0', 'DataProducer', DataProducer::class),
]);
if (class_exists('DrupalFinder\DrupalFinderComposerRuntime')) {
$drupalFinder = new DrupalFinder\DrupalFinderComposerRuntime();
} else {
$drupalFinder = new DrupalFinder\DrupalFinder();
$drupalFinder->locateRoot(__DIR__);
}
$drupalRoot = $drupalFinder->getDrupalRoot();
$rectorConfig->autoloadPaths([
$drupalRoot . '/core',
$drupalRoot . '/modules',
$drupalRoot . '/profiles',
$drupalRoot . '/themes'
]);
$rectorConfig->skip(['*/upgrade_status/tests/modules/*']);
$rectorConfig->fileExtensions(['php', 'module', 'theme', 'install', 'profile', 'inc', 'engine']);
$rectorConfig->importNames(true, false);
$rectorConfig->importShortClasses(false);
};
Invocation:
vendor/bin/rector process modules/graphql --dry-run
output:
klausi@drupal-11-web:/var/www/html$ vendor/bin/rector process modules/graphql
36/68 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░] 52%
In ProcessPool.php line 30:
Process "qabrfesumt" not found.
Looks like rector is crashing here?
Did anyone convert contrib annotation successfully with an automated tool?
Merged, thanks!
Merged!
Thanks for reporting - I would like to take a slightly different approach by refactoring this config setup into a common method on the schema plugin manager.
Resolved merge conflict, uploading stable patch file for composer patches.
Merged in the 2.0 branch and resolved conflicts.
Uploading stable patch file for composer patches.
Merged, thanks!
Cleaned up test fails in 🐛 Method Drupal\term_merge\Form\MergeTerms::__construct() invoked with 2 parameters, 3 required. Active .
Merged.
klausi → created an issue.
Thanks!
The tests fails look unrelated to me and should be cleaned up in a separate issue.
thanks, approach makes sense to me. Just one confusion about range().
Merged.
Fixed gitlab pipeline fails in 📌 Fix Gitlab pipeline in 4.x Active , merged that into the MR.
Put it on the merge train, should be committed soon.
Implemented casting to int and merged it, thanks!
OK sorry, can reproduce it now, but for some reason my merge request fixes it when I switch from "?int" to "int". Not sure why this works, but it does. Probably because of the new default value 0.
I thought about setting some non-zero default values for those limits, but they are really application specific and for me it is hard to come up with a value that will be good enough for any Drupal instance. Let me know if you have some inputs there!
New approach is in https://git.drupalcode.org/project/graphql/-/merge_requests/50
I'm super confused by this issue because I cannot reproduce it.
The error makes sense and I assume it would happen, but does not trigger on my local dev site.
What Drupal core version did you use? Which PHP version?
This should not be needed anymore, closing.
Merging in the 5.x branch fixed the composer fail.
Merged, thanks!
Hi, not sure why we need to fix this, the pipeline on 5.x is passing: https://git.drupalcode.org/project/graphql/-/pipelines/558569
Where is it failing for you? Can you add a link to the issue summary?
Thanks! As there are no PHP fatal errors in our tests I think we can commit this without adding test coverage. Just to make PHPStan happy!
Thanks! I think casting to string here is ok as the result of a dataproducer should probably not be a Translatable markup object.
Thanks, can you also fix examples/graphql_example/src/Wrappers/QueryConnection.php and cleanup up phpstan.neon so that all \Drupal ignores and the comment is removed?
Marking this as fixed, any further doc improvements can be done in a new issue.
Hm, please don't do rebases in merge requests. Now it is very hard for me to review your changes you added. Always do merges form the main branch in merge requests.
composer.lock looks good now, and if I interpret the rebase correctly there are no other changes. Looks ready!
Not sure why it would check JS files ... the default phpcs file from the gitlab template does not use JS files https://git.drupalcode.org/project/gitlab_templates/-/blob/main/assets/p...
Maybe try to copy that and list the extensions?
merge request created.
klausi → created an issue.
Not sure why the default batch limit is set to 10, that is way too low.
In Drupal 7 we use 500 for the variable search_api_saved_searches_load_max.
Should we update the default value on install to 500?
Uploading stable patch file for composer patches.
Created MR, what do you think?
klausi → created an issue.
Agreed - please put any bad words in the cspell config of Drupal core and the cspell config of the gitlab template.
Thanks, looks good to me except drupal/core-recipe-unpack being added to composer.lock. Can you revert that?
Coder 8.3.30 released!
https://www.drupal.org/project/coder/releases/8.3.30 →
Please update that in composer, then we can move to RTBC I think.
Nice, looks all good to me.
Then I will make a new Coder release now and we can use that.
Sure, I'm currently not working on it!
One nice find with the improved concat sniff: this exposed a bug in test code, yay! The test used ".." but should be "." https://git.drupalcode.org/project/drupal/-/merge_requests/12018/diffs#6...
Found one more Coder issue: 🐛 Compley array<> expressions should be allowed in @return comments Active .
I think we should upgrade straight to Coder 8.3.30 here (not released yet).
I started to work through the newly reported issues and also removed deprecated sniffs. This will not pass yet, but let me know if you like the approach.
I tested with a git checkout of Coder in the vendor directory, to ensure I'm running on the latest version of Coder. Once everything is passing here as we want it I can make a new Coder release.
Merged.
While testing the latest Coder dev version on core I detected another phpcs ignore inconvenience and fixed it in 📌 Allow phpcs:ignore before param comments Active .
Merged.
Note that CSS support has been removed from Coder. To check and fix CSS files please use Stylelint and use the Drupal core .stylelintrc.json configuration file.
Note that CSS support has been removed from Coder. To check and fix CSS files please use Stylelint and use the Drupal core .stylelintrc.json configuration file.
Note that CSS support has been removed from Coder. To check and fix CSS files please use Stylelint and use the Drupal core .stylelintrc.json configuration file.
Note that CSS support has been removed from Coder. To check and fix CSS files please use Stylelint and use the Drupal core .stylelintrc.json configuration file.
Yeah, I set it to hidden. Also added the PR link in the issue summary.
klausi → changed the visibility of the branch 3524264-deprecate-all-css to hidden.
We use cspell now to check for spelling, so another approach would be to add more bad flagWords into core's .cspell.json file. But I think that has the downside that it would not propagate to Drupal contrib projects, they would have to add the same flagWords to their cspell config? Not sure how the cspell gitlab job works.
So a sniff in Coder could be the more universal approach.
Fully agreed. Merged, thanks!
Alright, merged that now. Thank you all!
Thanks, did some manual test runs myself and realized we cannot implement DeprecatedSniff. The problem is that we cannot silence the deprecation from our ruleset.xml file. That means all users will always get the deprecated message which they cannot fix.
My workaround is now to just make the deprecated sniffs empty so that they don't check anything anymore. Not great, but at least it will not break the CI pipelines for people that still reference those sniffs in their config.
I updated the issue summary with instructions for the release notes.
I see, and is your global "phpcs" command pointing to the dev version of your Coder install?
What does "which phpcs" return? Is it the path where you installed Coder 8.3.x from git and PHPCS with it in the vendor directory?
Sorry, I used the wrong words. I meant PHP attributes:
/**
* Test with attribute before.
*/
// @phpstan-ignore-next-line
#[ExampleAttribute('foo', 'bar')]
public function ignore_phpstan_comment() {
}
and
/**
* Test with attribute after.
*/
#[ExampleAttribute('foo', 'bar')]
// @phpstan-ignore-next-line missingType.return
public function ignore_phpstan_comment() {
}