@smustgrave
> Gone back n forth but since it was a constant used through out can we add simple test coverage that if you use any of these deprecated constants you get a warning.
I couldn't find a way to trigger a runtime warning for constants. The Drupal docs have suggestions for other similar cases but not for constants:
https://www.drupal.org/about/core/policies/core-change-policies/drupal-d... →
Also looked into core's various tests but couldn't find anything that would fit here. Do you have an example?
Attached MR adds the option of configuring the title and placeholder HTML properties for search block forms.
A new test is included and as many dependents as I noticed (umami, olivero, help) have been updated.
diegoe → changed the visibility of the branch 760182_diegoe_search-block-title-and-placeholder to hidden.
diegoe → changed the visibility of the branch 3447950_diegoe_specific-phpstan-ignores to hidden.
diegoe → made their first commit to this issue’s fork.
The attached MR updates core to use specific phpstan error identifiers.
Unfortunately, phpstan-drupal hasn't updated to use the new Error Identifiers feature, so we can't ignore the specific errors coming from it. These remaining ignores are:
core/tests/Drupal/FunctionalTests/Core/Container/ServiceDeprecationTest.php: // @phpstan-ignore-next-line
core/tests/Drupal/FunctionalTests/Core/Container/ServiceDeprecationTest.php- \Drupal::service('deprecation_test.service');
core/tests/Drupal/FunctionalTests/Core/Container/ServiceDeprecationTest.php: // @phpstan-ignore-next-line
core/tests/Drupal/FunctionalTests/Core/Container/ServiceDeprecationTest.php- \Drupal::service('deprecation_test.alias');
--
core/tests/Drupal/Tests/Core/Render/RendererCallbackTest.php: // @phpstan-ignore-next-line
core/tests/Drupal/Tests/Core/Render/RendererCallbackTest.php- ['#pre_render' => ['\Drupal\Tests\Core\Render\callback'], '#type' => 'container'],
--
core/tests/Drupal/Tests/Core/Render/RendererCallbackTest.php: // @phpstan-ignore-next-line
core/tests/Drupal/Tests/Core/Render/RendererCallbackTest.php- ['#post_render' => ['\Drupal\Tests\Core\Render\RendererCallbackTest::renderCallback'], '#type' => 'container'],
--
core/tests/Drupal/Tests/Core/Render/RendererCallbackTest.php: // @phpstan-ignore-next-line
core/tests/Drupal/Tests/Core/Render/RendererCallbackTest.php- ['#access_callback' => [new static('test'), 'renderCallback'], '#type' => 'container'],
--
core/tests/Drupal/Tests/Core/Render/RendererCallbackTest.php: // @phpstan-ignore-next-line
core/tests/Drupal/Tests/Core/Render/RendererCallbackTest.php- ['#lazy_builder' => ['\Drupal\Tests\Core\Render\callback', []]],
--
core/tests/Drupal/KernelTests/Core/Entity/EntityQueryTest.php: // @phpstan-ignore-next-line
core/tests/Drupal/KernelTests/Core/Entity/EntityQueryTest.php- $this->storage->getQuery()->execute();
--
core/modules/navigation/src/Plugin/Block/NavigationShortcutsBlock.php: // @phpstan-ignore-next-line
core/modules/navigation/src/Plugin/Block/NavigationShortcutsBlock.php- '#lazy_builder' => ['navigation.shortcut_lazy_builder:lazyLinks', [$this->configuration['label']]],
--
core/modules/big_pipe/tests/modules/big_pipe_test/src/BigPipePlaceholderTestCases.php: // @phpstan-ignore-next-line
core/modules/big_pipe/tests/modules/big_pipe_test/src/BigPipePlaceholderTestCases.php- 'hello_or_hi',
--
core/modules/big_pipe/tests/modules/big_pipe_test/src/BigPipePlaceholderTestCases.php: // @phpstan-ignore-next-line
core/modules/big_pipe/tests/modules/big_pipe_test/src/BigPipePlaceholderTestCases.php- '#pre_render' => ['current_time'],
@andypost
I thought it was random failure at first, but then found an issue for it: https://www.drupal.org/project/drupal/issues/3448036 🐛 InstallerTranslationExistingFileTest fails on 11.x branch Active -- There's an MR already, so hopefully it will get fixed soon. I'll rebase on top of that when it's merged.
Opened a MR:
https://git.drupalcode.org/project/drupal/-/merge_requests/8118
Opened a draft CR:
https://www.drupal.org/node/3448089 →
diegoe → made their first commit to this issue’s fork.
Big update to formatting and structure of the page:
- Dropped the numbered list, as it was already broken
- Made all code be a proper code block
- Add another strategy to get entity UUIDs
- Avoid "here" link targets
- Use short and long command name in title, for SEO
- Highlight filenames
Just for posterity, I did some digging and this is the reason why things break:
See the routing expectations for each drupal-side module:
# https://git.drupalcode.org/project/cl_server/-/blob/2.x/cl_server.routing.yml
cl_server.render:
path: "/_cl_server"
# https://git.drupalcode.org/project/storybook/-/blob/1.x/storybook.routing.yml
storybook.render_story:
path: /storybook/stories/render/{hash}
Because `drupal/storybook` generates stories with a full `parameters.server.url` value, the `@lullabot/storybook-drupal-addon` addon will append `/_cl_server`, a _last part of the URL path_, to said URL:
// web/themes/custom/my_theme/templates/colors.stories.json
{
"title": "Color examples",
"parameters": {
"server": {
"url": "https://my-site.ddev.site/storybook/stories/render"
}
},
// https://github.com/Lullabot/storybook-drupal-addon/blob/main/src/fetchStoryHtml.ts#L46
const fetchUrl = new URL(`${url}/_cl_server`);
```
So, we end up with a URL that goes straight to `drupal/storybook`'s routing (despite the suspicious `/_cl_server` path):
<code>
# This is what `drupal/storybook` understands, a full base64 encoded JSON
/storybook/stories/render/aLongBase64Hash=
# This is what `drupal/cl_server` understands, a simple query string:
/_cl_server?_storyFileName=.%2Fweb%2Fthemes%2Fcustom%2Fmy_theme%2Ftemplates%2Fcolors.stories.json&_drupalTheme=my_theme
# This is what is getting requested:
/storybook/stories/render/_cl_server?_storyFileName=.%2Fweb%2Fthemes%2Fcustom%2Fmy_theme%2Ftemplates%2Fcolors.stories.json&_drupalTheme=my_theme
Since `drupal/storybook` is trying to `base64_decode()` the last part of the URL path, it tries to decode `"_cl_server"`, which doesn't work:
https://github.com/e0ipso/twig-storybook/blob/main/src/Service/StoryRend...
Perhaps it would be possible to have `twig-storybook` throw a more specific error if what fails is the `base64_decode()` call, like in this scenario. So that users know what "tampering with the URL" actually means in that case (?). Would you be interested in a ticket+patch for that @e0ipso?
Overall I agree that these are simply incompatible, but if anyone cared to have them both at the same time for whatever reason, then the fix is to edit your generated `.stories.json` files to point to a URL that makes sense for `@lullabot/storybook-drupal-addon`.