Invalid prop errors should be detailed in Component preview

wim leers → credited neha_bawankar → .

See the 2 "FAIL" test cases in the report by @neha_bawankar at #3528396-26: Video prop shape: e2e test coverage for video from media library the component instance form + fix revealed bug in evaluating `StaticPropSource`s → . I explained in #3528396-27: Video prop shape: e2e test coverage for video from media library the component instance form + fix revealed bug in evaluating `StaticPropSource`s → why those are both symptoms of this issue not being solved.

wim leers → credited effulgentsia → .

This is something @effulgentsia, @bnjmnm, I and possibly others discussed before this issue was created.

@lauriii wasn't around then. It came up again a few days ago. Thanks to @neha_bawankar the very confusing impact for content authors is now much clearer.

Time for Lauri to chime in and to work with designers to articulate a way forward here.

Per #3529788-32: Required `type: string` (without other constraints, so "prose strings") props should fall back to empty string (even though invalid) while Content Author is typing, to allow matching live preview in most cases → we also need to take into account when zend.assertions are set to 1 as that will also cause a render fail from \Drupal\Core\Template\ComponentsTwigExtension::validateProps which is dynamically injected into each component's twig template via \Drupal\Core\Template\ComponentNodeVisitor

Even if XB doesn't validate component input when creating the preview, that code will when assert is enabled.

We could either:

• Detect asserts are on and swap out the validator to do nothing on preview routes, OR
• Adapt the 'component failed to render' message to include assert is enabled messaging

I would vote for the first option as having to disable assert during development could be silently muting other errors

If the issue is the

          $component = Component::load($component_instance['component']);
          assert($component instanceof Component);

Could we change only this assert with a regular if if we're in preview? Something like this draft code:

          $component = Component::load($component_instance['component']);
          // Avoid asserts in preview with zend.assertions=1 to improve message to users.
          if (!$isPreview) {
            assert($component instanceof Component);
          }
          elseif (!$component instanceof Component) {
            // Or anything else.
            throw new ComponentNotFoundException($component_instance['component'] . ' doesn\'t exist.');
          }

I'm probably missing something but that's the only assert we need to sort, right?

#6: Ah, of course, that is what we missed in 📌 [PP-1] Failing kernel tests for all ways a component source can fail to render on the server side Active and 📌 Improve server-side error handling Active 😬

+1 for the proposal in #6 … except that that will then result in everything looking while to the content author while editing, but will result in a component instance failing to render on the live site? 😅 Or what am I missing?

It will depend on the twig file.

Something like a missing title here

<h1>{{ title }}</h1>

Would be white, but in other cases, there might be a fatal

{{ some_invalid_thing|t }}

We won't get to live site, because validation will prevent publishing

Oh, right. Great, then #6 is a solid plan! 👍 Thanks :)