Verified Commit ea8be0f9 authored by Dave Long's avatar Dave Long
Browse files

task: #3465228 Twig disallows dashes in variable names, so SDC should disallow...

task: #3465228 Twig disallows dashes in variable names, so SDC should disallow it in prop names and slots

By: pdureau
By: vensires
By: lirak
By: kwiseman
By: penyaskito
By: wim leers
(cherry picked from commit 7dd6a4c9)
parent a6c226b3
Loading
Loading
Loading
Loading
Loading
+16 −2
Changes for core/assets/schemas/v1/metadata-full.schema.json: 16 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -3,7 +3,20 @@
  "$schema": "http://json-schema.org/draft-04/schema#",
  "$defs": {
    "propDefinition": {
      "$ref": "http://json-schema.org/draft-04/schema#",
      "$comment": "Prop names are exposed as Twig variables in the component template. Twig identifiers must start with a letter or underscore and cannot contain dashes, so prop names must match ^[a-zA-Z_][a-zA-Z0-9_]*$. Same pattern as slotDefinition.",
      "allOf": [
        { "$ref": "http://json-schema.org/draft-04/schema#" },
        {
          "properties": {
            "properties": {
              "additionalProperties": false,
              "patternProperties": {
                "^[a-zA-Z_][a-zA-Z0-9_]*$": {}
              }
            }
          }
        }
      ],
      "meta:enum": {
        "type": "object",
        "minItems": 1,
@@ -22,9 +35,10 @@
    },
    "slotDefinition": {
      "type": "object",
      "$comment": "Slot names are exposed as Twig block names in the component template. Twig identifiers must start with a letter or underscore and cannot contain dashes, so slot names must match ^[a-zA-Z_][a-zA-Z0-9_]*$. Same pattern as propDefinition",
      "additionalProperties": false,
      "patternProperties": {
        "^[a-zA-Z0-9_-]+$": {
        "^[a-zA-Z_][a-zA-Z0-9_]*$": {
          "type": "object",
          "properties": {
            "title": {
+17 −2
Changes for core/assets/schemas/v1/metadata.schema.json: 17 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -3,7 +3,20 @@
  "$schema": "http://json-schema.org/draft-04/schema#",
  "$defs": {
    "propDefinition": {
      "$ref": "http://json-schema.org/draft-04/schema#",
      "$comment": "Prop names are exposed as Twig variables in the component template. Twig identifiers must start with a letter or underscore and cannot contain dashes, so prop names must match ^[a-zA-Z_][a-zA-Z0-9_]*$. Same pattern as slotDefinition.",
      "allOf": [
        { "$ref": "http://json-schema.org/draft-04/schema#" },
        {
          "properties": {
            "properties": {
              "additionalProperties": false,
              "patternProperties": {
                "^[a-zA-Z_][a-zA-Z0-9_]*$": {}
              }
            }
          }
        }
      ],
      "meta:enum": {
        "type": "object",
        "minItems": 1,
@@ -22,9 +35,10 @@
    },
    "slotDefinition": {
      "type": "object",
      "$comment": "Slot names are exposed as Twig block names in the component template. Twig identifiers must start with a letter or underscore and cannot contain dashes, so slot names must match ^[a-zA-Z_][a-zA-Z0-9_]*$. Same pattern as propDefinition",
      "additionalProperties": false,
      "patternProperties": {
        "^[a-zA-Z0-9_-]+$": {
        "^[a-zA-Z_][a-zA-Z0-9_]*$": {
          "type": "object",
          "properties": {
            "title": {
@@ -69,6 +83,7 @@
    },
    "variantDefinition": {
      "type": "object",
      "$comment": "Variant names may contain dashes because they are used as string values (CSS modifiers, equality checks) and are never converted into Twig variable or block names.",
      "additionalProperties": false,
      "patternProperties": {
        "^[a-zA-Z0-9_-]+$": {
+15 −0
Changes for core/lib/Drupal/Core/Theme/Component/ComponentValidator.php: 15 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -65,6 +65,21 @@ public function validateDefinition(array $definition, bool $enforce_schemas): bo
      );
      throw new InvalidComponentException($message);
    }
    // Prop/slot names are exposed as Twig identifiers in the component
    // template, and those must start with a letter or underscore and cannot
    // contain dashes.
    // @see https://www.php.net/manual/en/language.variables.basics.php
    $invalid_prop_names = array_filter(
      $prop_names,
      static fn(string $name) => !preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*$/', $name),
    );
    if ($invalid_prop_names) {
      throw new InvalidComponentException(sprintf(
        'The component "%s" declares prop(s) with invalid name(s): [%s]. Prop names must match the pattern "^[a-zA-Z_][a-zA-Z0-9_]*$" so they can be used as Twig variables.',
        $definition['id'],
        implode(', ', $invalid_prop_names),
      ));
    }
    // If the validator isn't set, then the validation library is not installed.
    if (!$this->validator) {
      return TRUE;
+44 −0
Changes for core/tests/Drupal/Tests/Core/Theme/Component/ComponentValidatorTest.php: 44 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -57,6 +57,18 @@ public static function dataProviderValidateDefinitionValid(): \Generator {
    foreach ($data as $definition) {
      yield [$definition];
    }

    // Variant names may contain dashes because they are used as string values
    // (e.g. CSS modifiers) and are never converted to Twig variable or block
    // names, unlike props and slots.
    $cta_with_dashed_variant = static::loadComponentDefinitionFromFs('my-cta');
    $cta_with_dashed_variant['variants'] = [
      'my-variant' => [
        'title' => 'My variant',
        'description' => 'Variant names with dashes are valid.',
      ],
    ];
    yield 'variant name with dashes is valid' => [$cta_with_dashed_variant];
  }

  /**
@@ -140,6 +152,38 @@ public static function dataProviderValidateDefinitionInvalid(): \Generator {
    ];
    yield 'invalid slot (name with spaces)' => [$cta_with_invalid_slot_name];

    $cta_with_dashed_slot_name = $valid_cta;
    $cta_with_dashed_slot_name['slots'] = [
      'invalid-slot' => [
        'title' => 'Invalid slot',
        'description' => 'Slot name cannot contain dashes because Twig disallows them in variable names.',
      ],
    ];
    yield 'invalid slot (name with dashes)' => [$cta_with_dashed_slot_name];

    $cta_with_dashed_prop_name = $valid_cta;
    $cta_with_dashed_prop_name['props']['properties']['my-prop'] = [
      'type' => 'string',
      'title' => 'My prop',
    ];
    yield 'invalid prop (name with dashes)' => [$cta_with_dashed_prop_name];

    $cta_with_leading_digit_prop_name = $valid_cta;
    $cta_with_leading_digit_prop_name['props']['properties']['1prop'] = [
      'type' => 'string',
      'title' => '1prop',
    ];
    yield 'invalid prop (name starts with digit)' => [$cta_with_leading_digit_prop_name];

    $cta_with_leading_digit_slot_name = $valid_cta;
    $cta_with_leading_digit_slot_name['slots'] = [
      '1slot' => [
        'title' => 'Invalid slot',
        'description' => 'Slot name cannot start with a digit because Twig identifiers must start with a letter or underscore.',
      ],
    ];
    yield 'invalid slot (name starts with digit)' => [$cta_with_leading_digit_slot_name];

    $cta_with_invalid_variant_title_type = $valid_cta;
    $cta_with_invalid_variant_title_type['variants'] = [
      'valid_variant' => [