Lima, Perú
Account created on 26 July 2008, over 16 years ago
#

Merge Requests

Recent comments

🇵🇪Peru diegoe Lima, Perú

I'm still kinda lost when it comes to all the various systems to send stuff at drupal.org, but I believe I have managed to send a functional, useful, MR:

https://git.drupalcode.org/project/field_sample_value/-/merge_requests/4

🇵🇪Peru diegoe Lima, Perú

diegoe changed the visibility of the branch field_sample_value-3459987 to hidden.

🇵🇪Peru diegoe Lima, Perú

diegoe made their first commit to this issue’s fork.

🇵🇪Peru diegoe Lima, Perú

Updated summary and removed corresponding tag.

How would I go about getting the usability team's review on this?

🇵🇪Peru diegoe Lima, Perú

> Could we open a follow up for this to address the other instances and postpone on the ticket for updating phpstan-drupal.

Opened https://www.drupal.org/project/drupal/issues/3456364 📌 Use specific Error Identifiers with @phpstan-ignore for phpstan-drupal errors Postponed , hopefully marked it correctly too.

Upstream now has https://github.com/mglaman/phpstan-drupal/issues/772

Removing Needs followup tag.

🇵🇪Peru diegoe Lima, Perú

@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?

🇵🇪Peru diegoe Lima, Perú

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.

🇵🇪Peru diegoe Lima, Perú

diegoe changed the visibility of the branch 760182_diegoe_search-block-title-and-placeholder to hidden.

🇵🇪Peru diegoe Lima, Perú

diegoe changed the visibility of the branch 3447950_diegoe_specific-phpstan-ignores to hidden.

🇵🇪Peru diegoe Lima, Perú

diegoe made their first commit to this issue’s fork.

🇵🇪Peru diegoe Lima, Perú

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'],
🇵🇪Peru diegoe Lima, Perú

diegoe made their first commit to this issue’s fork.

🇵🇪Peru diegoe Lima, Perú

Style filenames with code tags

🇵🇪Peru diegoe Lima, Perú

@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.

🇵🇪Peru diegoe Lima, Perú

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
🇵🇪Peru diegoe Lima, Perú

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`.

Production build 0.71.5 2024